From f7b39e13a2890212e277444a1dc8484838888fd4 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 19 May 2020 23:04:25 +0200 Subject: [PATCH 1/2] Cache appstore requests for 60 instead of 5 minutes Signed-off-by: Morris Jobke --- lib/private/App/AppStore/Fetcher/Fetcher.php | 4 ++-- tests/lib/App/AppStore/Fetcher/FetcherBase.php | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index 224ed7d1f0d33..d6c882cf7de0b 100644 --- a/lib/private/App/AppStore/Fetcher/Fetcher.php +++ b/lib/private/App/AppStore/Fetcher/Fetcher.php @@ -40,7 +40,7 @@ use OCP\Util; abstract class Fetcher { - const INVALIDATE_AFTER_SECONDS = 300; + const INVALIDATE_AFTER_SECONDS = 3600; /** @var IAppData */ protected $appData; @@ -152,7 +152,7 @@ public function get() { // No caching when the version has been updated if (isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->getVersion()) { - // If the timestamp is older than 300 seconds request the files new + // If the timestamp is older than 3600 seconds request the files new if ((int)$jsonBlob['timestamp'] > ($this->timeFactory->getTime() - self::INVALIDATE_AFTER_SECONDS)) { return $jsonBlob['data']; } diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index 9dac4dd749928..758d917fac470 100644 --- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php +++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php @@ -239,7 +239,7 @@ public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() { $this->timeFactory ->expects($this->at(0)) ->method('getTime') - ->willReturn(1501); + ->willReturn(4801); $client = $this->createMock(IClient::class); $this->clientService ->expects($this->once()) @@ -533,11 +533,11 @@ public function testGetMatchingETag() { $this->timeFactory ->expects($this->at(0)) ->method('getTime') - ->willReturn(1501); + ->willReturn(4801); $this->timeFactory ->expects($this->at(1)) ->method('getTime') - ->willReturn(1502); + ->willReturn(4802); $client = $this->createMock(IClient::class); $this->clientService ->expects($this->once()) @@ -559,7 +559,7 @@ public function testGetMatchingETag() { $response->method('getStatusCode') ->willReturn(304); - $newData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}'; + $newData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":4802,"ncversion":"11.0.0.2","ETag":"\"myETag\""}'; $file ->expects($this->at(1)) ->method('putContent') @@ -638,7 +638,7 @@ public function testGetNoMatchingETag() { $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\""}'; + $fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":4802,"ncversion":"11.0.0.2","ETag":"\"newETag\""}'; $file ->expects($this->at(1)) ->method('putContent') @@ -650,11 +650,11 @@ public function testGetNoMatchingETag() { $this->timeFactory ->expects($this->at(0)) ->method('getTime') - ->willReturn(1501); + ->willReturn(4801); $this->timeFactory ->expects($this->at(1)) ->method('getTime') - ->willReturn(1502); + ->willReturn(4802); $expected = [ [ From d26c5103e6d82f06dff35cf2412c719c139d47d0 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 19 May 2020 23:04:51 +0200 Subject: [PATCH 2/2] Compress the appstore requests by default In test it reduced the transfered data from 5 MB to 2 MB. This should reduce the load on the appstore significantly. Signed-off-by: Morris Jobke --- lib/private/App/AppStore/Fetcher/Fetcher.php | 5 +- .../lib/App/AppStore/Fetcher/FetcherBase.php | 47 +++++++++++++++++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index d6c882cf7de0b..2f4e8b047d9b7 100644 --- a/lib/private/App/AppStore/Fetcher/Fetcher.php +++ b/lib/private/App/AppStore/Fetcher/Fetcher.php @@ -97,12 +97,11 @@ protected function fetch($ETag, $content) { $options = [ 'timeout' => 10, + 'headers' => ['Accept-Encoding' => 'gzip'], ]; if ($ETag !== '') { - $options['headers'] = [ - 'If-None-Match' => $ETag, - ]; + $options['headers']['If-None-Match'] = $ETag; } $client = $this->clientService->newClient(); diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index 758d917fac470..36cfb9cf4ede0 100644 --- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php +++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php @@ -249,7 +249,15 @@ public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() { $client ->expects($this->once()) ->method('get') - ->with($this->endpoint) + ->with( + $this->equalTo($this->endpoint), + $this->equalTo([ + 'timeout' => 10, + 'headers' => [ + 'Accept-Encoding' => 'gzip', + ] + ]) + ) ->willReturn($response); $response ->expects($this->once()) @@ -342,7 +350,15 @@ public function testGetWithAlreadyExistingFileAndNoVersion() { $client ->expects($this->once()) ->method('get') - ->with($this->endpoint) + ->with( + $this->equalTo($this->endpoint), + $this->equalTo([ + 'timeout' => 10, + 'headers' => [ + 'Accept-Encoding' => 'gzip', + ] + ]) + ) ->willReturn($response); $response ->expects($this->once()) @@ -430,7 +446,15 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion() { $client ->expects($this->once()) ->method('get') - ->with($this->endpoint) + ->with( + $this->equalTo($this->endpoint), + $this->equalTo([ + 'timeout' => 10, + 'headers' => [ + 'Accept-Encoding' => 'gzip', + ] + ]) + ) ->willReturn($response); $response ->expects($this->once()) @@ -495,7 +519,15 @@ public function testGetWithExceptionInClient() { $client ->expects($this->once()) ->method('get') - ->with($this->endpoint) + ->with( + $this->equalTo($this->endpoint), + $this->equalTo([ + 'timeout' => 10, + 'headers' => [ + 'Accept-Encoding' => 'gzip', + ] + ]) + ) ->willThrowException(new \Exception()); $this->assertSame([], $this->fetcher->get()); @@ -552,7 +584,8 @@ public function testGetMatchingETag() { $this->equalTo([ 'timeout' => 10, 'headers' => [ - 'If-None-Match' => '"myETag"' + 'Accept-Encoding' => 'gzip', + 'If-None-Match' => '"myETag"', ] ]) )->willReturn($response); @@ -624,6 +657,7 @@ public function testGetNoMatchingETag() { $this->equalTo([ 'timeout' => 10, 'headers' => [ + 'Accept-Encoding' => 'gzip', 'If-None-Match' => '"myETag"', ] ]) @@ -710,6 +744,9 @@ public function testFetchAfterUpgradeNoETag() { $this->equalTo($this->endpoint), $this->equalTo([ 'timeout' => 10, + 'headers' => [ + 'Accept-Encoding' => 'gzip', + ], ]) ) ->willReturn($response);