Skip to content

Commit

Permalink
support 304 Not Modified responses
Browse files Browse the repository at this point in the history
  • Loading branch information
@sekimura
sekimura committed Oct 7, 2011
1 parent 5ed24aa commit aa33a68
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 6 deletions.
15 changes: 14 additions & 1 deletion lib/LWP/UserAgent/WithCache.pm
View file
Expand Up @@ -65,7 +65,20 @@ sub request {
}

my $res = $self->SUPER::request(@args);
$self->set_cache($uri, $res) if $res->code eq HTTP::Status::RC_OK;

## return cached data if it is "Not Modified"
if ($res->code eq HTTP::Status::RC_NOT_MODIFIED) {
my $not_modified_res = HTTP::Response->parse($obj->{as_string});
# hrm.. should we use '200 OK' here?
$not_modified_res->code(HTTP::Status::RC_NOT_MODIFIED);
$not_modified_res->message(HTTP::Status::status_message(HTTP::Status::RC_NOT_MODIFIED));

This comment has been minimized.

Sign in to view

Copy link
@ctcherry

ctcherry Oct 7, 2011

Contributor

I see how this would make sense, because from the real site you are requesting, the real response code is 304, but...

Since the idea is to return the original response body content from the cache, I preferred keeping the original status code (in this case its mostly going to be 200, maybe consider caching other ones as well?). Then any code that uses the library down stream can be ignorant to the caching, in theory.

This comment has been minimized.

Sign in to view

Copy link
@sekimura

sekimura Oct 7, 2011

Author Owner

ok. I'll make a change to keep original response code and message. thx!
return $not_modified_res;
}

## cache only "200 OK" content
if ($res->code eq HTTP::Status::RC_OK) {
$self->set_cache($uri, $res);
}

return $res;
}
Expand Down
50 changes: 45 additions & 5 deletions t/10_request.t
View file
@@ -1,4 +1,4 @@
use Test::More tests => 2;
use Test::More tests => 4;

use File::Temp qw/ tempfile tempdir /;
use LWP::UserAgent::WithCache;
Expand Down Expand Up @@ -32,9 +32,9 @@ EOF

my $uri = 'http://www.example.com/styles.css';
$ua->set_cache($uri, $res);
$res = $ua->get('http://www.example.com/styles.css');
my $cached_res = $ua->get('http://www.example.com/styles.css');

is ($res->code, 200);
is ($cached_res->code, 200);
}

# haven't expired yet
Expand All @@ -59,7 +59,47 @@ EOF

my $uri = 'http://www.example.com/styles.css';
$ua->set_cache($uri, $res);
$res = $ua->get('http://www.example.com/styles.css');
my $cached_res = $ua->get('http://www.example.com/styles.css');

is ($res->code, 200);
is ($cached_res->code, 200);
}

# handle 304 Not Modified response
{
$res = HTTP::Response->parse(<<'EOF');
HTTP/1.1 200 OK
Connection: close
Server: nginx/1.0.4
Content-Length: 237
Content-Type: text/css
Date:Fri, 07 Oct 2011 22:43:34 GMT
Last-Modified: Thr, 01 Jan 1970 00:00:00 GMT
/* This is the StyleCatcher theme addition. Do not remove this block. */
/* Selected Layout: */
@import url(base_theme.css);
@import url(http://mt.qootas.org/mt/mt-static/themes/minimalist-red/screen.css);
/* end StyleCatcher imports */
EOF

my $not_modified_res = HTTP::Response->parse(<<'EOF');
HTTP/1.1 304 Not Modified
Connection: close
Server: nginx/1.0.4
Date:Fri, 07 Oct 2011 22:43:34 GMT
Last-Modified: Thr, 01 Jan 1970 00:00:00 GMT
EOF

my $uri = 'http://www.example.com/styles.css';
$ua->set_cache($uri, $res);

## override request method to get not_modified_res
no warnings 'redefine';
local *LWP::UserAgent::request = sub {return $not_modified_res};

my $cached_res = $ua->get('http://www.example.com/styles.css');

is ($cached_res->code, 304);
is ($cached_res->content, $res->content);
}

0 comments on commit aa33a68

Please sign in to comment.