From 5ac8af27dcb1651a369838e82be8447ef6fa70d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 30 May 2018 09:38:27 +0200 Subject: [PATCH] Cleanup controller code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- settings/Controller/AppSettingsController.php | 336 ++++++++---------- .../Controller/AppSettingsControllerTest.php | 59 ++- 2 files changed, 174 insertions(+), 221 deletions(-) diff --git a/settings/Controller/AppSettingsController.php b/settings/Controller/AppSettingsController.php index b9ae26083b7..0fce6e07ded 100644 --- a/settings/Controller/AppSettingsController.php +++ b/settings/Controller/AppSettingsController.php @@ -152,6 +152,18 @@ class AppSettingsController extends Controller { return $templateResponse; } + private function getAppsWithUpdates() { + $appClass = new \OC_App(); + $apps = $appClass->listAllApps(); + foreach($apps as $key => $app) { + $newVersion = $this->installer->isUpdateAvailable($app['id']); + if($newVersion === false) { + unset($apps[$key]); + } + } + return $apps; + } + private function getBundles() { $result = []; $bundles = $this->bundleFetcher->getBundles(); @@ -166,6 +178,15 @@ class AppSettingsController extends Controller { } + /** + * Get all available categories + * + * @return JSONResponse + */ + public function listCategories(): JSONResponse { + return new JSONResponse($this->getAllCategories()); + } + private function getAllCategories() { $currentLanguage = substr($this->l10nFactory->findLanguage(), 0, 2); @@ -183,12 +204,114 @@ class AppSettingsController extends Controller { } /** - * Get all available categories + * Get all available apps in a category * + * @param string $category * @return JSONResponse + * @throws \Exception */ - public function listCategories(): JSONResponse { - return new JSONResponse($this->getAllCategories()); + public function listApps($category = ''): JSONResponse { + $appClass = new \OC_App(); + + switch ($category) { + case 'installed': + $apps = $appClass->listAllApps(); + break; + case 'updates': + $apps = $this->getAppsWithUpdates(); + break; + case 'enabled': + $apps = $appClass->listAllApps(); + $apps = array_filter($apps, function ($app) { + return $app['active']; + }); + break; + case 'disabled': + $apps = $appClass->listAllApps(); + $apps = array_filter($apps, function ($app) { + return !$app['active']; + }); + break; + case 'app-bundles': + $bundles = $this->bundleFetcher->getBundles(); + $apps = []; + $installedApps = $appClass->listAllApps(); + $appstoreApps = $this->getAppsForCategory(); + foreach($bundles as $bundle) { + foreach($bundle->getAppIdentifiers() as $identifier) { + $alreadyMatched = false; + foreach($installedApps as $app) { + if($app['id'] === $identifier) { + $app['bundleId'] = $bundle->getIdentifier(); + $apps[] = $app; + $alreadyMatched = true; + continue; + } + } + if (!$alreadyMatched) { + foreach ($appstoreApps as $app) { + if ($app['id'] === $identifier) { + $app['bundleId'] = $bundle->getIdentifier(); + $apps[] = $app; + continue; + } + } + } + } + } + break; + default: + $apps = $this->getAppsForCategory($category); + break; + } + + + // Fetch all apps from appstore + $appstoreData = []; + $fetchedApps = $this->appFetcher->get(); + foreach ($fetchedApps as $app) { + $appstoreData[$app['id']] = $app; + } + + $dependencyAnalyzer = new DependencyAnalyzer(new Platform($this->config), $this->l10n); + + // Extend existing app details + $apps = array_map(function($appData) use ($appstoreData, $dependencyAnalyzer) { + $appData['appstoreData'] = $appstoreData[$appData['id']]; + $appData['license'] = $appstoreData['releases'][0]['licenses']; + $appData['screenshot'] = isset($appstoreData['screenshots'][0]['url']) ? 'https://usercontent.apps.nextcloud.com/'.base64_encode($appstoreData['screenshots'][0]['url']) : ''; + $newVersion = $this->installer->isUpdateAvailable($appData['id']); + if($newVersion && $this->appManager->isInstalled($appData['id'])) { + $appData['update'] = $newVersion; + } + + // fix groups to be an array + $groups = array(); + if (is_string($appData['groups'])) { + $groups = json_decode($appData['groups']); + } + $appData['groups'] = $groups; + $appData['canUnInstall'] = !$appData['active'] && $appData['removable']; + + // fix licence vs license + if (isset($appData['license']) && !isset($appData['licence'])) { + $appData['licence'] = $appData['license']; + } + + // analyse dependencies + $missing = $dependencyAnalyzer->analyze($appData); + $appData['canInstall'] = empty($missing); + $appData['missingDependencies'] = $missing; + + $appData['missingMinOwnCloudVersion'] = !isset($appData['dependencies']['nextcloud']['@attributes']['min-version']); + $appData['missingMaxOwnCloudVersion'] = !isset($appData['dependencies']['nextcloud']['@attributes']['max-version']); + + return $appData; + }, $apps); + + usort($apps, [$this, 'sortApps']); + + return new JSONResponse(['apps' => $apps, 'status' => 'success']); } /** @@ -203,10 +326,6 @@ class AppSettingsController extends Controller { $formattedApps = []; $apps = $this->appFetcher->get(); foreach($apps as $app) { - if (isset($app['isFeatured'])) { - $app['featured'] = $app['isFeatured']; - } - // Skip all apps not in the requested category if ($requestedCategory !== '') { $isInCategory = false; @@ -285,202 +404,26 @@ class AppSettingsController extends Controller { $nextCloudVersionDependencies, $phpDependencies ), - 'level' => ($app['featured'] === true) ? 200 : 100, + 'level' => ($app['isFeatured'] === true) ? 200 : 100, 'missingMaxOwnCloudVersion' => false, 'missingMinOwnCloudVersion' => false, 'canInstall' => true, 'preview' => isset($app['screenshots'][0]['url']) ? 'https://usercontent.apps.nextcloud.com/'.base64_encode($app['screenshots'][0]['url']) : '', 'score' => $app['ratingOverall'], 'ratingNumOverall' => $app['ratingNumOverall'], - 'ratingNumThresholdReached' => $app['ratingNumOverall'] > 5 ? true : false, + 'ratingNumThresholdReached' => $app['ratingNumOverall'] > 5, 'removable' => $existsLocally, 'active' => $this->appManager->isEnabledForUser($app['id']), 'needsDownload' => !$existsLocally, 'groups' => $groups, 'fromAppStore' => true, - 'appstoreData' => $app + 'appstoreData' => $app, ]; - - - $newVersion = $this->installer->isUpdateAvailable($app['id']); - if($newVersion && $this->appManager->isInstalled($app['id'])) { - $formattedApps[count($formattedApps)-1]['update'] = $newVersion; - } } return $formattedApps; } - private function getAppsWithUpdates() { - $appClass = new \OC_App(); - $apps = $appClass->listAllApps(); - /** @var \OC\App\AppStore\Manager $manager */ - $manager = \OC::$server->query(\OC\App\AppStore\Manager::class); - foreach($apps as $key => $app) { - $newVersion = $this->installer->isUpdateAvailable($app['id']); - if($newVersion !== false) { - $apps[$key]['update'] = $newVersion; - $apps[$key]['appstoreData'] = $manager->getApp($app['id']); - } else { - unset($apps[$key]); - } - } - usort($apps, [$this, 'sortApps']); - return $apps; - } - - private function sortApps($a, $b) { - $a = (string)$a['name']; - $b = (string)$b['name']; - if ($a === $b) { - return 0; - } - return ($a < $b) ? -1 : 1; - } - - /** - * Get all available apps in a category - * - * @param string $category - * @return JSONResponse - * @throws \Exception - */ - public function listApps($category = '') { - $appClass = new \OC_App(); - $manager = \OC::$server->query(\OC\App\AppStore\Manager::class); - - switch ($category) { - // installed apps - case 'installed': - $apps = $appClass->listAllApps(); - foreach($apps as $key => $app) { - $newVersion = $this->installer->isUpdateAvailable($app['id']); - $apps[$key]['update'] = $newVersion; - } - - usort($apps, [$this, 'sortApps']); - break; - // updates - case 'updates': - $apps = $this->getAppsWithUpdates(); - break; - // enabled apps - case 'enabled': - $apps = $appClass->listAllApps(); - $apps = array_filter($apps, function ($app) { - return $app['active']; - }); - - foreach($apps as $key => $app) { - $newVersion = $this->installer->isUpdateAvailable($app['id']); - $apps[$key]['update'] = $newVersion; - } - - usort($apps, [$this, 'sortApps']); - break; - // disabled apps - case 'disabled': - $apps = $appClass->listAllApps(); - $apps = array_filter($apps, function ($app) { - return !$app['active']; - }); - - $apps = array_map(function ($app) { - $newVersion = $this->installer->isUpdateAvailable($app['id']); - if ($newVersion !== false) { - $app['update'] = $newVersion; - } - return $app; - }, $apps); - - usort($apps, [$this, 'sortApps']); - break; - case 'app-bundles': - $bundles = $this->bundleFetcher->getBundles(); - $apps = []; - foreach($bundles as $bundle) { - $newCategory = true; - $allApps = $appClass->listAllApps(); - - $newApps = $this->getAppsForCategory(); - foreach($allApps as $app) { - foreach($newApps as $key => $newApp) { - if($app['id'] === $newApp['id']) { - unset($newApps[$key]); - } - } - } - $allApps = array_merge($allApps, $newApps); - - - foreach($bundle->getAppIdentifiers() as $identifier) { - foreach($allApps as $app) { - if($app['id'] === $identifier) { - $app['bundleId'] = $bundle->getIdentifier(); - $apps[] = $app; - continue; - } - } - } - } - break; - default: - $apps = $this->getAppsForCategory($category); - break; - } - - // fix groups to be an array - $dependencyAnalyzer = new DependencyAnalyzer(new Platform($this->config), $this->l10n); - $apps = array_map(function($app) use ($dependencyAnalyzer) { - - // fix groups - $groups = array(); - if (is_string($app['groups'])) { - $groups = json_decode($app['groups']); - } - $app['groups'] = $groups; - $app['canUnInstall'] = !$app['active'] && $app['removable']; - - // fix licence vs license - if (isset($app['license']) && !isset($app['licence'])) { - $app['licence'] = $app['license']; - } - - // analyse dependencies - $missing = $dependencyAnalyzer->analyze($app); - $app['canInstall'] = empty($missing); - $app['missingDependencies'] = $missing; - - $app['missingMinOwnCloudVersion'] = !isset($app['dependencies']['nextcloud']['@attributes']['min-version']); - $app['missingMaxOwnCloudVersion'] = !isset($app['dependencies']['nextcloud']['@attributes']['max-version']); - - return $app; - }, $apps); - - // Add app store dump for app to data - $apps = array_map(function($appData) use ($manager) { - $appStoreData = $manager->getApp($appData['id']); - $appData['appstoreData'] = $appStoreData; - $appData['license'] = $appStoreData['releases'][0]['licenses']; - $appData['screenshot'] = isset($appStoreData['screenshots'][0]['url']) ? 'https://usercontent.apps.nextcloud.com/'.base64_encode($appStoreData['screenshots'][0]['url']) : ''; - return $appData; - }, $apps); - - return new JSONResponse(['apps' => $apps, 'status' => 'success']); - } - - private function getGroupList(array $groups) { - $groupManager = \OC::$server->getGroupManager(); - $groupsList = []; - foreach ($groups as $group) { - $groupItem = $groupManager->get($group); - if ($groupItem instanceof \OCP\IGroup) { - $groupsList[] = $groupManager->get($group); - } - } - return $groupsList; - } - /** * @PasswordConfirmationRequired * @@ -537,6 +480,18 @@ class AppSettingsController extends Controller { } } + private function getGroupList(array $groups) { + $groupManager = \OC::$server->getGroupManager(); + $groupsList = []; + foreach ($groups as $group) { + $groupItem = $groupManager->get($group); + if ($groupItem instanceof \OCP\IGroup) { + $groupsList[] = $groupManager->get($group); + } + } + return $groupsList; + } + /** * @PasswordConfirmationRequired * @@ -606,4 +561,13 @@ class AppSettingsController extends Controller { return new JSONResponse(['data' => ['message' => $this->l10n->t('Couldn\'t update app.')]], Http::STATUS_INTERNAL_SERVER_ERROR); } + private function sortApps($a, $b) { + $a = (string)$a['name']; + $b = (string)$b['name']; + if ($a === $b) { + return 0; + } + return ($a < $b) ? -1 : 1; + } + } diff --git a/tests/Settings/Controller/AppSettingsControllerTest.php b/tests/Settings/Controller/AppSettingsControllerTest.php index 2f916b89707..7190d2cb6c5 100644 --- a/tests/Settings/Controller/AppSettingsControllerTest.php +++ b/tests/Settings/Controller/AppSettingsControllerTest.php @@ -30,6 +30,7 @@ use OC\Settings\Controller\AppSettingsController; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\ILogger; use OCP\IURLGenerator; use OCP\L10N\IFactory; use Test\TestCase; @@ -69,6 +70,8 @@ class AppSettingsControllerTest extends TestCase { private $installer; /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ private $urlGenerator; + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + private $logger; public function setUp() { parent::setUp(); @@ -87,6 +90,7 @@ class AppSettingsControllerTest extends TestCase { $this->bundleFetcher = $this->createMock(BundleFetcher::class); $this->installer = $this->createMock(Installer::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->logger = $this->createMock(ILogger::class); $this->appSettingsController = new AppSettingsController( 'settings', @@ -100,7 +104,8 @@ class AppSettingsControllerTest extends TestCase { $this->l10nFactory, $this->bundleFetcher, $this->installer, - $this->urlGenerator + $this->urlGenerator, + $this->logger ); } @@ -109,32 +114,6 @@ class AppSettingsControllerTest extends TestCase { ->method('isUpdateAvailable') ->willReturn(false); $expected = new JSONResponse([ - [ - 'id' => 2, - 'ident' => 'installed', - 'displayName' => 'Your apps', - ], - [ - 'id' => 4, - 'ident' => 'updates', - 'displayName' => 'Updates', - 'counter' => 0, - ], - [ - 'id' => 0, - 'ident' => 'enabled', - 'displayName' => 'Enabled apps', - ], - [ - 'id' => 1, - 'ident' => 'disabled', - 'displayName' => 'Disabled apps', - ], - [ - 'id' => 3, - 'ident' => 'app-bundles', - 'displayName' => 'App bundles', - ], [ 'id' => 'auth', 'ident' => 'auth', @@ -196,6 +175,7 @@ class AppSettingsControllerTest extends TestCase { } public function testViewApps() { + $this->bundleFetcher->expects($this->once())->method('getBundles')->willReturn([]); $this->config ->expects($this->once()) ->method('getSystemValue') @@ -210,11 +190,15 @@ class AppSettingsControllerTest extends TestCase { $policy->addAllowedImageDomain('https://usercontent.apps.nextcloud.com'); $expected = new TemplateResponse('settings', - 'apps', + 'settings', [ - 'category' => 'installed', - 'appstoreEnabled' => true, - 'urlGenerator' => $this->urlGenerator, + 'serverData' => [ + 'updateCount' => 67, + 'appstoreEnabled' => true, + 'urlGenerator' => $this->urlGenerator, + 'bundles' => [], + 'developerDocumentation' => '' + ] ], 'user'); $expected->setContentSecurityPolicy($policy); @@ -223,6 +207,7 @@ class AppSettingsControllerTest extends TestCase { } public function testViewAppsAppstoreNotEnabled() { + $this->bundleFetcher->expects($this->once())->method('getBundles')->willReturn([]); $this->config ->expects($this->once()) ->method('getSystemValue') @@ -237,11 +222,15 @@ class AppSettingsControllerTest extends TestCase { $policy->addAllowedImageDomain('https://usercontent.apps.nextcloud.com'); $expected = new TemplateResponse('settings', - 'apps', + 'settings', [ - 'category' => 'installed', - 'appstoreEnabled' => false, - 'urlGenerator' => $this->urlGenerator, + 'serverData' => [ + 'updateCount' => 67, + 'appstoreEnabled' => false, + 'urlGenerator' => $this->urlGenerator, + 'bundles' => [], + 'developerDocumentation' => '' + ] ], 'user'); $expected->setContentSecurityPolicy($policy);