Skip to content

Commit

Permalink
fix(TemplateLayout): core is not an app but the server itself
Browse files Browse the repository at this point in the history
Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Jan 29, 2025
1 parent ef015a9 commit a82a097
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 56 deletions.
4 changes: 1 addition & 3 deletions lib/private/App/AppManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserSession;
use OCP\ServerVersion;
use OCP\Settings\IManager as ISettingsManager;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -80,7 +79,6 @@ public function __construct(
private ICacheFactory $memCacheFactory,
private IEventDispatcher $dispatcher,
private LoggerInterface $logger,
private ServerVersion $serverVersion,
) {
}

Expand Down Expand Up @@ -739,7 +737,7 @@ 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])) {
if ($appId === 'core') {
$this->appVersions[$appId] = $this->serverVersion->getVersionString();
$this->appVersions[$appId] = \OC_Util::getVersionString();
} else {
$appInfo = $this->getAppInfo($appId);
$this->appVersions[$appId] = ($appInfo !== null && isset($appInfo['version'])) ? $appInfo['version'] : '0';
Expand Down
1 change: 0 additions & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,6 @@ public function __construct($webRoot, \OC\Config $config) {
$c->get(ICacheFactory::class),
$c->get(IEventDispatcher::class),
$c->get(LoggerInterface::class),
$c->get(ServerVersion::class),
);
});
/** @deprecated 19.0.0 */
Expand Down
17 changes: 11 additions & 6 deletions lib/private/TemplateLayout.php
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,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
27 changes: 2 additions & 25 deletions tests/lib/App/AppManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserSession;
use OCP\ServerVersion;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
Expand Down Expand Up @@ -101,8 +100,6 @@ protected function getAppConfig() {

protected IURLGenerator&MockObject $urlGenerator;

protected ServerVersion&MockObject $serverVersion;

/** @var IAppManager */
protected $manager;

Expand All @@ -118,7 +115,6 @@ protected function setUp(): void {
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->serverVersion = $this->createMock(ServerVersion::class);

$this->overwriteService(AppConfig::class, $this->appConfig);
$this->overwriteService(IURLGenerator::class, $this->urlGenerator);
Expand All @@ -140,7 +136,6 @@ protected function setUp(): void {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
);
}

Expand Down Expand Up @@ -291,7 +286,6 @@ public function testEnableAppForGroups() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppPath',
Expand Down Expand Up @@ -345,7 +339,6 @@ public function testEnableAppForGroupsAllowedTypes(array $appInfo) {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppPath',
Expand Down Expand Up @@ -407,7 +400,6 @@ public function testEnableAppForGroupsForbiddenTypes($type) {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppPath',
Expand Down Expand Up @@ -610,7 +602,6 @@ public function testGetAppsNeedingUpgrade() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods(['getAppInfo'])
->getMock();
Expand Down Expand Up @@ -669,7 +660,6 @@ public function testGetIncompatibleApps() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods(['getAppInfo'])
->getMock();
Expand Down Expand Up @@ -961,7 +951,6 @@ public function testGetAppVersion() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppInfo',
Expand All @@ -973,10 +962,6 @@ public function testGetAppVersion() {
->with('myapp')
->willReturn(['version' => '99.99.99-rc.99']);

$this->serverVersion
->expects(self::never())
->method('getVersionString');

$this->assertEquals(
'99.99.99-rc.99',
$manager->getAppVersion('myapp'),
Expand All @@ -992,7 +977,6 @@ public function testGetAppVersionCore() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppInfo',
Expand All @@ -1002,10 +986,8 @@ public function testGetAppVersionCore() {
$manager->expects(self::never())
->method('getAppInfo');

$this->serverVersion
->expects(self::once())
->method('getVersionString')
->willReturn('1.2.3-beta.4');
$util = new \OC_Util();
self::invokePrivate($util, 'versionCache', [['OC_VersionString' => '1.2.3-beta.4']]);

$this->assertEquals(
'1.2.3-beta.4',
Expand All @@ -1022,7 +1004,6 @@ public function testGetAppVersionUnknown() {
$this->cacheFactory,
$this->eventDispatcher,
$this->logger,
$this->serverVersion,
])
->onlyMethods([
'getAppInfo',
Expand All @@ -1034,10 +1015,6 @@ public function testGetAppVersionUnknown() {
->with('unknown')
->willReturn(null);

$this->serverVersion
->expects(self::never())
->method('getVersionString');

$this->assertEquals(
'0',
$manager->getAppVersion('unknown'),
Expand Down
46 changes: 25 additions & 21 deletions tests/lib/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
use OC\AppConfig;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IAppConfig;
use OCP\IURLGenerator;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\IUserManager;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -457,12 +462,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 @@ -506,9 +511,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 @@ -539,8 +544,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 @@ -556,13 +561,12 @@ 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(),
\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(IGroupManager::class),
\OCP\Server::get(ICacheFactory::class),
\OCP\Server::get(IEventDispatcher::class),
\OCP\Server::get(LoggerInterface::class),
));
}

Expand Down Expand Up @@ -621,14 +625,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 a82a097

Please sign in to comment.