فهرست منبع

Fix the three bugs the filetest sweep turned up

Trog::Renderer::Base's pre-flight check was wrong twice over.  The condition,
'-f $t || -s $t', short-circuits on -f, so an empty file passed -- and a
*directory* passed too, on -s returning its size.  Verified: of a plain file,
an empty file, a directory and a missing path, it accepted the first three.
The author meant &&.  Separately it resolved through template_dir(), which
returns a single winning directory, while the render hands the bare name to
Xslate with the whole search path, so it could pass on a file Xslate would
not use and name a nonexistent stock path in its error while a good theme
template existed.  Both go away by letting Xslate resolve and wrapping the
render: its failure is authoritative because it uses the path that will
actually be used, and the message now names the whole search path.

Trog::Data::FlatFile::write did -f, then a separate read, then a separate
truncating open.  Two workers saving the same post could both read the
existing revisions and each write back only their own.  It is now one handle
held across the whole read-modify-write under flock.  Three concurrent
writers doing 25 saves each: before, 25 of 75 revisions survived and two
workers died on half-written JSON; after, all 75.

Trog::SQLite::dbh touched the database file before connecting.  DBI:SQLite
creates it on connect, confirmed, so the line did nothing -- and it could not
simply be made unconditional, as that bumps mtime on a live database.  Gone,
along with the File::Touch import it was the only user of.

perlcritic bin lib is now completely clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJ2NAMQyytncwHKSmD5dFe
George Baugh 1 ماه پیش
والد
کامیت
fe2c0cea0f
3فایلهای تغییر یافته به همراه33 افزوده شده و 18 حذف شده
  1. 18 9
      lib/Trog/Data/FlatFile.pm
  2. 13 7
      lib/Trog/Renderer/Base.pm
  3. 2 2
      lib/Trog/SQLite.pm

+ 18 - 9
lib/Trog/Data/FlatFile.pm

@@ -3,7 +3,8 @@ package Trog::Data::FlatFile;
 use v5.36;
 use re '/aa';
 
-use Carp qw{confess};
+use Carp  qw{confess};
+use Fcntl qw{LOCK_EX SEEK_SET};
 use JSON::MaybeXS;
 use File::Slurper;
 use File::Copy;
@@ -113,16 +114,24 @@ sub write ( $self, $data ) {
     foreach my $post (@$data) {
         my $file   = "$datastore/$post->{id}";
         my $update = [$post];
-        if ( -f $file ) {
-            my $slurped = File::Slurper::read_binary($file);
-            my $parsed  = $parser->decode($slurped);
-
-            $update = [ ( @$parsed, $post ) ];
-        }
 
         mkdir $datastore;
-        open( my $fh, '>', $file ) or confess "Could not open $file";
-        print $fh $parser->encode($update);
+
+        # One locked handle for the whole read-modify-write.  This used to be a
+        # -f, then a separate read, then a separate truncating open; two workers
+        # saving the same post could both read the existing revisions and then
+        # each write back only their own, silently dropping the post's history.
+        # '+>>' creates the file if it is not there, and never truncates.
+        open( my $fh, '+>>', $file ) or confess "Could not open $file: $!";
+        flock( $fh, LOCK_EX )        or confess "Could not lock $file: $!";
+
+        seek( $fh, 0, SEEK_SET ) or confess "Could not rewind $file: $!";
+        my $slurped = do { local $/; <$fh> };
+        $update = [ ( @{ $parser->decode($slurped) }, $post ) ] if length( $slurped // '' );
+
+        seek( $fh, 0, SEEK_SET ) or confess "Could not rewind $file: $!";
+        truncate( $fh, 0 )       or confess "Could not truncate $file: $!";
+        print {$fh} $parser->encode($update);
         close $fh;
 
         Trog::SQLite::TagIndex::add_post( $post, $self );

+ 13 - 7
lib/Trog/Renderer/Base.pm

@@ -40,18 +40,14 @@ Recognized options:
     child_processor / child_renderer => override how post bodies are rendered by
                    the render_it() template function.  Built for you if omitted.
 
-Dies unless the template exists.  Returns the body string for components, and a
-PSGI arrayref of [ code, headers, body ] otherwise.
+Dies unless Xslate can resolve and render the template.  Returns the body
+string for components, and a PSGI arrayref of [ code, headers, body ] otherwise.
 
 =cut
 
 sub render (%options) {
     die "Templated renders require a template to be passed" unless $options{template};
 
-    my $template_dir = Trog::Themes::template_dir( $options{template}, $options{contenttype}, $options{component} );
-    my $t            = "$template_dir/$options{template}";
-    die "Templated renders require an existing template to be passed, got $template_dir/$options{template}" unless -f $t || -s $t;
-
     # Xslate resolves includes against every dir in path, first match winning,
     # so handing it both lets a theme override individual templates without
     # having to fork the ones next to them.  With no theme configured this is
@@ -80,7 +76,17 @@ sub render (%options) {
     );
 
     my $code = $options{code};
-    my $body = encode_utf8( $renderers{$renderer_key}->render( $options{template}, $options{data} ) );
+
+    # Xslate resolves the template itself, against the same @template_dirs it
+    # will actually use.  There used to be a pre-flight test here, and it was
+    # wrong twice over: it resolved through template_dir(), which returns one
+    # winning directory rather than the search path, so it could pass on a file
+    # Xslate would not use and name a nonexistent stock path in its error while
+    # a good theme template existed.  And the condition itself, -f $t || -s $t,
+    # short-circuited on -f, so an empty file passed, while a *directory* passed
+    # on -s returning its size.
+    my $body = eval { encode_utf8( $renderers{$renderer_key}->render( $options{template}, $options{data} ) ) };
+    die "Could not render template '$options{template}' (searched @template_dirs): $@" unless defined $body;
 
     # Users can supply a post_processor to futz with the output (such as with minifiers) if they wish.
     $body = $options{post_processor}->($body) if $options{post_processor} && ref $options{post_processor} eq 'CODE';

+ 2 - 2
lib/Trog/SQLite.pm

@@ -7,7 +7,6 @@ use POSIX qw{floor};
 
 use DBI;
 use DBD::SQLite;
-use File::Touch;
 use File::Slurper();
 use List::Util qw{any};
 
@@ -44,7 +43,8 @@ sub dbh {
     $dbh //= {};
     return $dbh->{$dbname} if $dbh->{$dbname};
 
-    File::Touch::touch($dbname) unless -f $dbname;
+    # No touch first: DBI:SQLite creates the file on connect.  It also cannot
+    # simply be made unconditional, as that would bump mtime on a live database.
     my $db = DBI->connect( "dbi:SQLite:dbname=$dbname", "", "" );
 
     if ($schema) {