From 245501fb0c2aedd7b332a1c6978dac604992069c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 15 Dec 2016 22:04:03 +0100 Subject: [PATCH] Clear appstore cache on version upgrade * Add version to cached json * Compare version * Updated calls * Updated tests Signed-off-by: Roeland Jago Douma --- .../App/AppStore/Fetcher/AppFetcher.php | 15 +- .../App/AppStore/Fetcher/CategoryFetcher.php | 8 +- lib/private/App/AppStore/Fetcher/Fetcher.php | 17 ++- lib/private/Server.php | 3 +- settings/Application.php | 3 +- .../App/AppStore/Fetcher/AppFetcherTest.php | 1 + .../AppStore/Fetcher/CategoryFetcherTest.php | 3 +- .../lib/App/AppStore/Fetcher/FetcherBase.php | 134 +++++++++++++++++- 8 files changed, 159 insertions(+), 25 deletions(-) diff --git a/lib/private/App/AppStore/Fetcher/AppFetcher.php b/lib/private/App/AppStore/Fetcher/AppFetcher.php index b8efdef433..9ebc12dbc2 100644 --- a/lib/private/App/AppStore/Fetcher/AppFetcher.php +++ b/lib/private/App/AppStore/Fetcher/AppFetcher.php @@ -28,9 +28,6 @@ use OCP\Http\Client\IClientService; use OCP\IConfig; class AppFetcher extends Fetcher { - /** @var IConfig */ - private $config; - /** * @param IAppData $appData * @param IClientService $clientService @@ -44,11 +41,11 @@ class AppFetcher extends Fetcher { parent::__construct( $appData, $clientService, - $timeFactory + $timeFactory, + $config ); $this->fileName = 'apps.json'; - $this->config = $config; $versionArray = explode('.', $this->config->getSystemValue('version')); $this->endpointUrl = sprintf( @@ -65,12 +62,8 @@ class AppFetcher extends Fetcher { * @return array */ protected function fetch() { - $client = $this->clientService->newClient(); - $response = $client->get($this->endpointUrl); - $responseJson = []; - $responseJson['data'] = json_decode($response->getBody(), true); - $responseJson['timestamp'] = $this->timeFactory->getTime(); - $response = $responseJson; + /** @var mixed[] $response */ + $response = parent::fetch(); $ncVersion = $this->config->getSystemValue('version'); $ncMajorVersion = explode('.', $ncVersion)[0]; diff --git a/lib/private/App/AppStore/Fetcher/CategoryFetcher.php b/lib/private/App/AppStore/Fetcher/CategoryFetcher.php index 74201ec373..8b79259a66 100644 --- a/lib/private/App/AppStore/Fetcher/CategoryFetcher.php +++ b/lib/private/App/AppStore/Fetcher/CategoryFetcher.php @@ -24,20 +24,24 @@ namespace OC\App\AppStore\Fetcher; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Http\Client\IClientService; +use OCP\IConfig; class CategoryFetcher extends Fetcher { /** * @param IAppData $appData * @param IClientService $clientService * @param ITimeFactory $timeFactory + * @param IConfig $config */ public function __construct(IAppData $appData, IClientService $clientService, - ITimeFactory $timeFactory) { + ITimeFactory $timeFactory, + IConfig $config) { parent::__construct( $appData, $clientService, - $timeFactory + $timeFactory, + $config ); $this->fileName = 'categories.json'; $this->endpointUrl = 'https://apps.nextcloud.com/api/v1/categories.json'; diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index f82e1a253f..2067242e81 100644 --- a/lib/private/App/AppStore/Fetcher/Fetcher.php +++ b/lib/private/App/AppStore/Fetcher/Fetcher.php @@ -25,6 +25,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Http\Client\IClientService; +use OCP\IConfig; abstract class Fetcher { const INVALIDATE_AFTER_SECONDS = 300; @@ -35,6 +36,8 @@ abstract class Fetcher { protected $clientService; /** @var ITimeFactory */ protected $timeFactory; + /** @var IConfig */ + protected $config; /** @var string */ protected $fileName; /** @var string */ @@ -44,13 +47,16 @@ abstract class Fetcher { * @param IAppData $appData * @param IClientService $clientService * @param ITimeFactory $timeFactory + * @param IConfig $config */ public function __construct(IAppData $appData, IClientService $clientService, - ITimeFactory $timeFactory) { + ITimeFactory $timeFactory, + IConfig $config) { $this->appData = $appData; $this->clientService = $clientService; $this->timeFactory = $timeFactory; + $this->config = $config; } /** @@ -64,6 +70,7 @@ abstract class Fetcher { $responseJson = []; $responseJson['data'] = json_decode($response->getBody(), true); $responseJson['timestamp'] = $this->timeFactory->getTime(); + $responseJson['ncversion'] = $this->config->getSystemValue('version'); return $responseJson; } @@ -80,8 +87,12 @@ abstract class Fetcher { $file = $rootFolder->getFile($this->fileName); $jsonBlob = json_decode($file->getContent(), true); if(is_array($jsonBlob)) { - // If the timestamp is older than 300 seconds request the files new - if((int)$jsonBlob['timestamp'] > ($this->timeFactory->getTime() - self::INVALIDATE_AFTER_SECONDS)) { + /* + * If the timestamp is older than 300 seconds request the files new + * If the version changed (update!) also refresh + */ + if((int)$jsonBlob['timestamp'] > ($this->timeFactory->getTime() - self::INVALIDATE_AFTER_SECONDS) && + isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->config->getSystemValue('version', '0.0.0')) { return $jsonBlob['data']; } } diff --git a/lib/private/Server.php b/lib/private/Server.php index b2fa9691b1..969e5a25b9 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -360,7 +360,8 @@ class Server extends ServerContainer implements IServerContainer { return new CategoryFetcher( $this->getAppDataDir('appstore'), $this->getHTTPClientService(), - $this->query(TimeFactory::class) + $this->query(TimeFactory::class), + $this->getConfig() ); }); $this->registerService('UserCache', function ($c) { diff --git a/settings/Application.php b/settings/Application.php index d907cd666f..44747c2f68 100644 --- a/settings/Application.php +++ b/settings/Application.php @@ -106,7 +106,8 @@ class Application extends App { return new CategoryFetcher( $server->getAppDataDir('appstore'), $server->getHTTPClientService(), - $server->query(TimeFactory::class) + $server->query(TimeFactory::class), + $server->getConfig() ); }); } diff --git a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php index a9cd5a190d..3affab2dba 100644 --- a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php +++ b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php @@ -1883,6 +1883,7 @@ EJL3BaQAQaASSsvFrcozYxrQG4VzEg== ), ), 'timestamp' => 1234, + 'ncversion' => '11.0.0.2', ); $dataToPut = $expected; diff --git a/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php b/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php index db4354119a..9955715bca 100644 --- a/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php +++ b/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php @@ -32,7 +32,8 @@ class CategoryFetcherTest extends FetcherBase { $this->fetcher = new CategoryFetcher( $this->appData, $this->clientService, - $this->timeFactory + $this->timeFactory, + $this->config ); } } diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index 840655d78d..cb47d0e08a 100644 --- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php +++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php @@ -55,9 +55,16 @@ abstract class FetcherBase extends TestCase { $this->clientService = $this->createMock(IClientService::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->config = $this->createMock(IConfig::class); + + $this->config + ->method('getSystemValue') + ->with( + $this->equalTo('version'), + $this->anything() + )->willReturn('11.0.0.2'); } - public function testGetWithAlreadyExistingFileAndUpToDateTimestamp() { + public function testGetWithAlreadyExistingFileAndUpToDateTimestampAndVersion() { $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class); $this->appData @@ -73,7 +80,7 @@ abstract class FetcherBase extends TestCase { $file ->expects($this->once()) ->method('getContent') - ->willReturn('{"timestamp":1200,"data":[{"id":"MyApp"}]}'); + ->willReturn('{"timestamp":1200,"data":[{"id":"MyApp"}],"ncversion":"11.0.0.2"}'); $this->timeFactory ->expects($this->once()) ->method('getTime') @@ -87,7 +94,7 @@ abstract class FetcherBase extends TestCase { $this->assertSame($expected, $this->fetcher->get()); } - public function testGetWithNotExistingFileAndUpToDateTimestamp() { + public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() { $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class); $this->appData @@ -120,7 +127,7 @@ 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}'; + $fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2"}'; $file ->expects($this->at(0)) ->method('putContent') @@ -162,7 +169,7 @@ abstract class FetcherBase extends TestCase { $file ->expects($this->at(0)) ->method('getContent') - ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}}'); + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.2"}'); $this->timeFactory ->expects($this->at(0)) ->method('getTime') @@ -182,7 +189,7 @@ 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}'; + $fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2"}'; $file ->expects($this->at(1)) ->method('putContent') @@ -208,6 +215,121 @@ abstract class FetcherBase extends TestCase { $this->assertSame($expected, $this->fetcher->get()); } + public function testGetWithAlreadyExistingFileAndNoVersion() { + $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); + $file + ->expects($this->at(0)) + ->method('getContent') + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}'); + $this->timeFactory + ->expects($this->at(0)) + ->method('getTime') + ->willReturn(1201); + $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->endpoint) + ->willReturn($response); + $response + ->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"}'; + $file + ->expects($this->at(1)) + ->method('putContent') + ->with($fileData); + $file + ->expects($this->at(2)) + ->method('getContent') + ->willReturn($fileData); + + $expected = [ + [ + 'id' => 'MyNewApp', + 'foo' => 'foo', + ], + [ + 'id' => 'bar', + ], + ]; + $this->assertSame($expected, $this->fetcher->get()); + } + + public function testGetWithAlreadyExistingFileAndOutdatedVersion() { + $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); + $file + ->expects($this->at(0)) + ->method('getContent') + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.1"}'); + $this->timeFactory + ->method('getTime') + ->willReturn(1201); + $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->endpoint) + ->willReturn($response); + $response + ->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"}'; + $file + ->expects($this->at(1)) + ->method('putContent') + ->with($fileData); + $file + ->expects($this->at(2)) + ->method('getContent') + ->willReturn($fileData); + + $expected = [ + [ + 'id' => 'MyNewApp', + 'foo' => 'foo', + ], + [ + 'id' => 'bar', + ], + ]; + $this->assertSame($expected, $this->fetcher->get()); + } + public function testGetWithExceptionInClient() { $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class);