Add ETag validation to appstore requests

* If the ETag if present store it
* If a stored ETag is present then pass it along (with the original
response) to get
* Add tests
* Added files to classmap

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This commit is contained in:
Roeland Jago Douma 2016-12-22 09:46:10 +01:00 committed by Joas Schilling
parent bae5d4acfe
commit 89df653ec8
No known key found for this signature in database
GPG Key ID: E166FD8976B3BAC8
4 changed files with 204 additions and 11 deletions

View File

@ -59,11 +59,14 @@ class AppFetcher extends Fetcher {
/**
* Only returns the latest compatible app release in the releases array
*
* @param string $ETag
* @param string $content
*
* @return array
*/
protected function fetch() {
protected function fetch($ETag, $content) {
/** @var mixed[] $response */
$response = parent::fetch();
$response = parent::fetch($ETag, $content);
$ncVersion = $this->config->getSystemValue('version');
$ncMajorVersion = explode('.', $ncVersion)[0];

View File

@ -21,6 +21,7 @@
namespace OC\App\AppStore\Fetcher;
use OCP\AppFramework\Http;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
@ -62,15 +63,37 @@ abstract class Fetcher {
/**
* Fetches the response from the server
*
* @param string $ETag
* @param string $content
*
* @return array
*/
protected function fetch() {
protected function fetch($ETag, $content) {
$options = [];
if ($ETag !== '') {
$options['headers'] = [
'If-None-Match' => $ETag,
];
}
$client = $this->clientService->newClient();
$response = $client->get($this->endpointUrl);
$response = $client->get($this->endpointUrl, $options);
$responseJson = [];
$responseJson['data'] = json_decode($response->getBody(), true);
if ($response->getStatusCode() === Http::STATUS_NOT_MODIFIED) {
$responseJson['data'] = json_decode($content, true);
} else {
$responseJson['data'] = json_decode($response->getBody(), true);
$ETag = $response->getHeader('ETag');
}
$responseJson['timestamp'] = $this->timeFactory->getTime();
$responseJson['ncversion'] = $this->config->getSystemValue('version');
if ($ETag !== '') {
$responseJson['ETag'] = $ETag;
}
return $responseJson;
}
@ -82,6 +105,9 @@ abstract class Fetcher {
public function get() {
$rootFolder = $this->appData->getFolder('/');
$ETag = '';
$content = '';
try {
// File does already exists
$file = $rootFolder->getFile($this->fileName);
@ -95,6 +121,11 @@ abstract class Fetcher {
isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->config->getSystemValue('version', '0.0.0')) {
return $jsonBlob['data'];
}
if (isset($jsonBlob['ETag'])) {
$ETag = $jsonBlob['ETag'];
$content = json_encode($jsonBlob['data']);
}
}
} catch (NotFoundException $e) {
// File does not already exists
@ -103,7 +134,7 @@ abstract class Fetcher {
// Refresh the file content
try {
$responseJson = $this->fetch();
$responseJson = $this->fetch($ETag, $content);
$file->putContent(json_encode($responseJson));
return json_decode($file->getContent(), true)['data'];
} catch (\Exception $e) {

View File

@ -99,6 +99,9 @@ EOD;
->expects($this->once())
->method('getBody')
->willReturn(self::$responseJson);
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"myETag"');
$this->timeFactory
->expects($this->once())
->method('getTime')
@ -1883,6 +1886,7 @@ EJL3BaQAQaASSsvFrcozYxrQG4VzEg==
),
'timestamp' => 1234,
'ncversion' => '11.0.0.2',
'ETag' => '"myETag"',
);
$dataToPut = $expected;

View File

@ -127,7 +127,10 @@ abstract class FetcherBase extends TestCase {
->expects($this->once())
->method('getBody')
->willReturn('[{"id":"MyNewApp", "foo": "foo"}, {"id":"bar"}]');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2"}';
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"myETag"');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(0))
->method('putContent')
@ -189,7 +192,10 @@ abstract class FetcherBase extends TestCase {
->expects($this->once())
->method('getBody')
->willReturn('[{"id":"MyNewApp", "foo": "foo"}, {"id":"bar"}]');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2"}';
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"myETag"');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(1))
->method('putContent')
@ -251,7 +257,10 @@ abstract class FetcherBase extends TestCase {
->expects($this->once())
->method('getBody')
->willReturn('[{"id":"MyNewApp", "foo": "foo"}, {"id":"bar"}]');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1201,"ncversion":"11.0.0.2"}';
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"myETag"');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1201,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(1))
->method('putContent')
@ -289,7 +298,7 @@ abstract class FetcherBase extends TestCase {
$file
->expects($this->at(0))
->method('getContent')
->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.1"}');
->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.1"');
$this->timeFactory
->method('getTime')
->willReturn(1201);
@ -308,7 +317,10 @@ abstract class FetcherBase extends TestCase {
->expects($this->once())
->method('getBody')
->willReturn('[{"id":"MyNewApp", "foo": "foo"}, {"id":"bar"}]');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1201,"ncversion":"11.0.0.2"}';
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"myETag"');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1201,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(1))
->method('putContent')
@ -364,4 +376,147 @@ abstract class FetcherBase extends TestCase {
$this->assertSame([], $this->fetcher->get());
}
public function testGetMatchingETag() {
$folder = $this->createMock(ISimpleFolder::class);
$file = $this->createMock(ISimpleFile::class);
$this->appData
->expects($this->once())
->method('getFolder')
->with('/')
->willReturn($folder);
$folder
->expects($this->once())
->method('getFile')
->with($this->fileName)
->willReturn($file);
$origData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(0))
->method('getContent')
->willReturn($origData);
$this->timeFactory
->expects($this->at(0))
->method('getTime')
->willReturn(1501);
$this->timeFactory
->expects($this->at(1))
->method('getTime')
->willReturn(1502);
$client = $this->createMock(IClient::class);
$this->clientService
->expects($this->once())
->method('newClient')
->willReturn($client);
$response = $this->createMock(IResponse::class);
$client
->expects($this->once())
->method('get')
->with(
$this->equalTo($this->endpoint),
$this->equalTo([
'headers' => [
'If-None-Match' => '"myETag"'
]
])
)->willReturn($response);
$response->method('getStatusCode')
->willReturn(304);
$newData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
$file
->expects($this->at(1))
->method('putContent')
->with($newData);
$file
->expects($this->at(2))
->method('getContent')
->willReturn($newData);
$expected = [
[
'id' => 'MyNewApp',
'foo' => 'foo',
],
[
'id' => 'bar',
],
];
$this->assertSame($expected, $this->fetcher->get());
}
public function testGetNoMatchingETag() {
$folder = $this->createMock(ISimpleFolder::class);
$file = $this->createMock(ISimpleFile::class);
$this->appData
->expects($this->once())
->method('getFolder')
->with('/')
->willReturn($folder);
$folder
->expects($this->at(0))
->method('getFile')
->with($this->fileName)
->willReturn($file);
$file
->expects($this->at(0))
->method('getContent')
->willReturn('{"data":[{"id":"MyOldApp","abc":"def"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}');
$client = $this->createMock(IClient::class);
$this->clientService
->expects($this->once())
->method('newClient')
->willReturn($client);
$response = $this->createMock(IResponse::class);
$client
->expects($this->once())
->method('get')
->with(
$this->equalTo($this->endpoint),
$this->equalTo([
'headers' => [
'If-None-Match' => '"myETag"',
]
])
)
->willReturn($response);
$response->method('getStatusCode')
->willReturn(200);
$response
->expects($this->once())
->method('getBody')
->willReturn('[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}]');
$response->method('getHeader')
->with($this->equalTo('ETag'))
->willReturn('"newETag"');
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"newETag\""}';
$file
->expects($this->at(1))
->method('putContent')
->with($fileData);
$file
->expects($this->at(2))
->method('getContent')
->willReturn($fileData);
$this->timeFactory
->expects($this->at(0))
->method('getTime')
->willReturn(1501);
$this->timeFactory
->expects($this->at(1))
->method('getTime')
->willReturn(1502);
$expected = [
[
'id' => 'MyNewApp',
'foo' => 'foo',
],
[
'id' => 'bar',
],
];
$this->assertSame($expected, $this->fetcher->get());
}
}