Merge pull request #29256 from nextcloud/fix/noid/harmonize-appstoree…

…nabled-check Fix appstoreenabled check
nextcloud · Oct 21, 2021 · 0bef570 · 0bef570
2 parents c5405be + f24e821
commit 0bef570
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 46 deletions.
diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php
@@ -131,7 +131,7 @@ public function __construct(string $appName,
 	public function viewApps(): TemplateResponse {
 		\OC_Util::addScript('settings', 'apps');
 		$params = [];
-		$params['appstoreEnabled'] = $this->config->getSystemValue('appstoreenabled', true) === true;
+		$params['appstoreEnabled'] = $this->config->getSystemValueBool('appstoreenabled', true);
 		$params['updateCount'] = count($this->getAppsWithUpdates());
 		$params['developerDocumentation'] = $this->urlGenerator->linkToDocs('developer-manual');
 		$params['bundles'] = $this->getBundles();

diff --git a/apps/settings/tests/Controller/AppSettingsControllerTest.php b/apps/settings/tests/Controller/AppSettingsControllerTest.php
@@ -190,7 +190,7 @@ public function testViewApps() {
 			->willReturn(false);
 		$this->config
 			->expects($this->once())
-			->method('getSystemValue')
+			->method('getSystemValueBool')
 			->with('appstoreenabled', true)
 			->willReturn(true);
 		$this->navigationManager
@@ -224,7 +224,7 @@ public function testViewAppsAppstoreNotEnabled() {
 		$this->bundleFetcher->expects($this->once())->method('getBundles')->willReturn([]);
 		$this->config
 			->expects($this->once())
-			->method('getSystemValue')
+			->method('getSystemValueBool')
 			->with('appstoreenabled', true)
 			->willReturn(false);
 		$this->navigationManager

diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php
@@ -90,7 +90,7 @@ public function __construct(Factory $appDataFactory,
 	 * @return array
 	 */
 	protected function fetch($ETag, $content) {
-		$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
+		$appstoreenabled = $this->config->getSystemValueBool('appstoreenabled', true);
 		if ((int)$this->config->getAppValue('settings', 'appstore-fetcher-lastFailure', '0') > time() - self::RETRY_AFTER_FAILURE_SECONDS) {
 			return [];
 		}
@@ -148,7 +148,7 @@ protected function fetch($ETag, $content) {
 	 * @return array
 	 */
 	public function get($allowUnstable = false) {
-		$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
+		$appstoreenabled = $this->config->getSystemValueBool('appstoreenabled', true);
 		$internetavailable = $this->config->getSystemValue('has_internet_connection', true);
 
 		if (!$appstoreenabled || !$internetavailable) {

diff --git a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php
@@ -1868,16 +1868,18 @@ protected function setUp(): void {
 	public function testGetWithFilter() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.2';
 				} elseif ($key === 'appstoreurl' && $default === 'https://apps.nextcloud.com/api/v1') {
 					return 'https://custom.appsstore.endpoint/api/v1';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$file = $this->createMock(ISimpleFile::class);
 		$folder = $this->createMock(ISimpleFolder::class);
@@ -1951,13 +1953,15 @@ public function testAppstoreDisabled() {
 		$this->config
 			->method('getSystemValue')
 			->willReturnCallback(function ($var, $default) {
-				if ($var === 'appstoreenabled') {
-					return false;
-				} elseif ($var === 'version') {
+				if ($var === 'version') {
 					return '11.0.0.2';
 				}
 				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(false);
 		$this->appData
 			->expects($this->never())
 			->method('getFolder');
@@ -1977,6 +1981,10 @@ public function testNoInternet() {
 				}
 				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 		$this->appData
 			->expects($this->never())
 			->method('getFolder');
@@ -1987,16 +1995,18 @@ public function testNoInternet() {
 	public function testSetVersion() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '10.0.7.2';
 				} elseif ($key === 'appstoreurl' && $default === 'https://apps.nextcloud.com/api/v1') {
 					return 'https://custom.appsstore.endpoint/api/v1';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$file = $this->createMock(ISimpleFile::class);
 		$folder = $this->createMock(ISimpleFolder::class);
@@ -2070,9 +2080,7 @@ public function testSetVersion() {
 	public function testGetAppsAllowlist() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.2';
 				} elseif ($key === 'appstoreurl' && $default === 'https://apps.nextcloud.com/api/v1') {
 					return 'https://custom.appsstore.endpoint/api/v1';
@@ -2082,7 +2090,11 @@ public function testGetAppsAllowlist() {
 					return $default;
 				}
 			});
-
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
+
 		$file = $this->createMock(ISimpleFile::class);
 		$folder = $this->createMock(ISimpleFolder::class);
 		$folder
@@ -2122,7 +2134,7 @@ public function testGetAppsAllowlist() {
 			->expects($this->once())
 			->method('getTime')
 			->willReturn(1234);
-			
+
 		$this->registry
 			->expects($this->exactly(2))
 			->method('delegateHasValidSubscription')

diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php
@@ -77,7 +77,7 @@ protected function setUp(): void {
 	public function testGetWithAlreadyExistingFileAndUpToDateTimestampAndVersion() {
 		$this->config
 			->expects($this->at(0))
-			->method('getSystemValue')
+			->method('getSystemValueBool')
 			->with('appstoreenabled', true)
 			->willReturn(true);
 		$this->config
@@ -126,9 +126,7 @@ public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() {
 		$this->config
 			->method('getSystemValue')
 			->willReturnCallback(function ($var, $default) {
-				if ($var === 'appstoreenabled') {
-					return true;
-				} elseif ($var === 'has_internet_connection') {
+				if ($var === 'has_internet_connection') {
 					return true;
 				} elseif ($var === 'appstoreurl') {
 					return 'https://apps.nextcloud.com/api/v1';
@@ -137,6 +135,10 @@ public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() {
 				}
 				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', $this->anything())
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -202,14 +204,16 @@ public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() {
 	public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.2';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -279,9 +283,7 @@ public function testGetWithAlreadyExistingFileAndNoVersion() {
 		$this->config
 			->method('getSystemValue')
 			->willReturnCallback(function ($var, $default) {
-				if ($var === 'appstoreenabled') {
-					return true;
-				} elseif ($var === 'has_internet_connection') {
+				if ($var === 'has_internet_connection') {
 					return true;
 				} elseif ($var === 'appstoreurl') {
 					return 'https://apps.nextcloud.com/api/v1';
@@ -290,6 +292,10 @@ public function testGetWithAlreadyExistingFileAndNoVersion() {
 				}
 				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -355,9 +361,7 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
 		$this->config
 			->method('getSystemValue')
 			->willReturnCallback(function ($var, $default) {
-				if ($var === 'appstoreenabled') {
-					return true;
-				} elseif ($var === 'has_internet_connection') {
+				if ($var === 'has_internet_connection') {
 					return true;
 				} elseif ($var === 'appstoreurl') {
 					return 'https://apps.nextcloud.com/api/v1';
@@ -366,6 +370,10 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
 				}
 				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -429,12 +437,12 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
 	public function testGetWithExceptionInClient() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} else {
-					return $default;
-				}
+				return $default;
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -469,14 +477,16 @@ public function testGetWithExceptionInClient() {
 	public function testGetMatchingETag() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.2';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -550,14 +560,16 @@ public function testGetMatchingETag() {
 	public function testGetNoMatchingETag() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.2';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -637,14 +649,16 @@ public function testGetNoMatchingETag() {
 	public function testFetchAfterUpgradeNoETag() {
 		$this->config->method('getSystemValue')
 			->willReturnCallback(function ($key, $default) {
-				if ($key === 'appstoreenabled') {
-					return true;
-				} elseif ($key === 'version') {
+				if ($key === 'version') {
 					return '11.0.0.3';
 				} else {
 					return $default;
 				}
 			});
+		$this->config
+			->method('getSystemValueBool')
+			->with('appstoreenabled', true)
+			->willReturn(true);
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);