Merge pull request #2707 from nextcloud/clear_appstore_cache_on_upgrade

Clear appstore cache on version upgrade

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 b8efdef4336..9ebc12dbc27 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 74201ec3737..8b79259a66a 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 f82e1a253f6..2067242e817 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 b2fa9691b16..969e5a25b9b 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 d907cd666fb..44747c2f681 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 a9cd5a190d0..3affab2dbaa 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 db4354119a0..9955715bca4 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 840655d78d5..cb47d0e08ac 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);