[Catalyst-commits] r13674 - in trunk/Catalyst-Plugin-PageCache: . lib/Catalyst/Plugin

timbunce at dev.catalyst.perl.org timbunce at dev.catalyst.perl.org
Mon Nov 1 15:23:57 GMT 2010


Author: timbunce
Date: 2010-11-01 15:23:57 +0000 (Mon, 01 Nov 2010)
New Revision: 13674

Modified:
   trunk/Catalyst-Plugin-PageCache/Changes
   trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm
Log:
Refactored page cache storage logic, inspired by RT#53303.

Simplified logic in cache_page().
Removed difference in handling between cache_page($x) and cache_page(cache_seconds => $x).
Use $expires->can('epoch') instead of isa DateTime.
Added new _get_page_cache_expiration_time() method.
Refactored a new _store_page_in_cache() out of finalize().



Modified: trunk/Catalyst-Plugin-PageCache/Changes
===================================================================
--- trunk/Catalyst-Plugin-PageCache/Changes	2010-11-01 12:47:56 UTC (rev 13673)
+++ trunk/Catalyst-Plugin-PageCache/Changes	2010-11-01 15:23:57 UTC (rev 13674)
@@ -10,6 +10,7 @@
         - Allow key_maker to be the name of a method to be called on $c. RT#53529.
         - Assorted performance optimizations.
         - Add cache_dispatch_hook and cache_finalize_hook to config. RT#53503.
+        - Refactored page cache storage logic, inspired by RT#53303.
 
 0.22    2009-06-25 10:38:00
         - Update to use MRO::Compat

Modified: trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm
===================================================================
--- trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm	2010-11-01 12:47:56 UTC (rev 13673)
+++ trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm	2010-11-01 15:23:57 UTC (rev 13674)
@@ -18,31 +18,26 @@
 
 sub cache_page {
     my ( $c, @args ) = @_;
+    my %options;
 
-    # Parameters passed in?
+    # Mark that cache_page has been called but defer on selecting the time
+    # period to cache for until finalize. _get_page_cache_expiration_time has
+    # more options at that time, such as various headers which may have been set.
 
-    if ( ref($args[0]) eq 'HASH' || @args > 1 ) {
-        my $options = ref( $args[0] ) ? shift : { @args };
+    if (@args == 1) {
+        my $expires = shift @args;
 
-        $options->{cache_seconds} = $c->config->{'Plugin::PageCache'}->{expires}
-            unless exists $options->{cache_seconds};
+        # Allow specific end time
+        $expires = $expires->epoch - time()
+            if ref($expires) && $expires->can('epoch');
 
-        $c->_cache_page( $options );
-        return;
+        $options{cache_seconds} = $expires;
     }
+    else {
+        %options = @args;
+    }
 
-    my $expires = shift @args;
-
-    # Allow specific end time
-    $expires = $expires->epoch - time
-        if ref($expires) && $expires->isa('DateTime');
-
-    $expires ||= $c->config->{'Plugin::PageCache'}->{expires};
-
-    # mark the page for caching during finalize
-    if ( $expires > 0 ) {
-        $c->_cache_page( { cache_seconds => $expires } );
-    }
+    $c->_cache_page( \%options );
 }
 
 sub clear_cached_page {
@@ -85,6 +80,37 @@
     }
 }
 
+
+# return the time that the item should expire
+sub _get_page_cache_expiration_time {
+    my ($c, $options) = @_;
+    my $config_PageCache = $c->config->{'Plugin::PageCache'};
+    my $is_debug = $config_PageCache->{debug};
+
+    my $expires;
+
+    # Use the explicitely passed in duration if available.
+    # XXX but why then ignore it if it's <= 0 ?
+    if ($options->{cache_seconds} and $options->{cache_seconds} > 0) {
+        $c->log->debug("expires in specified $options->{cache_seconds}s")
+            if $is_debug;
+
+        $expires = time() + $options->{cache_seconds};
+    }
+    # else {
+    #   ... calculate expiry based on the response headers
+    # }
+    # If all else fails, fallback to the default 'expires' configuration value.
+    else {
+        $c->log->debug("expires in default $config_PageCache->{expires}s")
+            if $is_debug;
+        $expires = time() + $config_PageCache->{expires};
+    }
+
+    return $expires;
+}
+
+
 sub dispatch {
     my $c = shift;
 
@@ -170,7 +196,8 @@
 }
 
 # See if request matches last_modified date in cache
-# and return true if so.
+# if so, arrange to return a 304 Not Modified status
+# and return true.
 
 sub _page_cache_not_modified {
     my ( $c, $data ) = @_;
@@ -276,61 +303,73 @@
         }
     }
 
-    if ( my $options = $c->_cache_page ) {
-        my $key = $c->_get_page_cache_key;
+    if ($c->_cache_page) {
+        my $data = $c->_store_page_in_cache($c->_cache_page) ;
 
-        my $now = time;
-        my $headers = $c->res->headers;
+        # Check for If-Modified-Since
+        $c->_page_cache_not_modified( $data ) if $data;
+    }
 
-        $c->log->debug(
-            "Caching page $key for $options->{cache_seconds} seconds"
-        ) if ($config_PageCache->{debug});
+    return $c->next::method(@_);
+}
 
-        # Cache some additional metadata along with the content
-        # Some caches don't support expirations, so we do it manually
-        my $data = {
-            body => $c->res->body || undef,
-            content_type => [ $c->res->content_type ] || undef,
-            content_encoding => $c->res->content_encoding || undef,
-            create_time      => $options->{last_modified}
-                || $headers->last_modified
-                || $now,
-            expire_time => $now + $options->{cache_seconds},
+
+sub _store_page_in_cache {
+    my ($c, $options) = @_;
+
+    my $key = $c->_get_page_cache_key;
+    my $config_PageCache = $c->config->{'Plugin::PageCache'};
+
+    my $headers = $c->res->headers;
+
+    # Cache some additional metadata along with the content
+    # Some caches don't support expirations, so we do it manually
+    my $data = {
+        body => $c->res->body || undef,
+        content_type => [ $c->res->content_type ] || undef,
+        content_encoding => $c->res->content_encoding || undef,
+        create_time      => $options->{last_modified}
+            || $headers->last_modified
+            || $now,
+        expire_time => $c->_get_page_cache_expiration_time($options),
+    };
+
+    return undef if $data->{expire_time} <= $now;
+
+    $c->log->debug(
+        "Caching page $key for ". ($data->{expire_time} - time()) ." seconds"
+    ) if ($config_PageCache->{debug});
+    
+    if ( $config_PageCache->{cache_headers} ) {
+        $data->{headers} = {
+            map { $_ => $headers->header($_) }
+                $headers->header_field_names
         };
-        
-        if ( $config_PageCache->{cache_headers} ) {
-            $data->{headers} = {
-                map { $_ => $headers->header($_) }
-                  $headers->header_field_names
-            };
-        }
+    }
 
-        $data->{expires} = $options->{expires} if exists $options->{expires};
+    $data->{expires} = $options->{expires} if exists $options->{expires};
 
-        my $cache = $c->cache; # curry the cache just once, here
+    my $cache = $c->cache; # curry the cache just once, here
 
-        $cache->set( $key, $data );
+    $cache->set( $key, $data );
 
-        $c->_set_page_cache_headers( $data );  # don't forget the first time
+    $c->_set_page_cache_headers( $data );  # don't forget the first time
 
-        if ( !$config_PageCache->{disable_index} ) {
-            # Keep an index cache of all pages that have been cached, for use
-            # with clear_cached_page
-            my $index = $cache->get( "_page_cache_index" ) || {};
-            $index->{$key} = 1;
+    if ( !$config_PageCache->{disable_index} ) {
+        # Keep an index cache of all pages that have been cached, for use
+        # with clear_cached_page
+        my $index = $cache->get( "_page_cache_index" ) || {};
+        $index->{$key} = 1;
 
-            # Save date in cache
-            $cache->set( "_page_cache_index", $index,
-                $config_PageCache->{no_expire});
-        }
-
-        # Check for If-Modified-Since
-        $c->_page_cache_not_modified( $data );
+        # Save date in cache
+        $cache->set( "_page_cache_index", $index,
+            $config_PageCache->{no_expire});
     }
 
-    return $c->next::method(@_);
+    return $data;
 }
 
+
 sub setup {
     my $c = shift;
 
@@ -626,7 +665,7 @@
 The page will be cached for $expire seconds.  Every user who visits the URI(s)
 referenced by that controller will receive the page directly from cache.  Your
 controller will not be processed again until the cache expires.  You can set
-this value to as low as 60 seconds if you have heavy traffic to greatly
+this value to a low value, such as 60 seconds, if you have heavy traffic, to greatly
 improve site performance.
 
 Pass in a DateTime object to make the cache expire at a given point in time.
@@ -654,19 +693,15 @@
 
 =over 4
 
-=item last_modified
-
-Last modified time in epoch seconds.  If not set will use either the
-current Last-Modified header, or if not set, the current time.
-
 =item cache_seconds
 
 This is the number of seconds to keep the page in the page cache, which may be
 different (normally longer) than the time that client caches may store the page.
+This is the value set when only a single parameter is passed.
 
 =item expires
 
-This is the lenght of time in seconds that a client may cache the page
+This is the length of time in seconds that a client may cache the page
 before revalidating (by asking the server if the document has changed).
 
 Unlike above, this is a fixed setting that each client will see.  Regardless of
@@ -677,6 +712,11 @@
 will be sent telling the client to not cache the page.  Allows caching expensive
 content to generate, but any changes will be seen right away.
 
+=item last_modified
+
+Last modified time in epoch seconds.  If not set will use either the
+current Last-Modified header, or if not set, the current time.
+
 =back
 
 =head2 clear_cached_page




More information about the Catalyst-commits mailing list