From 54ac40e7c464fa21674def755026a3e99bafd88b Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Sun, 5 Dec 2010 21:48:57 +0100 Subject: [PATCH] gitweb/lib - capture output directly to cache entry file This commit makes it possible to capture output redirecting STDOUT to cache entry file; this way we don't need to store whole output in memory in order to cache it. The ->capture() method in GitwebCache::Capture::Simple is extended so it signature is now ->capture($code[, $to]). This means that it accepts optional $to parameter, which specifies either filehandle open for writing, or a file name, which would be used to capture output instead of in-memory file. The ->capture_start() and ->capture_stop() methods that are used by ->capture() also got updated. GitwebCache::SimpleFileCache acquired new ->compute_fh($key, $code_fh) method. Instead of passing and returning data like ->compute(), it passes and returns filehandles and filenames. While $code parameter in ->compute($key, $code) call is expected to generate and return data to be cached, the $code_fh parameter in ->compute_fh($key, $code_fh) call is expected to print generated data to filehandle provided as its parameter. Instead of returning data (either from cache or generated) like ->compute(), the new ->compute_fh() method returns filehandle and filename to read data from. The cache_output subroutine in GitwebCache::CacheOutput now uses ->compute_fh method if cache supports it. The contents of file that is returned by ->compute_fh is dumped to STDOUT using File::Copy::copy. Note that ->compute_fh interface is non-standard, and is not supported by any caching module known to author. Closest to it is ->handle($key) method in the Cache[1] interface (proxied to ->entry($key)->handle()). [1]: http://p3rl.org/Cache Added tests for capturing to file in t9503, and of ->compute_fh method in t9504. Inspired-by-idea-by: John 'Warthog9' Hawley Signed-off-by: Jakub Narebski --- gitweb/lib/GitwebCache/CacheOutput.pm | 17 +++++++++ gitweb/lib/GitwebCache/Capture/Simple.pm | 28 +++++++++++---- gitweb/lib/GitwebCache/SimpleFileCache.pm | 59 +++++++++++++++++++++++++++++++ t/t9503/test_cache_interface.pl | 32 +++++++++++++++++ t/t9504/test_capture_interface.pl | 17 +++++++++ 5 files changed, 146 insertions(+), 7 deletions(-) diff --git a/gitweb/lib/GitwebCache/CacheOutput.pm b/gitweb/lib/GitwebCache/CacheOutput.pm index 4a96ac9e57..7aeb895ab9 100644 --- a/gitweb/lib/GitwebCache/CacheOutput.pm +++ b/gitweb/lib/GitwebCache/CacheOutput.pm @@ -17,6 +17,8 @@ package GitwebCache::CacheOutput; use strict; use warnings; +use File::Copy; + use Exporter qw(import); our @EXPORT = qw(cache_output capture_stop); our %EXPORT_TAGS = (all => [ @EXPORT ]); @@ -38,6 +40,21 @@ sub cache_output { $capture = setup_capture($capture); + if ($cache->can('compute_fh')) { + my ($fh, $filename) = $cache->compute_fh($key, sub { + my $fh = shift; + $capture->capture($code, $fh); + }); + + if (defined $fh) { + binmode $fh, ':raw'; + binmode STDOUT, ':raw'; + copy($fh, \*STDOUT); + } + + return; + } + my $data; if ($cache->can('compute')) { $data = cache_output_compute($cache, $capture, $key, $code); diff --git a/gitweb/lib/GitwebCache/Capture/Simple.pm b/gitweb/lib/GitwebCache/Capture/Simple.pm index 3585e5848e..74dd24048e 100644 --- a/gitweb/lib/GitwebCache/Capture/Simple.pm +++ b/gitweb/lib/GitwebCache/Capture/Simple.pm @@ -18,6 +18,7 @@ use strict; use warnings; use PerlIO; +use Symbol qw(qualify_to_ref); # Constructor sub new { @@ -32,7 +33,7 @@ sub new { sub capture { my ($self, $code) = @_; - $self->capture_start(); + $self->capture_start(@_[2..$#_]); # pass rest of params $code->(); return $self->capture_stop(); } @@ -42,6 +43,7 @@ sub capture { # Start capturing data (STDOUT) sub capture_start { my $self = shift; + my $to = shift; # save copy of real STDOUT via duplicating it my @layers = PerlIO::get_layers(\*STDOUT); @@ -51,11 +53,23 @@ sub capture_start { # close STDOUT, so that it isn't used anymode (to have it fd0) close STDOUT; - # reopen STDOUT as in-memory file - $self->{'data'} = ''; - unless (open STDOUT, '>', \$self->{'data'}) { - open STDOUT, '>&', fileno($self->{'orig_stdout'}); - die "Couldn't reopen STDOUT as in-memory file for capture: $!"; + if (defined $to) { + $self->{'to'} = $to; + if (defined fileno(qualify_to_ref($to))) { + # if $to is filehandle, redirect + open STDOUT, '>&', fileno($to); + } elsif (! ref($to)) { + # if $to is name of file, open it + open STDOUT, '>', $to; + } + + } else { + # reopen STDOUT as in-memory file + $self->{'data'} = ''; + unless (open STDOUT, '>', \$self->{'data'}) { + open STDOUT, '>&', fileno($self->{'orig_stdout'}); + die "Couldn't reopen STDOUT as in-memory file for capture: $!"; + } } _relayer(\*STDOUT, \@layers); @@ -76,7 +90,7 @@ sub capture_stop { open STDOUT, '>&', fileno($self->{'orig_stdout'}); _relayer(\*STDOUT, \@layers); - return $self->{'data'}; + return exists $self->{'to'} ? $self->{'to'} : $self->{'data'}; } # taken from Capture::Tiny by David Golden, Apache License 2.0 diff --git a/gitweb/lib/GitwebCache/SimpleFileCache.pm b/gitweb/lib/GitwebCache/SimpleFileCache.pm index 581a574751..12af44f1cd 100644 --- a/gitweb/lib/GitwebCache/SimpleFileCache.pm +++ b/gitweb/lib/GitwebCache/SimpleFileCache.pm @@ -438,6 +438,65 @@ sub compute { return $data; } +# ...................................................................... +# nonstandard interface methods + +sub get_fh { + my ($self, $key) = @_; + + my $path = $self->path_to_key($key); + return unless (defined $path); + + return unless ($self->is_valid($key)); + + open my $fh, '<', $path or return; + return ($fh, $path); +} + +sub set_coderef_fh { + my ($self, $key, $code) = @_; + + my $path = $self->path_to_key($key, \my $dir); + return unless (defined $path && defined $dir); + + # ensure that directory leading to cache file exists + if (!-d $dir) { + # mkpath will croak()/die() if there is an error + mkpath($dir, 0, 0777); + } + + # generate a temporary file + my ($fh, $tempfile) = _tempfile_to_path($path, $dir); + + # code writes to filehandle or file + $code->($fh, $tempfile); + + close $fh; + rename($tempfile, $path) + or die "Couldn't rename temporary file '$tempfile' to '$path': $!"; + + open $fh, '<', $path or return; + return ($fh, $path); +} + +# ($fh, $filename) = $cache->compute_fh($key, $code); +# +# Combines the get and set operations in a single call. Attempts to +# get $key; if successful, returns the filehandle it can be read from. +# Otherwise, calls $code passing filehandle to write to as a +# parameter; contents of this file is then used as the new value for +# $key; returns filehandle from which one can read newly generated data. +sub compute_fh { + my ($self, $key, $code) = @_; + + my ($fh, $filename) = $self->get_fh($key); + if (!defined $fh) { + ($fh, $filename) = $self->set_coderef_fh($key, $code); + } + + return ($fh, $filename); +} + 1; __END__ # end of package GitwebCache::SimpleFileCache; diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl index 951304300d..0b4b09fa64 100755 --- a/t/t9503/test_cache_interface.pl +++ b/t/t9503/test_cache_interface.pl @@ -82,6 +82,38 @@ subtest 'CHI interface' => sub { done_testing(); }; +# Test the getting and setting of a cached value +# (compute_fh interface) +# +$call_count = 0; +sub write_value { + my $fh = shift; + $call_count++; + print {$fh} $value; +} +sub compute_fh_output { + my ($cache, $key, $code_fh) = @_; + + my ($fh, $filename) = $cache->compute_fh($key, $code_fh); + + local $/ = undef; + return <$fh>; +} +subtest 'compute_fh interface' => sub { + can_ok($cache, qw(compute_fh)); + + $cache->remove($key); + is(compute_fh_output($cache, $key, \&write_value), $value, + "compute_fh 1st time (set) returns '$value'"); + is(compute_fh_output($cache, $key, \&write_value), $value, + "compute_fh 2nd time (get) returns '$value'"); + is(compute_fh_output($cache, $key, \&write_value), $value, + "compute_fh 3rd time (get) returns '$value'"); + cmp_ok($call_count, '==', 1, 'write_value() is called once from compute_fh'); + + done_testing(); +}; + # Test cache expiration # subtest 'cache expiration' => sub { diff --git a/t/t9504/test_capture_interface.pl b/t/t9504/test_capture_interface.pl index 47ab804592..26c93037d5 100755 --- a/t/t9504/test_capture_interface.pl +++ b/t/t9504/test_capture_interface.pl @@ -6,6 +6,7 @@ use strict; use utf8; use Test::More; +use File::Compare; # test source version use lib $ENV{GITWEBLIBDIR} || "$ENV{GIT_BUILD_DIR}/gitweb/lib"; @@ -84,6 +85,22 @@ SKIP: { is($captured, '', "doesn't print while capturing"); } +# Test capturing to file +# +my $test_data = 'Capture this'; +open my $fh, '>', 'expected' or die "Couldn't open file for writing: $!"; +print {$fh} $test_data; +close $fh; + +$capture->capture(sub { print $test_data; }, 'actual'); +cmp_ok(compare('expected', 'actual'), '==', 0, 'capturing to file via filename'); + +open my $fh, '>', 'actual' or die "Couldn't open file for writing: $!"; +$capture->capture(sub { print $test_data; }, $fh); +close $fh; +cmp_ok(compare('expected', 'actual'), '==', 0, 'capturing to file via filehandle'); + + done_testing(); # Local Variables: -- 2.11.4.GIT