Skip to content

Commit 7c75181

Browse files
committed
CachedClient: header 'Last-Modified' has higher priority than 'ETag'
Some GitHub responses contain static ETag 'nil' and they control caching by timestamps.
1 parent e110135 commit 7c75181

2 files changed

Lines changed: 32 additions & 3 deletions

File tree

src/Github/Http/CachedClient.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ public function request(Request $request)
7575
}
7676

7777
/** @var $cached Response */
78-
if ($cached->hasHeader('ETag')) {
79-
$request->addHeader('If-None-Match', $cached->getHeader('ETag'));
80-
} elseif ($cached->hasHeader('Last-Modified')) {
78+
if ($cached->hasHeader('Last-Modified')) {
8179
$request->addHeader('If-Modified-Since', $cached->getHeader('Last-Modified'));
80+
} elseif ($cached->hasHeader('ETag')) {
81+
$request->addHeader('If-None-Match', $cached->getHeader('ETag'));
8282
}
8383
}
8484

tests/Github/Http/CachedClient.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,35 @@ class CachingTestCase extends Tester\TestCase
184184
}
185185

186186

187+
public function testPreferIfModifiedAgainstETag()
188+
{
189+
$this->innerClient->onRequest = function (Http\Request $request) {
190+
Assert::false($request->hasHeader('If-None-Match'));
191+
Assert::false($request->hasHeader('If-Modified-Since'));
192+
193+
return new Http\Response(200, ['Last-Modified' => 'today', 'ETag' => 'e-tag'], "response-{$request->getContent()}");
194+
};
195+
196+
$response = $this->client->request(new Http\Request('', '', [], '1'));
197+
Assert::same('response-1', $response->getContent());
198+
Assert::same(1, $this->innerClient->requestCount);
199+
200+
201+
$this->innerClient->onRequest = function (Http\Request $request) {
202+
Assert::false($request->hasHeader('ETag'));
203+
Assert::same('today', $request->getHeader('If-Modified-Since'));
204+
205+
return new Http\Response(304, [], "response-{$request->getContent()}");
206+
};
207+
208+
$response = $this->client->request(new Http\Request('', '', [], '2'));
209+
Assert::same('response-1', $response->getContent());
210+
Assert::type('Milo\Github\Http\Response', $response->getPrevious());
211+
Assert::same(304, $response->getPrevious()->getCode());
212+
Assert::same(2, $this->innerClient->requestCount);
213+
}
214+
215+
187216
public function testRepeatedRequest()
188217
{
189218
$request = new Http\Request('', '', [], 'same');

0 commit comments

Comments
 (0)