Item14061: Fix cache refresh code

And add unit tests. - The tests for cache header were completely ineffective. - Core didn't add a cache header, so they were moot too. - refresh=cache was ignored - There were no unit tests covering the refresh options.
foswiki · May 1, 2016 · 5df1654 · 5df1654
1 parent d469821
commit 5df1654
Show file tree

Hide file tree

Showing 3 changed files with 205 additions and 9 deletions.
diff --git a/UnitTestContrib/test/unit/CacheTests.pm b/UnitTestContrib/test/unit/CacheTests.pm
@@ -59,7 +59,12 @@ sub fixture_groups {
         }
     }
 
-    return ( \@page, [ 'view', 'rest' ], [ 'NoCompress', 'Compress' ] );
+    return (
+        \@page,
+        [ 'view',       'rest' ],
+        [ 'NoCompress', 'Compress' ],
+        [ 'refresh_fire', 'refresh_on', 'refresh_cache', 'timing' ],
+    );
 }
 
 sub dbcheckDBI {
@@ -250,12 +255,12 @@ sub check {
     $this->assert( $one =~ s/\r//g,          'Failed to remove \r' );
     $this->assert( $one =~ s/^(.*?)\n\n+//s, 'Failed to remove HTTP headers' );
     my $one_head = $1;
-    $this->assert( $one_head !~ /X-Foswiki-Pagecache/i, $one_head );
+    $this->assert_does_not_match( qr/X-Foswiki-Pagecache/i, $one_head );
 
     $this->assert( $two =~ s/\r//g,          'Failed to remove \r' );
     $this->assert( $two =~ s/^(.*?)\n\n+//s, 'Failed to remove HTTP headers' );
     my $two_heads = $1;
-    $this->assert( $two_heads =~ /^X-Foswiki-Pagecache: 1$/im, $two_heads );
+    $this->assert_matches( qr/X-Foswiki-Pagecache: 1/i, $two_heads );
 
     print STDERR "To cache:   "
       . timestr( timediff( $p1end, $p1start ) ) . "\n";
@@ -292,14 +297,131 @@ s/<(span|div)([^>]*?)(\d+?)(show|hide|toggle)([^>]*?)>/'<'.$1.$2._mangleID($3).$
     return;
 }
 
+sub check_refresh {
+
+    my $this     = shift;
+    my $pathinfo = shift;
+    my $refresh  = shift;
+
+    $UI_FN ||= $this->getUIFn( $this->{uifn} );
+    $Foswiki::cfg{Cache}{Debug} = 1;
+    my $query = Unit::Request->new( { skin => ['none'], } );
+    $query->path_info($pathinfo);
+    $query->method('GET');
+
+    $this->createNewFoswikiSession( $this->{test_user_login},
+        $query, { $this->{uifn} => 1 } );
+
+    # This first request should *not* be satisfied from the cache, but
+    # the cache should be populated with the result.
+    my ( $one, $result, $stdout, $stderr ) = $this->capture(
+        sub {
+            no strict 'refs';
+            &{$UI_FN}( $this->{session} );
+            use strict 'refs';
+            $Foswiki::engine->finalize( $this->{session}{response},
+                $this->{session}{request} );
+        }
+    );
+
+    $this->createNewFoswikiSession( $this->{test_user_login},
+        $query, { $this->{uifn} => 1 } );
+
+    # This second request should be satisfied from the cache
+    # How do we know it was?
+    ( my $two, $result, $stdout, $stderr ) = $this->capture(
+        sub {
+            no strict 'refs';
+            &{$UI_FN}( $this->{session} );
+            use strict 'refs';
+            $Foswiki::engine->finalize( $this->{session}{response},
+                $this->{session}{request} );
+        }
+    );
+
+    $query = Unit::Request->new( { skin => ['none'], refresh => $refresh, } );
+    $query->path_info($pathinfo);
+    $query->method('GET');
+    $this->createNewFoswikiSession( $this->{test_user_login},
+        $query, { $this->{uifn} => 1 } );
+
+    # This third request with refresh should not be satisfied from the cache
+    # How do we know it was?
+    ( my $three, $result, $stdout, $stderr ) = $this->capture(
+        sub {
+            no strict 'refs';
+            &{$UI_FN}( $this->{session} );
+            use strict 'refs';
+            $Foswiki::engine->finalize( $this->{session}{response},
+                $this->{session}{request} );
+        }
+    );
+    $this->assert( $one =~ s/\r//g,          'Failed to remove \r' );
+    $this->assert( $one =~ s/^(.*?)\n\n+//s, 'Failed to remove HTTP headers' );
+    my $one_head = $1;
+    $this->assert_does_not_match( qr/X-Foswiki-Pagecache/i, $one_head );
+
+    $this->assert( $two =~ s/\r//g,          'Failed to remove \r' );
+    $this->assert( $two =~ s/^(.*?)\n\n+//s, 'Failed to remove HTTP headers' );
+    my $two_heads = $1;
+    $this->assert_matches( qr/X-Foswiki-Pagecache: 1/i, $two_heads );
+
+    $this->assert( $three =~ s/\r//g, 'Failed to remove \r' );
+    $this->assert( $three =~ s/^(.*?)\n\n+//s,
+        'Failed to remove HTTP headers' );
+    my $three_heads = $1;
+
+    if ($refresh) {
+        $this->assert_does_not_match( qr/X-Foswiki-Pagecache: 1/i,
+            $three_heads );
+    }
+    else {
+        $this->assert_matches( qr/X-Foswiki-Pagecache: 1/i, $three_heads );
+    }
+
+    return;
+}
+
+sub refresh_on {
+    my $this = shift;
+    $this->{refresh} = 'on';
+}
+
+sub refresh_cache {
+    my $this = shift;
+    $this->{refresh} = 'cache';
+}
+
+sub refresh_fire {
+    my $this = shift;
+    $this->{refresh} = 'fire';
+}
+
+sub timing {
+    my $this = shift;
+    $this->{refresh} = undef;
+}
+
 sub verify_simple {
     my $this = shift;
-    $this->check('/');
+
+    if ( $this->{refresh} ) {
+        $this->check_refresh( '/', $this->{refresh} );
+    }
+    else {
+        $this->check('/');
+    }
 }
 
 sub verify_topic {
     my $this = shift;
-    $this->check("/$Foswiki::cfg{SystemWebName}/FileAttribute");
+    if ( $this->{refresh} ) {
+        $this->check_refresh( "/$Foswiki::cfg{SystemWebName}/FileAttribute",
+            $this->{refresh} );
+    }
+    else {
+        $this->check("/$Foswiki::cfg{SystemWebName}/FileAttribute");
+    }
 }
 
 sub verify_utf8_topic {
@@ -312,7 +434,73 @@ sub verify_utf8_topic {
     $meta->text($topic);
     $meta->save();
 
-    $this->check( Encode::encode_utf8("/$web/$topic") );
+    if ( $this->{refresh} ) {
+        $this->check_refresh( Encode::encode_utf8("/$web/$topic"),
+            $this->{refresh} );
+    }
+    else {
+        $this->check( Encode::encode_utf8("/$web/$topic") );
+    }
+}
+
+#  Make sure that only admin users can issue refresh_all
+sub test_refresh_all {
+    my $this = shift;
+
+    SQLite();
+    $Foswiki::cfg{Cache}{Enabled} = 1;
+
+    my $pathinfo = "/";
+
+    $UI_FN ||= $this->getUIFn("view");
+    $Foswiki::cfg{Cache}{Debug} = 1;
+    my $query = Unit::Request->new( { skin => ['none'], refresh => 'all', } );
+
+    $query->path_info($pathinfo);
+    $query->method('GET');
+
+    $this->createNewFoswikiSession( $this->{test_user_login},
+        $query, { view => 1 } );
+
+    my ( $one, $result, $stdout, $stderr ) = $this->capture(
+        sub {
+            try {
+                no strict 'refs';
+                &{$UI_FN}( $this->{session} );
+                use strict 'refs';
+                $Foswiki::engine->finalize( $this->{session}{response},
+                    $this->{session}{request} );
+            }
+            catch Foswiki::OopsException with {
+                my $e = shift;
+                $this->assert_str_equals( "cache_refresh", $e->{def},
+                    $e->stringify() );
+                $this->assert_str_equals( "accessdenied", $e->{template},
+                    $e->stringify() );
+            };
+
+        }
+    );
+
+    $this->createNewFoswikiSession( $Foswiki::cfg{AdminUserLogin},
+        $query, { view => 1 } );
+
+    ( my $two, $result, $stdout, $stderr ) = $this->capture(
+        sub {
+            try {
+                no strict 'refs';
+                &{$UI_FN}( $this->{session} );
+                use strict 'refs';
+                $Foswiki::engine->finalize( $this->{session}{response},
+                    $this->{session}{request} );
+            }
+            catch Foswiki::OopsException with {
+                my $e = shift;
+                $this->assert( 0, "Incorrect exception: " . $e->stringify() );
+            };
+
+        }
+    );
 }
 
 1;
diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
@@ -912,8 +912,6 @@ sub satisfiedByCache {
             $hdrs->{'Content-Encoding'} = $encoding;
             $hdrs->{'Vary'}             = 'Accept-Encoding';
 
-            # Mark the response so we know it was satisfied from the cache
-            $hdrs->{'X-Foswiki-PageCache'} = 1;
         }
         else {
         # e.g. CLI request satisfied from the cache, or old browser that doesn't
@@ -941,6 +939,8 @@ sub satisfiedByCache {
     }
 
     # set remaining headers
+    # Mark the response so we know it was satisfied from the cache
+    $hdrs->{'X-Foswiki-PageCache'} = 1;
     $text = undef unless $this->setETags( $cachedPage, $hdrs );
     $this->generateHTTPHeaders($hdrs);
 

diff --git a/core/lib/Foswiki/PageCache.pm b/core/lib/Foswiki/PageCache.pm
@@ -223,7 +223,7 @@ sub cachePage {
     my $variationKey = $this->genVariationKey();
 
     # remove old entries
-    if ( $session->inContext("isadmin") && $refresh =~ m/^(on|cache|all)$/ ) {
+    if ( $refresh =~ m/^(on|cache|all)$/ ) {
         $this->deletePage( $web, $topic );    # removes all variations
     }
     else {
@@ -316,6 +316,7 @@ sub getPage {
 
         if ( $session->{users}->isAdmin( $session->{user} ) ) {
             $this->deleteAll();
+            return undef;
         }
         else {
             my $session = $Foswiki::Plugins::SESSION;
@@ -334,6 +335,13 @@ sub getPage {
 
     if ( $refresh eq 'fire' ) {    # simulates a "save" of the current topic
         $this->fireDependency( $web, $topic );
+
+        #return undef;
+    }
+
+    if ( $refresh =~ m/^(on|cache)$/ ) {
+        $this->deletePage( $web, $topic );    # removes all variations
+                                              #return undef;
     }
 
     # check cacheability