diff options
-rw-r--r-- | apps/updatenotification/lib/Notification/AppUpdateNotifier.php | 11 | ||||
-rw-r--r-- | lib/private/App/AppManager.php | 16 | ||||
-rw-r--r-- | lib/private/NavigationManager.php | 12 | ||||
-rw-r--r-- | lib/private/Server.php | 3 | ||||
-rw-r--r-- | lib/public/App/IAppManager.php | 9 | ||||
-rw-r--r-- | tests/lib/App/AppManagerTest.php | 129 | ||||
-rw-r--r-- | tests/lib/AppTest.php | 6 | ||||
-rw-r--r-- | tests/lib/NavigationManagerTest.php | 23 |
8 files changed, 170 insertions, 39 deletions
diff --git a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php index 9fb40e76a13..d5509bfcd75 100644 --- a/apps/updatenotification/lib/Notification/AppUpdateNotifier.php +++ b/apps/updatenotification/lib/Notification/AppUpdateNotifier.php @@ -84,16 +84,7 @@ class AppUpdateNotifier implements INotifier { // Prepare translation factory for requested language $l = $this->l10nFactory->get(Application::APP_NAME, $languageCode); - // See if we can find the app icon - if not fall back to default icon - $possibleIcons = [$appId . '-dark.svg', 'app-dark.svg', $appId . '.svg', 'app.svg']; - $icon = null; - foreach ($possibleIcons as $iconName) { - try { - $icon = $this->urlGenerator->imagePath($appId, $iconName); - } catch (\RuntimeException $e) { - // ignore - } - } + $icon = $this->appManager->getAppIcon($appId); if ($icon === null) { $icon = $this->urlGenerator->imagePath('core', 'default-app-icon'); } diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 5eeb7ef1fbd..5dec4e7ccde 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -56,6 +56,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; use OCP\Settings\IManager as ISettingsManager; @@ -104,9 +105,24 @@ class AppManager implements IAppManager { private ICacheFactory $memCacheFactory, private IEventDispatcher $dispatcher, private LoggerInterface $logger, + private IURLGenerator $urlGenerator, ) { } + public function getAppIcon(string $appId): ?string { + $possibleIcons = [$appId . '.svg', 'app.svg', $appId . '-dark.svg', 'app-dark.svg']; + $icon = null; + foreach ($possibleIcons as $iconName) { + try { + $icon = $this->urlGenerator->imagePath($appId, $iconName); + break; + } catch (\RuntimeException $e) { + // ignore + } + } + return $icon; + } + /** * @return string[] $appId => $enabled */ diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 17573d9db86..cac52ab5c9f 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -372,16 +372,18 @@ class NavigationManager implements INavigationManager { $order = $nav['order'] ?? 100; $type = $nav['type']; $route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : ''; - $icon = $nav['icon'] ?? 'app.svg'; - foreach ([$icon, "$app.svg"] as $i) { + $icon = $nav['icon'] ?? null; + if ($icon !== null) { try { - $icon = $this->urlGenerator->imagePath($app, $i); - break; + $icon = $this->urlGenerator->imagePath($app, $icon); } catch (\RuntimeException $ex) { - // no icon? - ignore it then + // ignore } } if ($icon === null) { + $icon = $this->appManager->getAppIcon($app); + } + if ($icon === null) { $icon = $this->urlGenerator->imagePath('core', 'default-app-icon'); } diff --git a/lib/private/Server.php b/lib/private/Server.php index 385d654b951..2732b6486f2 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -893,7 +893,8 @@ class Server extends ServerContainer implements IServerContainer { $c->get(IGroupManager::class), $c->get(ICacheFactory::class), $c->get(IEventDispatcher::class), - $c->get(LoggerInterface::class) + $c->get(LoggerInterface::class), + $c->get(IURLGenerator::class), ); }); /** @deprecated 19.0.0 */ diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php index d15cfdbea96..968771388dc 100644 --- a/lib/public/App/IAppManager.php +++ b/lib/public/App/IAppManager.php @@ -62,6 +62,15 @@ interface IAppManager { public function getAppVersion(string $appId, bool $useCache = true): string; /** + * Returns the app icon or null if none is found + * + * @param string $appId + * @return string|null + * @since 29.0.0 + */ + public function getAppIcon(string $appId): string|null; + + /** * Check if an app is enabled for user * * @param string $appId diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index 3733d6cd2e9..09a64f84469 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -23,6 +23,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; @@ -98,6 +99,8 @@ class AppManagerTest extends TestCase { /** @var LoggerInterface|MockObject */ protected $logger; + protected IURLGenerator|MockObject $urlGenerator; + /** @var IAppManager */ protected $manager; @@ -112,6 +115,7 @@ class AppManagerTest extends TestCase { $this->cache = $this->createMock(ICache::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->cacheFactory->expects($this->any()) ->method('createDistributed') ->with('settings') @@ -123,10 +127,74 @@ class AppManagerTest extends TestCase { $this->groupManager, $this->cacheFactory, $this->eventDispatcher, - $this->logger + $this->logger, + $this->urlGenerator, ); } + /** + * @dataProvider dataGetAppIcon + */ + public function testGetAppIcon($callback, string|null $expected) { + $this->urlGenerator->expects($this->atLeastOnce()) + ->method('imagePath') + ->willReturnCallback($callback); + + $this->assertEquals($expected, $this->manager->getAppIcon('test')); + } + + public function dataGetAppIcon(): array { + $nothing = function ($appId) { + $this->assertEquals('test', $appId); + throw new \RuntimeException(); + }; + + $createCallback = function ($workingIcons) { + return function ($appId, $icon) use ($workingIcons) { + $this->assertEquals('test', $appId); + if (in_array($icon, $workingIcons)) { + return '/path/' . $icon; + } + throw new \RuntimeException(); + }; + }; + + return [ + 'does not find anything' => [ + $nothing, + null, + ], + 'only app.svg' => [ + $createCallback(['app.svg']), + '/path/app.svg', + ], + 'only app-dark.svg' => [ + $createCallback(['app-dark.svg']), + '/path/app-dark.svg', + ], + 'only appname -dark.svg' => [ + $createCallback(['test-dark.svg']), + '/path/test-dark.svg', + ], + 'only appname.svg' => [ + $createCallback(['test.svg']), + '/path/test.svg', + ], + 'priotize custom over default' => [ + $createCallback(['app.svg', 'test.svg']), + '/path/test.svg', + ], + 'priotize default over dark' => [ + $createCallback(['test-dark.svg', 'app-dark.svg', 'app.svg']), + '/path/app.svg', + ], + 'priotize custom over default' => [ + $createCallback(['test.svg', 'test-dark.svg', 'app-dark.svg']), + '/path/test.svg', + ], + ]; + } + public function testEnableApp() { // making sure "files_trashbin" is disabled if ($this->manager->isEnabledForUser('files_trashbin')) { @@ -170,9 +238,16 @@ class AppManagerTest extends TestCase { /** @var AppManager|MockObject $manager */ $manager = $this->getMockBuilder(AppManager::class) ->setConstructorArgs([ - $this->userSession, $this->config, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher, $this->logger + $this->userSession, + $this->config, + $this->appConfig, + $this->groupManager, + $this->cacheFactory, + $this->eventDispatcher, + $this->logger, + $this->urlGenerator, ]) - ->setMethods([ + ->onlyMethods([ 'getAppPath', ]) ->getMock(); @@ -218,9 +293,16 @@ class AppManagerTest extends TestCase { /** @var AppManager|MockObject $manager */ $manager = $this->getMockBuilder(AppManager::class) ->setConstructorArgs([ - $this->userSession, $this->config, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher, $this->logger + $this->userSession, + $this->config, + $this->appConfig, + $this->groupManager, + $this->cacheFactory, + $this->eventDispatcher, + $this->logger, + $this->urlGenerator, ]) - ->setMethods([ + ->onlyMethods([ 'getAppPath', 'getAppInfo', ]) @@ -274,9 +356,16 @@ class AppManagerTest extends TestCase { /** @var AppManager|MockObject $manager */ $manager = $this->getMockBuilder(AppManager::class) ->setConstructorArgs([ - $this->userSession, $this->config, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher, $this->logger + $this->userSession, + $this->config, + $this->appConfig, + $this->groupManager, + $this->cacheFactory, + $this->eventDispatcher, + $this->logger, + $this->urlGenerator, ]) - ->setMethods([ + ->onlyMethods([ 'getAppPath', 'getAppInfo', ]) @@ -470,8 +559,17 @@ class AppManagerTest extends TestCase { public function testGetAppsNeedingUpgrade() { /** @var AppManager|MockObject $manager */ $manager = $this->getMockBuilder(AppManager::class) - ->setConstructorArgs([$this->userSession, $this->config, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher, $this->logger]) - ->setMethods(['getAppInfo']) + ->setConstructorArgs([ + $this->userSession, + $this->config, + $this->appConfig, + $this->groupManager, + $this->cacheFactory, + $this->eventDispatcher, + $this->logger, + $this->urlGenerator, + ]) + ->onlyMethods(['getAppInfo']) ->getMock(); $appInfos = [ @@ -521,8 +619,17 @@ class AppManagerTest extends TestCase { public function testGetIncompatibleApps() { /** @var AppManager|MockObject $manager */ $manager = $this->getMockBuilder(AppManager::class) - ->setConstructorArgs([$this->userSession, $this->config, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher, $this->logger]) - ->setMethods(['getAppInfo']) + ->setConstructorArgs([ + $this->userSession, + $this->config, + $this->appConfig, + $this->groupManager, + $this->cacheFactory, + $this->eventDispatcher, + $this->logger, + $this->urlGenerator, + ]) + ->onlyMethods(['getAppInfo']) ->getMock(); $appInfos = [ diff --git a/tests/lib/AppTest.php b/tests/lib/AppTest.php index 12fbdb011d9..4e723e5d2f1 100644 --- a/tests/lib/AppTest.php +++ b/tests/lib/AppTest.php @@ -14,6 +14,8 @@ use OC\App\InfoParser; use OC\AppConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; +use OCP\IURLGenerator; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -537,6 +539,7 @@ class AppTest extends \Test\TestCase { private function setupAppConfigMock() { + /** @var AppConfig|MockObject */ $appConfig = $this->getMockBuilder(AppConfig::class) ->setMethods(['getValues']) ->setConstructorArgs([\OC::$server->getDatabaseConnection()]) @@ -561,7 +564,8 @@ class AppTest extends \Test\TestCase { \OC::$server->getGroupManager(), \OC::$server->getMemCacheFactory(), \OC::$server->get(IEventDispatcher::class), - \OC::$server->get(LoggerInterface::class) + \OC::$server->get(LoggerInterface::class), + \OC::$server->get(IURLGenerator::class), )); } diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php index 63160e78de7..beefc2353d6 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -224,19 +224,19 @@ class NavigationManagerTest extends TestCase { ->method('isEnabledForUser') ->with('theming') ->willReturn(true); - $this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation); - /* + $this->appManager->expects($this->once()) + ->method('getAppInfo') + ->with('test') + ->willReturn($navigation); + $this->urlGenerator->expects($this->any()) + ->method('imagePath') + ->willReturnCallback(function ($appName, $file) { + return "/apps/$appName/img/$file"; + }); $this->appManager->expects($this->any()) - ->method('getAppInfo') - ->will($this->returnValueMap([ - ['test', null, null, $navigation], - ['theming', null, null, null], - ])); - */ + ->method('getAppIcon') + ->willReturnCallback(fn (string $appName) => "/apps/$appName/img/app.svg"); $this->l10nFac->expects($this->any())->method('get')->willReturn($l); - $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) { - return "/apps/$appName/img/$file"; - }); $this->urlGenerator->expects($this->any())->method('linkToRoute')->willReturnCallback(function ($route) { if ($route === 'core.login.logout') { return 'https://example.com/logout'; @@ -534,6 +534,7 @@ class NavigationManagerTest extends TestCase { ->with('theming') ->willReturn(true); $this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation); + $this->appManager->expects($this->once())->method('getAppIcon')->with('test')->willReturn('/apps/test/img/app.svg'); $this->l10nFac->expects($this->any())->method('get')->willReturn($l); $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) { return "/apps/$appName/img/$file"; |