Fix unit tests and improve appstore logic

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

4 files changed, 178 insertions, 9 deletions

diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php
index 7f38e72be2d..ab0e299f0a2 100644
--- a/lib/private/App/AppStore/Fetcher/Fetcher.php
+++ b/lib/private/App/AppStore/Fetcher/Fetcher.php
@@ -69,9 +69,7 @@ abstract class Fetcher {
 	 * @return array
 	 */
 	protected function fetch($ETag, $content) {
-
-		$systemConfig = \OC::$server->getSystemConfig();
-		$appstoreenabled = $systemConfig->getValue('appstoreenabled', true);
+		$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
 
 		if (!$appstoreenabled) {
 			return [];
@@ -111,8 +109,7 @@ abstract class Fetcher {
 	 * @return array
 	 */
 	public function get() {
-		$systemConfig = \OC::$server->getSystemConfig();
-		$appstoreenabled = $systemConfig->getValue('appstoreenabled', true);
+		$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
 
 		if (!$appstoreenabled) {
 			return [];
diff --git a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php
index 9d09898bb95..4a5222fa915 100644
--- a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php
+++ b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php
@@ -59,11 +59,10 @@ EOD;
 		$this->config = $this->createMock(IConfig::class);
 
 		$this->config
-			->expects($this->atLeastOnce())
+			->expects($this->at(0))
 			->method('getSystemValue')
 			->with('version')
 			->willReturn('11.0.0.2');
-
 		$this->fetcher = new AppFetcher(
 			$this->appData,
 			$this->clientService,
@@ -73,6 +72,27 @@ EOD;
 	}
 
 	public function testGetWithFilter() {
+		$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('version')
+			->willReturn('11.0.0.2');
+		$this->config
+			->expects($this->at(3))
+			->method('getSystemValue')
+			->with('version')
+			->willReturn('11.0.0.2');
+
 		$file = $this->createMock(ISimpleFile::class);
 		$folder = $this->createMock(ISimpleFolder::class);
 		$folder
@@ -1920,4 +1940,17 @@ EJL3BaQAQaASSsvFrcozYxrQG4VzEg==
 
 		$this->assertEquals($expected['data'], $this->fetcher->get());
 	}
+
+	public function testAppstoreDisabled() {
+		$this->config
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('appstoreenabled', true)
+			->willReturn(false);
+		$this->appData
+			->expects($this->never())
+			->method('getFolder');
+
+		$this->assertEquals([], $this->fetcher->get());
+	}
 }
diff --git a/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php b/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php
index 9955715bca4..27f33bed997 100644
--- a/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php
+++ b/tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php
@@ -36,4 +36,18 @@ class CategoryFetcherTest extends FetcherBase  {
 			$this->config
 		);
 	}
+
+	public function testAppstoreDisabled() {
+		$this->config
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('appstoreenabled', true)
+			->willReturn(false);
+		$this->appData
+			->expects($this->never())
+			->method('getFolder');
+
+		$this->assertEquals([], $this->fetcher->get());
+
+	}
 }
diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php
index 73fcbbaab6f..1cec5270000 100644
--- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php
+++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php
@@ -55,16 +55,22 @@ abstract class FetcherBase extends TestCase {
 		$this->clientService = $this->createMock(IClientService::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->config = $this->createMock(IConfig::class);
+	}
 
+	public function testGetWithAlreadyExistingFileAndUpToDateTimestampAndVersion() {
 		$this->config
+			->expects($this->at(0))
+			->method('getSystemValue')
+			->with('appstoreenabled', true)
+			->willReturn(true);
+		$this->config
+			->expects($this->at(1))
 			->method('getSystemValue')
 			->with(
 				$this->equalTo('version'),
 				$this->anything()
 			)->willReturn('11.0.0.2');
-	}
 
-	public function testGetWithAlreadyExistingFileAndUpToDateTimestampAndVersion() {
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -95,6 +101,24 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() {
+		$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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -157,6 +181,24 @@ 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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -222,6 +264,24 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetWithAlreadyExistingFileAndNoVersion() {
+		$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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -283,6 +343,24 @@ abstract class FetcherBase extends TestCase {
 	}
 
 	public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
+		$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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -343,6 +421,17 @@ 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);
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -378,6 +467,24 @@ 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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData
@@ -447,6 +554,24 @@ 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');
+
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
 		$this->appData