Merge pull request #4650 from nextcloud/appcache_on_updatev12.0.0beta1

Do not use caching for appstore fetchers after upgrade

3 files changed, 129 insertions, 78 deletions

diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php
index ab0e299f0a2..bde925b745f 100644
--- a/lib/private/App/AppStore/Fetcher/Fetcher.php
+++ b/lib/private/App/AppStore/Fetcher/Fetcher.php
@@ -125,19 +125,19 @@ 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 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'];
-				}
 
-				if (isset($jsonBlob['ETag'])) {
-					$ETag = $jsonBlob['ETag'];
-					$content = json_encode($jsonBlob['data']);
+				// No caching when the version has been updated
+				if (isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->config->getSystemValue('version', '0.0.0')) {
+
+					// If the timestamp is older than 300 seconds request the files new
+					if ((int)$jsonBlob['timestamp'] > ($this->timeFactory->getTime() - self::INVALIDATE_AFTER_SECONDS)) {
+						return $jsonBlob['data'];
+					}
+
+					if (isset($jsonBlob['ETag'])) {
+						$ETag = $jsonBlob['ETag'];
+						$content = json_encode($jsonBlob['data']);
+					}
 				}
 			}
 		} catch (NotFoundException $e) {
diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php
index 1cec5270000..96e4f3ae81a 100644
--- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php
+++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php
@@ -181,23 +181,16 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() {
-		$this->config
-			->expects($this->at(0))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(2))
-			->method('getSystemValue')
-			->with(
-				$this->equalTo('version'),
-				$this->anything()
-			)->willReturn('11.0.0.2');
+		$this->config->method('getSystemValue')
+			->willReturnCallback(function($key, $default) {
+				if ($key === 'appstoreenabled') {
+					return true;
+				} else if ($key === 'version') {
+					return '11.0.0.2';
+				} else {
+					return $default;
+				}
+			});
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -421,16 +414,14 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetWithExceptionInClient() {
-		$this->config
-			->expects($this->at(0))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
+		$this->config->method('getSystemValue')
+			->willReturnCallback(function($key, $default) {
+				if ($key === 'appstoreenabled') {
+					return true;
+				} else {
+					return $default;
+				}
+			});
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -448,10 +439,6 @@ abstract class FetcherBase extends TestCase {
 			->expects($this->at(0))
 			->method('getContent')
 			->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}}');
-		$this->timeFactory
-			->expects($this->at(0))
-			->method('getTime')
-			->willReturn(1501);
 		$client = $this->createMock(IClient::class);
 		$this->clientService
 			->expects($this->once())
@@ -467,23 +454,16 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetMatchingETag() {
-		$this->config
-			->expects($this->at(0))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(2))
-			->method('getSystemValue')
-			->with(
-				$this->equalTo('version'),
-				$this->anything()
-			)->willReturn('11.0.0.2');
+		$this->config->method('getSystemValue')
+			->willReturnCallback(function($key, $default) {
+				if ($key === 'appstoreenabled') {
+					return true;
+				} else if ($key === 'version') {
+					return '11.0.0.2';
+				} else {
+					return $default;
+				}
+			});
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -554,23 +534,16 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetNoMatchingETag() {
-		$this->config
-			->expects($this->at(0))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('appstoreenabled', true)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(2))
-			->method('getSystemValue')
-			->with(
-				$this->equalTo('version'),
-				$this->anything()
-			)->willReturn('11.0.0.2');
+		$this->config->method('getSystemValue')
+			->willReturnCallback(function($key, $default) {
+				if ($key === 'appstoreenabled') {
+					return true;
+				} else if ($key === 'version') {
+					return '11.0.0.2';
+				} else {
+					return $default;
+				}
+			});
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -644,4 +617,82 @@ abstract class FetcherBase extends TestCase {
 		];
 		$this->assertSame($expected, $this->fetcher->get());
 	}
+
+
+	public function testFetchAfterUpgradeNoETag() {
+		$this->config->method('getSystemValue')
+			->willReturnCallback(function($key, $default) {
+				if ($key === 'appstoreenabled') {
+					return true;
+				} else if ($key === 'version') {
+					return '11.0.0.3';
+				} else {
+					return $default;
+				}
+			});
+
+		$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([])
+			)
+			->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":1501,"ncversion":"11.0.0.3","ETag":"\"newETag\""}';
+		$file
+			->expects($this->at(1))
+			->method('putContent')
+			->with($fileData);
+		$file
+			->expects($this->at(2))
+			->method('getContent')
+			->willReturn($fileData);
+		$this->timeFactory
+			->expects($this->once())
+			->method('getTime')
+			->willReturn(1501);
+
+		$expected = [
+			[
+				'id' => 'MyNewApp',
+				'foo' => 'foo',
+			],
+			[
+				'id' => 'bar',
+			],
+		];
+		$this->assertSame($expected, $this->fetcher->get());
+	}
 }
diff --git a/version.php b/version.php
index c4a33654a8e..b7e8b1ac6d8 100644
--- a/version.php
+++ b/version.php
@@ -26,7 +26,7 @@
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.
 
-$OC_Version = array(12, 0, 0, 15);
+$OC_Version = array(12, 0, 0, 16);
 
 // The human readable string
 $OC_VersionString = '12.0 beta 1';