problem with caching and cascade

[rmesser]

Text::Xslate is a great module, but I think there is a bug when using cascade with multiple directories in the path. Essentially if you have "path_b" and then "path_a" listed in the path input to Text::Xslate->new, and then have a file in path_b that should cascade to a file in path_a, it doesn't work. It does work if you set cache => 0, which leads me to believe that checking the freshness of the cache isn't working in this scenario.

Here is a test script that illustrates the issue. I believe this should pass, but it doesn't with Text::Xslate 3.0.0 (and prior versions that I tried).

use Text::Xslate;
use Test::More tests => 2;

use strict;

use File::Slurp qw(write_file);
use File::Path qw(remove_tree);

# here's some test template files
my $base_a = <<'EOF';
here is base A
: block body -> {}
EOF

my $sub_a = <<'EOF';
: cascade base
: override body {
this is sub A
: }
EOF

my $sub_b = <<'EOF';
: cascade base
: override body {
this is sub B
: }
EOF

# remove old directories if they exist and re-create
remove_tree('path_a', 'path_b');
mkdir 'path_a';
mkdir 'path_b';

write_file('path_a/base.tx', $base_a);
write_file('path_a/sub.tx', $sub_a);

my $tx = Text::Xslate->new(
    path => ['path_b', 'path_a'],

    # NOTE: this test will pass if the following line is uncommented
    # cache => 0
);

my $out = $tx->render('sub.tx');

# expect both base and sub A since there is nothing in path B
is($out, "here is base A\nthis is sub A\n", "cascade with base in same directory");

# now a new path_b sub file, and render again
write_file('path_b/sub.tx', $sub_b);
my $out2 = $tx->render('sub.tx');

# we should get the A base (since there is no B base) with the B sub (since that is first in path)
is($out2, "here is base A\nthis is sub B\n", "cascade with base in different directory");

[rmesser]

@gfx, thank you for the fix. However, in my testing there is still a problem with caching and cascading. For example, the test from my original posting still fails with version 3.1.0, which I just installed from CPAN. Do it also fail for you, and do you agree that it should pass? Thanks,

Rob

[gfx]

OMG, this ticket was incorrectly closed :(

[rmesser]

This issue is slowing down my application somewhat, so I'd happily pay to sponsor a fix. @gfx, is there any chance you would be interested in such a thing? If so please let me know. Thank you.

[gfx]

@rmesser Can you wait a week? I'll look into this issue in the next weekend.

[rmesser]

Sure, a week is not a big deal. Please let me know what you find out and if you think a sponsored bug fix makes sense. Thanks.

[gfx]

I have looked into this issue and have found it is exactly an issue on Xslate caching system, but it's not easy to fix. Please wait one more week.

FYI: to workaround this issue, re-create the Xslate instance after adding a file to template paths in order to clear in-memory caches.

[rmesser]

Ok, thank you for the update and for the suggested work-around. I will try your suggestion and also look forward to your next update. Let me know if there is anything I can do to help.

[rmesser]

Any update on this? The work-around does work, but it's a little awkward because with the way we are using Xslate, users can add templates and so we have to add code to monitor directories or just turn caching off entirely. (Sorry in advance to ask again about it, but just wondering.) Thanks.

[rmesser]

Just to follow-up on this... it took a while for me to get around to it (closer to our product release so that performance matters more), but I did implement a work-around. I figured I might as well post it here. And @gfx, I can see why you didn't implement a fix for this. Really it is kind of an odd corner case -- the cached files are invalidated by a file which didn't even exist when the cache was created. So why would you want to check for that every time, even if the cache => 1 setting is used? I suppose using the path, you could keep a list of the files that could invalidate the cache, and include files that don't exist. Then if one of those files was created, you would check for it just like you would for the files that were actually used to create the cache. But then to be thorough you'd also have to check for the deletion of files, since if the template used cascade and multiple paths, removing a file in the path could cause the compiled version to be built using a different file.

So if you don't think it is worth it to change the current behavior, I can certainly understand that. What we are doing is simply killing the entire cache when a new file arrives, and checking that once per web request. For us this works because the files don't change very often.

As a side note, I think it might not be a bad idea for Text::Xslate to have some cache handling and clearing methods in addition to the "cache" input. For us, cache => 1 is too often -- we might use the same template many times on one web page, and checking with each use seems wasteful. But cache => 2 never checks at all, leaving it up to the programmer to clear the cache with some method like I have below. Really what I'd ideally like as a user of Text::Xslate is for Text::Xslate to clear the cache on demand. That way I wouldn't have to set cache => 1, but I also wouldn't have to write my own method for clearing the cache when using cache => 2.

Anyway, here is the simple cache clearing code we are using, a bit rough but it works for our purposes.

#==
# Check for a file in our path that is later than any file in our cache directory.  If found, clear the cache
# entirely.  This is a fairly rough approach, but template files don't change that often, so we won't incur the cost
# of deleting all cache files very often either.
#
# Return a true value if the cache was cleared, false otherwise.
#==
sub _check_xslate_cache {
    my ( $cache_dir, $template_paths ) = @_;

    my $latest_cached = 0;
    my $path;

    my $iter = Path::Tiny::path($cache_dir)->iterator( { recurse => 1 } );
    while ( $path = $iter->() ) {
        if ( $path->is_file ) {
            $latest_cached = List::Util::max( $latest_cached, _hr_mtime($path) );
        }
    }

    return 0 if not $latest_cached;    # nothing to clear if no cached files found

    my $clear_cache = 0;

    # now if any files under $paths are more recent, clear the entire cache.  This is a bit indiscriminate, because
    # in theory we could only clear the files impacted, not the entire directory.  But it's not worth the trouble
    # since .tx files don't change very often.
CHECK_TIMES: foreach my $source_dir (@$template_paths) {
        $iter = Path::Tiny::path($source_dir)->iterator( { recurse => 1 } );
        while ( $path = $iter->() ) {
            if ( $path->is_file and _hr_mtime($path) > $latest_cached ) {
                $clear_cache = 1;
                last CHECK_TIMES;    # once we find one file newer than $latest_cached, no need to look further
            }
        }
    }

    if ($clear_cache) {
        Path::Tiny::path($cache_dir)->remove_tree;
    }

    return $clear_cache;
}

# helper function take a Path::Tiny object and return the hi-res mtime (modification time).  We could use the built-in
# Path::Tiny::stat method, which uses File::stat, but that only returns second resolutions.  Sometimes caching is done
# in the same second as creating the .tx file, so we prefer sub-second resolution like Time::HiRes::stat.  Of course,
# this may come up in test scripts more than in production, but still seems useful.
sub _hr_mtime {
    # magic numbers here: 0 is the index where Path::Tiny stores the full filename,
    # and 9 is the index to mtime for stat.
    return (Time::HiRes::stat(shift->[0]))[9];
}

[jomo666]

Any idea about permanent fix? Me english is bad, and I don't understand the "warnocked". I just hope that isn't mean something like: "this will be never fixed" :) :)