Add check_last_modified_header option - do not add ETag if Last-Modified set

http://code.google.com/speed/page-speed/docs/caching.html#LeverageBrowserCaching

"It is important to specify one of Expires or Cache-Control max-age,
and _one_ of Last-Modified or ETag"

3 files changed, 87 insertions, 14 deletions

diff --git a/Changes b/Changes
index 083f5eb..8a5ed14 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,10 @@
 Revision history for Perl extension Plack::Middleware::ETag
 
+0.03    Thu Apr 14 17:25:26 2011
+        - add check_last_modified_header (patch by Ranguard)
+        http://code.google.com/speed/page-speed/docs/caching.html#LeverageBrowserCaching
+        recommends not having both Last-Modified and ETag
+
 0.02    Sat Jan 22 10:22:50 2011
         - add cache-control as suggested by ingy and miyagawa
 
diff --git a/lib/Plack/Middleware/ETag.pm b/lib/Plack/Middleware/ETag.pm
index 187b7b1..3623747 100644
--- a/lib/Plack/Middleware/ETag.pm
+++ b/lib/Plack/Middleware/ETag.pm
@@ -4,23 +4,27 @@ use strict;
 use warnings;
 use Digest::SHA;
 use Plack::Util;
-use Plack::Util::Accessor qw( file_etag cache_control);
+use Plack::Util::Accessor
+    qw( file_etag cache_control check_last_modified_header);
 
-our $VERSION = '0.02';
+our $VERSION = '0.03';
 
 use parent qw/Plack::Middleware/;
 
 sub call {
     my $self = shift;
-    my $res     = $self->app->(@_);
+    my $res  = $self->app->(@_);
 
     $self->response_cb(
         $res,
         sub {
-            my $res = shift;
+            my $res     = shift;
             my $headers = $res->[1];
             return if ( !defined $res->[2] );
             return if ( Plack::Util::header_exists( $headers, 'ETag' ) );
+            return
+                if ( $self->check_last_modified_header()
+                && Plack::Util::header_exists( $headers, 'Last-Modified' ) );
 
             my $etag;
 
@@ -29,23 +33,23 @@ sub call {
                 my $file_attr = $self->file_etag || [qw/inode mtime size/];
                 my @stats = stat $res->[2];
                 if ( $stats[9] == time - 1 ) {
-                    # if the file was modified less than one second before the request
-                    # it may be modified in a near future, so we return a weak etag
+
+            # if the file was modified less than one second before the request
+            # it may be modified in a near future, so we return a weak etag
                     $etag = "W/";
                 }
                 if ( grep {/inode/} @$file_attr ) {
-                    $etag .= (sprintf "%x", $stats[2]);
+                    $etag .= ( sprintf "%x", $stats[2] );
                 }
                 if ( grep {/mtime/} @$file_attr ) {
-                    $etag .= "-" if ($etag && $etag !~ /-$/);
+                    $etag .= "-" if ( $etag && $etag !~ /-$/ );
                     $etag .= ( sprintf "%x", $stats[9] );
                 }
                 if ( grep {/size/} @$file_attr ) {
-                    $etag .= "-" if ($etag && $etag !~ /-$/);
+                    $etag .= "-" if ( $etag && $etag !~ /-$/ );
                     $etag .= ( sprintf "%x", $stats[7] );
                 }
-            }
-            else {
+            } else {
                 my $sha = Digest::SHA->new;
                 $sha->add( @{ $res->[2] } );
                 $etag = $sha->hexdigest;
@@ -64,9 +68,9 @@ sub _set_cache_control {
     if ( ref $self->cache_control && ref $self->cache_control eq 'ARRAY' ) {
         Plack::Util::header_set( $headers, 'Cache-Control',
             join( ', ', @{ $self->cache_control } ) );
-    }
-    else {
-        Plack::Util::header_set( $headers, 'Cache-Control', 'must-revalidate' );
+    } else {
+        Plack::Util::header_set( $headers, 'Cache-Control',
+            'must-revalidate' );
     }
 }
 
@@ -118,6 +122,10 @@ Will add "Cache-Control: must-revalidate" to the headers.
 
 Will add "Cache-Control: must-revalidate, max-age=3600" to the headers.
 
+=item check_last_modified_header
+
+Will not add an ETag if there is already a Last-Modified header.
+
 =back
 
 =head1 AUTHOR
diff --git a/t/02_last_modified.t b/t/02_last_modified.t
new file mode 100644
index 0000000..d14998b
--- /dev/null
+++ b/t/02_last_modified.t
@@ -0,0 +1,60 @@
+use strict;
+use warnings;
+use Test::More;
+
+use Digest::SHA;
+
+use Plack::Test;
+use Plack::Builder;
+use HTTP::Request::Common;
+
+my $content = [qw/hello world/];
+my $sha     = Digest::SHA->new->add(@$content)->hexdigest;
+
+my $app = sub {
+    [   '200',
+        [   'Content-Type'  => 'text/html',
+            'Last-Modified' => 'Wed, 07 Apr 2010 15:07:04 GMT'
+        ],
+        $content
+    ];
+};
+
+my $handler = builder {
+    enable "Plack::Middleware::ETag";
+    $app;
+};
+
+my $handler_with_last_mod = builder {
+    enable "Plack::Middleware::ETag", check_last_modified_header => 1;
+    $app;
+};
+
+# Don't break backwards compat
+test_psgi
+    app    => $handler,
+    client => sub {
+    my $cb = shift;
+    {
+        my $req = GET "http://localhost/";
+        my $res = $cb->($req);
+        ok $res->header('ETag');
+        ok $res->header('Last-Modified');
+        is $res->header('ETag'), $sha;
+    }
+    };
+
+# With check_last_modified_header there should be no etag set
+test_psgi
+    app    => $handler_with_last_mod,
+    client => sub {
+    my $cb = shift;
+    {
+        my $req = GET "http://localhost/";
+        my $res = $cb->($req);
+        ok !$res->header('ETag');
+        ok $res->header('Last-Modified');
+    }
+    };
+
+done_testing;