Skip to content

Commit

Permalink
fix: Correctly return app id and app version for core styles and im…
Browse files Browse the repository at this point in the history 
…ages

fix(TemplateLayout): `core` is not an app but the server itself

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
@susnux
susnux committed Jan 29, 2025
1 parent 7a8285b commit c244364
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 29 deletions.
8 changes: 6 additions & 2 deletions lib/private/App/AppManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,12 @@ public function getAppInfo(string $appId, bool $path = false, $lang = null) {

public function getAppVersion(string $appId, bool $useCache = true): string {
if (!$useCache || !isset($this->appVersions[$appId])) {
$appInfo = $this->getAppInfo($appId);
$this->appVersions[$appId] = ($appInfo !== null && isset($appInfo['version'])) ? $appInfo['version'] : '0';
if ($appId === 'core') {
$this->appVersions[$appId] = \OC_Util::getVersionString();
} else {
$appInfo = $this->getAppInfo($appId);
$this->appVersions[$appId] = ($appInfo !== null && isset($appInfo['version'])) ? $appInfo['version'] : '0';
}
}
return $this->appVersions[$appId];
}
Expand Down
19 changes: 13 additions & 6 deletions lib/private/TemplateLayout.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,18 @@ private function getVersionHashByPath(string $path): string|false {
return false;
}

$appVersion = $this->appManager->getAppVersion($appId);
// For shipped apps the app version is not a single source of truth, we rather also need to consider the Nextcloud version
if ($this->appManager->isShipped($appId)) {
$appVersion .= '-' . self::$versionHash;
}
if ($appId === 'core') {
// core is not a real app but the server itself
$hash = self::$versionHash;
} else {
$appVersion = $this->appManager->getAppVersion($appId);
// For shipped apps the app version is not a single source of truth, we rather also need to consider the Nextcloud version
if ($this->appManager->isShipped($appId)) {
$appVersion .= '-' . self::$versionHash;
}

$hash = substr(md5($appVersion), 0, 8);
$hash = substr(md5($appVersion), 0, 8);
}
self::$cacheBusterCache[$path] = $hash;
}

Expand Down Expand Up @@ -392,6 +397,8 @@ public function getAppNamefromPath(string $path) {
if ($pathParts[0] === 'css') {
// This is a scss request
return $pathParts[1];
} elseif ($pathParts[0] === 'core') {
return 'core';
}
return end($pathParts);
}
Expand Down
95 changes: 95 additions & 0 deletions tests/lib/App/AppManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ protected function setUp(): void {
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);

$this->overwriteService(AppConfig::class, $this->appConfig);
$this->overwriteService(IURLGenerator::class, $this->urlGenerator);

$this->cacheFactory->expects($this->any())
->method('createDistributed')
->with('settings')
Expand All @@ -132,6 +136,14 @@ protected function setUp(): void {
);
}

protected function tearDown(): void {
parent::tearDown();

// Reset static OC_Util
$util = new \OC_Util();
self::invokePrivate($util, 'versionCache', [null]);
}

/**
* @dataProvider dataGetAppIcon
*/
Expand Down Expand Up @@ -888,4 +900,87 @@ public function testGetDefaultAppForUser($defaultApps, $userDefaultApps, $userAp

$this->assertEquals($expectedApp, $this->manager->getDefaultAppForUser(null, $withFallbacks));
}

public function testGetAppVersion() {
$manager = $this->getMockBuilder(AppManager::class)
->setConstructorArgs([
$this->userSession,
$this->config,
$this->appConfig,
$this->groupManager,
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
])
->onlyMethods([
'getAppInfo',
])
->getMock();

$manager->expects(self::once())
->method('getAppInfo')
->with('myapp')
->willReturn(['version' => '99.99.99-rc.99']);

$this->assertEquals(
'99.99.99-rc.99',
$manager->getAppVersion('myapp'),
);
}

public function testGetAppVersionCore() {
$manager = $this->getMockBuilder(AppManager::class)
->setConstructorArgs([
$this->userSession,
$this->config,
$this->appConfig,
$this->groupManager,
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
])
->onlyMethods([
'getAppInfo',
])
->getMock();

$manager->expects(self::never())
->method('getAppInfo');

$util = new \OC_Util();
self::invokePrivate($util, 'versionCache', [['OC_VersionString' => '1.2.3-beta.4']]);

$this->assertEquals(
'1.2.3-beta.4',
$manager->getAppVersion('core'),
);
}

public function testGetAppVersionUnknown() {
$manager = $this->getMockBuilder(AppManager::class)
->setConstructorArgs([
$this->userSession,
$this->config,
$this->appConfig,
$this->groupManager,
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
])
->onlyMethods([
'getAppInfo',
])
->getMock();

$manager->expects(self::once())
->method('getAppInfo')
->with('unknown')
->willReturn(null);

$this->assertEquals(
'0',
$manager->getAppVersion('unknown'),
);
}

}
48 changes: 27 additions & 21 deletions tests/lib/AppTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@
use OC\AppConfig;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IAppConfig;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -459,12 +465,12 @@ public function appConfigValuesProvider() {
*
* @dataProvider appConfigValuesProvider
*/
public function testEnabledApps($user, $expectedApps, $forceAll) {
$userManager = \OC::$server->getUserManager();
$groupManager = \OC::$server->getGroupManager();
$user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
$user2 = $userManager->createUser(self::TEST_USER2, self::TEST_USER2);
$user3 = $userManager->createUser(self::TEST_USER3, self::TEST_USER3);
public function testEnabledApps($user, $expectedApps, $forceAll): void {
$userManager = \OCP\Server::get(IUserManager::class);
$groupManager = \OCP\Server::get(IGroupManager::class);
$user1 = $userManager->createUser(self::TEST_USER1, 'NotAnEasyPassword123456+');
$user2 = $userManager->createUser(self::TEST_USER2, 'NotAnEasyPassword123456_');
$user3 = $userManager->createUser(self::TEST_USER3, 'NotAnEasyPassword123456?');

$group1 = $groupManager->createGroup(self::TEST_GROUP1);
$group1->addUser($user1);
Expand Down Expand Up @@ -508,9 +514,9 @@ public function testEnabledApps($user, $expectedApps, $forceAll) {
* Test isEnabledApps() with cache, not re-reading the list of
* enabled apps more than once when a user is set.
*/
public function testEnabledAppsCache() {
$userManager = \OC::$server->getUserManager();
$user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
public function testEnabledAppsCache(): void {
$userManager = \OCP\Server::get(IUserManager::class);
$user1 = $userManager->createUser(self::TEST_USER1, 'NotAnEasyPassword123456+');

\OC_User::setUserId(self::TEST_USER1);

Expand Down Expand Up @@ -541,8 +547,8 @@ public function testEnabledAppsCache() {
private function setupAppConfigMock() {
/** @var AppConfig|MockObject */
$appConfig = $this->getMockBuilder(AppConfig::class)
->setMethods(['getValues'])
->setConstructorArgs([\OC::$server->getDatabaseConnection()])
->onlyMethods(['getValues'])
->setConstructorArgs([\OCP\Server::get(IDBConnection::class)])
->disableOriginalConstructor()
->getMock();

Expand All @@ -558,14 +564,14 @@ private function setupAppConfigMock() {
private function registerAppConfig(AppConfig $appConfig) {
$this->overwriteService(AppConfig::class, $appConfig);
$this->overwriteService(AppManager::class, new AppManager(
\OC::$server->getUserSession(),
\OC::$server->getConfig(),
$appConfig,
\OC::$server->getGroupManager(),
\OC::$server->getMemCacheFactory(),
\OC::$server->get(IEventDispatcher::class),
\OC::$server->get(LoggerInterface::class),
\OC::$server->get(IURLGenerator::class),
\OCP\Server::get(IUserSession::class),
\OCP\Server::get(IConfig::class),
\OCP\Server::get(IAppConfig::class),
\OCP\Server::get(IGroupManager::class),
\OCP\Server::get(ICacheFactory::class),
\OCP\Server::get(IEventDispatcher::class),
\OCP\Server::get(LoggerInterface::class),
\OCP\Server::get(IURLGenerator::class),
));
}

Expand Down Expand Up @@ -624,14 +630,14 @@ public function testParseAppInfo(array $data, array $expected) {

public function testParseAppInfoL10N() {
$parser = new InfoParser();
$data = $parser->parse(\OC::$SERVERROOT. "/tests/data/app/description-multi-lang.xml");
$data = $parser->parse(\OC::$SERVERROOT . '/tests/data/app/description-multi-lang.xml');
$this->assertEquals('English', \OC_App::parseAppInfo($data, 'en')['description']);
$this->assertEquals('German', \OC_App::parseAppInfo($data, 'de')['description']);
}

public function testParseAppInfoL10NSingleLanguage() {
$parser = new InfoParser();
$data = $parser->parse(\OC::$SERVERROOT. "/tests/data/app/description-single-lang.xml");
$data = $parser->parse(\OC::$SERVERROOT . '/tests/data/app/description-single-lang.xml');
$this->assertEquals('English', \OC_App::parseAppInfo($data, 'en')['description']);
}
}

0 comments on commit c244364

Please sign in to comment.