From 095463d5685daf1f00a20f0c5caccfa7ebbc62e7 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 May 2017 08:44:20 +0200 Subject: [PATCH 1/3] Do not do ETag caching when the version is updated Signed-off-by: Roeland Jago Douma --- lib/private/App/AppStore/Fetcher/Fetcher.php | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index ab0e299f0a..bde925b745 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) { From 762284ce939041b74d0a8fecac3600f7c419384c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 May 2017 08:59:53 +0200 Subject: [PATCH 2/3] Fix and update tests Signed-off-by: Roeland Jago Douma --- .../lib/App/AppStore/Fetcher/FetcherBase.php | 181 +++++++++++------- 1 file changed, 116 insertions(+), 65 deletions(-) diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index 1cec527000..96e4f3ae81 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()); + } } From aa4cebe7495d496c695a23ba7eed5d19d306db51 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 2 May 2017 09:00:09 +0200 Subject: [PATCH 3/3] Bump version Signed-off-by: Roeland Jago Douma --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index c4a33654a8..b7e8b1ac6d 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';