Skip to content

Commit 7d8588f

Browse files
committed
fix(TemplateLayout): core is not an app but the server itself
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 82abdf6 commit 7d8588f

File tree

5 files changed

+39
-56
lines changed

5 files changed

+39
-56
lines changed

lib/private/App/AppManager.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
use OCP\IURLGenerator;
2828
use OCP\IUser;
2929
use OCP\IUserSession;
30-
use OCP\ServerVersion;
3130
use OCP\Settings\IManager as ISettingsManager;
3231
use Psr\Log\LoggerInterface;
3332

@@ -80,7 +79,6 @@ public function __construct(
8079
private ICacheFactory $memCacheFactory,
8180
private IEventDispatcher $dispatcher,
8281
private LoggerInterface $logger,
83-
private ServerVersion $serverVersion,
8482
) {
8583
}
8684

@@ -739,7 +737,7 @@ public function getAppInfo(string $appId, bool $path = false, $lang = null) {
739737
public function getAppVersion(string $appId, bool $useCache = true): string {
740738
if (!$useCache || !isset($this->appVersions[$appId])) {
741739
if ($appId === 'core') {
742-
$this->appVersions[$appId] = $this->serverVersion->getVersionString();
740+
$this->appVersions[$appId] = \OC_Util::getVersionString();
743741
} else {
744742
$appInfo = $this->getAppInfo($appId);
745743
$this->appVersions[$appId] = ($appInfo !== null && isset($appInfo['version'])) ? $appInfo['version'] : '0';

lib/private/Server.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,6 @@ public function __construct($webRoot, \OC\Config $config) {
861861
$c->get(ICacheFactory::class),
862862
$c->get(IEventDispatcher::class),
863863
$c->get(LoggerInterface::class),
864-
$c->get(ServerVersion::class),
865864
);
866865
});
867866
/** @deprecated 19.0.0 */

lib/private/TemplateLayout.php

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,13 +342,18 @@ private function getVersionHashByPath(string $path): string|false {
342342
return false;
343343
}
344344

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

351-
$hash = substr(md5($appVersion), 0, 8);
355+
$hash = substr(md5($appVersion), 0, 8);
356+
}
352357
self::$cacheBusterCache[$path] = $hash;
353358
}
354359

tests/lib/App/AppManagerTest.php

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
use OCP\IURLGenerator;
2626
use OCP\IUser;
2727
use OCP\IUserSession;
28-
use OCP\ServerVersion;
2928
use PHPUnit\Framework\MockObject\MockObject;
3029
use Psr\Log\LoggerInterface;
3130
use Test\TestCase;
@@ -101,8 +100,6 @@ protected function getAppConfig() {
101100

102101
protected IURLGenerator&MockObject $urlGenerator;
103102

104-
protected ServerVersion&MockObject $serverVersion;
105-
106103
/** @var IAppManager */
107104
protected $manager;
108105

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

123119
$this->overwriteService(AppConfig::class, $this->appConfig);
124120
$this->overwriteService(IURLGenerator::class, $this->urlGenerator);
@@ -140,7 +136,6 @@ protected function setUp(): void {
140136
$this->cacheFactory,
141137
$this->eventDispatcher,
142138
$this->logger,
143-
$this->serverVersion,
144139
);
145140
}
146141

@@ -283,7 +278,6 @@ public function testEnableAppForGroups() {
283278
$this->cacheFactory,
284279
$this->eventDispatcher,
285280
$this->logger,
286-
$this->serverVersion,
287281
])
288282
->onlyMethods([
289283
'getAppPath',
@@ -337,7 +331,6 @@ public function testEnableAppForGroupsAllowedTypes(array $appInfo) {
337331
$this->cacheFactory,
338332
$this->eventDispatcher,
339333
$this->logger,
340-
$this->serverVersion,
341334
])
342335
->onlyMethods([
343336
'getAppPath',
@@ -399,7 +392,6 @@ public function testEnableAppForGroupsForbiddenTypes($type) {
399392
$this->cacheFactory,
400393
$this->eventDispatcher,
401394
$this->logger,
402-
$this->serverVersion,
403395
])
404396
->onlyMethods([
405397
'getAppPath',
@@ -602,7 +594,6 @@ public function testGetAppsNeedingUpgrade() {
602594
$this->cacheFactory,
603595
$this->eventDispatcher,
604596
$this->logger,
605-
$this->serverVersion,
606597
])
607598
->onlyMethods(['getAppInfo'])
608599
->getMock();
@@ -661,7 +652,6 @@ public function testGetIncompatibleApps() {
661652
$this->cacheFactory,
662653
$this->eventDispatcher,
663654
$this->logger,
664-
$this->serverVersion,
665655
])
666656
->onlyMethods(['getAppInfo'])
667657
->getMock();
@@ -953,7 +943,6 @@ public function testGetAppVersion() {
953943
$this->cacheFactory,
954944
$this->eventDispatcher,
955945
$this->logger,
956-
$this->serverVersion,
957946
])
958947
->onlyMethods([
959948
'getAppInfo',
@@ -965,10 +954,6 @@ public function testGetAppVersion() {
965954
->with('myapp')
966955
->willReturn(['version' => '99.99.99-rc.99']);
967956

968-
$this->serverVersion
969-
->expects(self::never())
970-
->method('getVersionString');
971-
972957
$this->assertEquals(
973958
'99.99.99-rc.99',
974959
$manager->getAppVersion('myapp'),
@@ -984,7 +969,6 @@ public function testGetAppVersionCore() {
984969
$this->cacheFactory,
985970
$this->eventDispatcher,
986971
$this->logger,
987-
$this->serverVersion,
988972
])
989973
->onlyMethods([
990974
'getAppInfo',
@@ -994,10 +978,8 @@ public function testGetAppVersionCore() {
994978
$manager->expects(self::never())
995979
->method('getAppInfo');
996980

997-
$this->serverVersion
998-
->expects(self::once())
999-
->method('getVersionString')
1000-
->willReturn('1.2.3-beta.4');
981+
$util = new \OC_Util();
982+
self::invokePrivate($util, 'versionCache', [['OC_VersionString' => '1.2.3-beta.4']]);
1001983

1002984
$this->assertEquals(
1003985
'1.2.3-beta.4',
@@ -1014,7 +996,6 @@ public function testGetAppVersionUnknown() {
1014996
$this->cacheFactory,
1015997
$this->eventDispatcher,
1016998
$this->logger,
1017-
$this->serverVersion,
1018999
])
10191000
->onlyMethods([
10201001
'getAppInfo',
@@ -1026,10 +1007,6 @@ public function testGetAppVersionUnknown() {
10261007
->with('unknown')
10271008
->willReturn(null);
10281009

1029-
$this->serverVersion
1030-
->expects(self::never())
1031-
->method('getVersionString');
1032-
10331010
$this->assertEquals(
10341011
'0',
10351012
$manager->getAppVersion('unknown'),

tests/lib/AppTest.php

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@
1212
use OC\AppConfig;
1313
use OCP\EventDispatcher\IEventDispatcher;
1414
use OCP\IAppConfig;
15-
use OCP\IURLGenerator;
15+
use OCP\ICacheFactory;
16+
use OCP\IConfig;
17+
use OCP\IDBConnection;
18+
use OCP\IGroupManager;
19+
use OCP\IUserManager;
20+
use OCP\IUserSession;
1621
use PHPUnit\Framework\MockObject\MockObject;
1722
use Psr\Log\LoggerInterface;
1823

@@ -457,12 +462,12 @@ public function appConfigValuesProvider() {
457462
*
458463
* @dataProvider appConfigValuesProvider
459464
*/
460-
public function testEnabledApps($user, $expectedApps, $forceAll) {
461-
$userManager = \OC::$server->getUserManager();
462-
$groupManager = \OC::$server->getGroupManager();
463-
$user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
464-
$user2 = $userManager->createUser(self::TEST_USER2, self::TEST_USER2);
465-
$user3 = $userManager->createUser(self::TEST_USER3, self::TEST_USER3);
465+
public function testEnabledApps($user, $expectedApps, $forceAll): void {
466+
$userManager = \OCP\Server::get(IUserManager::class);
467+
$groupManager = \OCP\Server::get(IGroupManager::class);
468+
$user1 = $userManager->createUser(self::TEST_USER1, 'NotAnEasyPassword123456+');
469+
$user2 = $userManager->createUser(self::TEST_USER2, 'NotAnEasyPassword123456_');
470+
$user3 = $userManager->createUser(self::TEST_USER3, 'NotAnEasyPassword123456?');
466471

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

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

@@ -539,8 +544,8 @@ public function testEnabledAppsCache() {
539544
private function setupAppConfigMock() {
540545
/** @var AppConfig|MockObject */
541546
$appConfig = $this->getMockBuilder(AppConfig::class)
542-
->setMethods(['getValues'])
543-
->setConstructorArgs([\OC::$server->getDatabaseConnection()])
547+
->onlyMethods(['getValues'])
548+
->setConstructorArgs([\OCP\Server::get(IDBConnection::class)])
544549
->disableOriginalConstructor()
545550
->getMock();
546551

@@ -556,13 +561,12 @@ private function setupAppConfigMock() {
556561
private function registerAppConfig(AppConfig $appConfig) {
557562
$this->overwriteService(AppConfig::class, $appConfig);
558563
$this->overwriteService(AppManager::class, new AppManager(
559-
\OC::$server->getUserSession(),
560-
\OC::$server->getConfig(),
561-
\OC::$server->getGroupManager(),
562-
\OC::$server->getMemCacheFactory(),
563-
\OC::$server->get(IEventDispatcher::class),
564-
\OC::$server->get(LoggerInterface::class),
565-
\OC::$server->get(IURLGenerator::class),
564+
\OCP\Server::get(IUserSession::class),
565+
\OCP\Server::get(IConfig::class),
566+
\OCP\Server::get(IGroupManager::class),
567+
\OCP\Server::get(ICacheFactory::class),
568+
\OCP\Server::get(IEventDispatcher::class),
569+
\OCP\Server::get(LoggerInterface::class),
566570
));
567571
}
568572

@@ -621,14 +625,14 @@ public function testParseAppInfo(array $data, array $expected) {
621625

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

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

0 commit comments

Comments
 (0)