aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--apps/updatenotification/lib/Notification/AppUpdateNotifier.php11
-rw-r--r--lib/private/App/AppManager.php16
-rw-r--r--lib/private/NavigationManager.php12
-rw-r--r--lib/private/Server.php3
-rw-r--r--lib/public/App/IAppManager.php9
-rw-r--r--tests/lib/App/AppManagerTest.php129
-rw-r--r--tests/lib/AppTest.php6
-rw-r--r--tests/lib/NavigationManagerTest.php23
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";