From b7c15949ce854bc91edec124478f1aa1ca2f690a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 31 Jul 2025 15:14:48 +0200 Subject: [PATCH] chore: Get rid of AppLocator helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Integrity/CheckApp.php | 5 ++- lib/private/App/AppManager.php | 18 +++++++--- lib/private/DB/MigrationService.php | 17 ++++++---- lib/private/Installer.php | 1 - lib/private/IntegrityCheck/Checker.php | 6 ++-- .../IntegrityCheck/Helpers/AppLocator.php | 33 ------------------ lib/private/Server.php | 2 -- tests/lib/DB/MigrationsTest.php | 7 ++-- tests/lib/IntegrityCheck/CheckerTest.php | 24 +++++-------- .../IntegrityCheck/Helpers/AppLocatorTest.php | 34 ------------------- 10 files changed, 41 insertions(+), 106 deletions(-) delete mode 100644 lib/private/IntegrityCheck/Helpers/AppLocator.php delete mode 100644 tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php diff --git a/core/Command/Integrity/CheckApp.php b/core/Command/Integrity/CheckApp.php index 0145a3f8070..df9e9973aa7 100644 --- a/core/Command/Integrity/CheckApp.php +++ b/core/Command/Integrity/CheckApp.php @@ -5,11 +5,11 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OC\Core\Command\Integrity; use OC\Core\Command\Base; use OC\IntegrityCheck\Checker; -use OC\IntegrityCheck\Helpers\AppLocator; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OCP\App\IAppManager; use Symfony\Component\Console\Input\InputArgument; @@ -25,7 +25,6 @@ use Symfony\Component\Console\Output\OutputInterface; class CheckApp extends Base { public function __construct( private Checker $checker, - private AppLocator $appLocator, private FileAccessHelper $fileAccessHelper, private IAppManager $appManager, ) { @@ -70,7 +69,7 @@ class CheckApp extends Base { foreach ($appIds as $appId) { $path = (string)$input->getOption('path'); if ($path === '') { - $path = $this->appLocator->getAppPath($appId); + $path = $this->appManager->getAppPath($appId); } if ($this->appManager->isShipped($appId) || $this->fileAccessHelper->file_exists($path . '/appinfo/signature.json')) { diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 6fb64610b48..c5ca98436af 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -252,9 +252,14 @@ class AppManager implements IAppManager { foreach ($apps as $app) { // If the app is already loaded then autoloading it makes no sense if (!$this->isAppLoaded($app)) { - $path = \OC_App::getAppPath($app); - if ($path !== false) { + try { + $path = $this->getAppPath($app); \OC_App::registerAutoloading($app, $path); + } catch (AppPathNotFoundException $e) { + $this->logger->info('Error during app loading: ' . $e->getMessage(), [ + 'exception' => $e, + 'app' => $app, + ]); } } } @@ -450,8 +455,13 @@ class AppManager implements IAppManager { return; } $this->loadedApps[$app] = true; - $appPath = \OC_App::getAppPath($app); - if ($appPath === false) { + try { + $appPath = $this->getAppPath($app); + } catch (AppPathNotFoundException $e) { + $this->logger->info('Error during app loading: ' . $e->getMessage(), [ + 'exception' => $e, + 'app' => $app, + ]); return; } $eventLogger = \OC::$server->get(IEventLogger::class); diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 40579c7a898..e1723ff09a9 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -13,8 +13,8 @@ use Doctrine\DBAL\Schema\SchemaException; use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; use OC\App\InfoParser; -use OC\IntegrityCheck\Helpers\AppLocator; use OC\Migration\SimpleOutput; +use OCP\App\IAppManager; use OCP\AppFramework\App; use OCP\AppFramework\QueryException; use OCP\DB\ISchemaWrapper; @@ -39,7 +39,12 @@ class MigrationService { /** * @throws \Exception */ - public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null, ?LoggerInterface $logger = null) { + public function __construct( + string $appName, + Connection $connection, + ?IOutput $output = null, + ?LoggerInterface $logger = null, + ) { $this->appName = $appName; $this->connection = $connection; if ($logger === null) { @@ -58,10 +63,8 @@ class MigrationService { $this->migrationsNamespace = 'OC\\Core\\Migrations'; $this->checkOracle = true; } else { - if ($appLocator === null) { - $appLocator = new AppLocator(); - } - $appPath = $appLocator->getAppPath($appName); + $appManager = Server::get(IAppManager::class); + $appPath = $appManager->getAppPath($appName); $namespace = App::buildAppNamespace($appName); $this->migrationsPath = "$appPath/lib/Migration"; $this->migrationsNamespace = $namespace . '\\Migration'; @@ -728,7 +731,7 @@ class MigrationService { } } - private function ensureMigrationsAreLoaded() { + private function ensureMigrationsAreLoaded(): void { if (empty($this->migrations)) { $this->migrations = $this->findMigrations(); } diff --git a/lib/private/Installer.php b/lib/private/Installer.php index c303c1b6cfb..bb437b49db9 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -18,7 +18,6 @@ use OC\Archive\TAR; use OC\DB\Connection; use OC\DB\MigrationService; use OC\Files\FilenameValidator; -use OC_App; use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\BackgroundJob\IJobList; diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php index 2bd6e426b79..e98f77acaa0 100644 --- a/lib/private/IntegrityCheck/Checker.php +++ b/lib/private/IntegrityCheck/Checker.php @@ -10,7 +10,6 @@ namespace OC\IntegrityCheck; use OC\Core\Command\Maintenance\Mimetype\GenerateMimetypeFileBuilder; use OC\IntegrityCheck\Exceptions\InvalidSignatureException; -use OC\IntegrityCheck\Helpers\AppLocator; use OC\IntegrityCheck\Helpers\EnvironmentHelper; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OC\IntegrityCheck\Iterator\ExcludeFileByNameFilterIterator; @@ -44,7 +43,6 @@ class Checker { private ServerVersion $serverVersion, private EnvironmentHelper $environmentHelper, private FileAccessHelper $fileAccessHelper, - private AppLocator $appLocator, private ?IConfig $config, private ?IAppConfig $appConfig, ICacheFactory $cacheFactory, @@ -460,7 +458,7 @@ class Checker { public function verifyAppSignature(string $appId, string $path = '', bool $forceVerify = false): array { try { if ($path === '') { - $path = $this->appLocator->getAppPath($appId); + $path = $this->appManager->getAppPath($appId); } $result = $this->verify( $path . '/appinfo/signature.json', @@ -545,7 +543,7 @@ class Checker { $appNeedsToBeChecked = false; if ($isShipped) { $appNeedsToBeChecked = true; - } elseif ($this->fileAccessHelper->file_exists($this->appLocator->getAppPath($appId) . '/appinfo/signature.json')) { + } elseif ($this->fileAccessHelper->file_exists($this->appManager->getAppPath($appId) . '/appinfo/signature.json')) { // Otherwise only if the application explicitly ships a signature.json file $appNeedsToBeChecked = true; } diff --git a/lib/private/IntegrityCheck/Helpers/AppLocator.php b/lib/private/IntegrityCheck/Helpers/AppLocator.php deleted file mode 100644 index 148a3aeda76..00000000000 --- a/lib/private/IntegrityCheck/Helpers/AppLocator.php +++ /dev/null @@ -1,33 +0,0 @@ -get(ServerVersion::class), $c->get(EnvironmentHelper::class), new FileAccessHelper(), - new AppLocator(), $config, $appConfig, $c->get(ICacheFactory::class), diff --git a/tests/lib/DB/MigrationsTest.php b/tests/lib/DB/MigrationsTest.php index 2b39b26d852..2116678baa1 100644 --- a/tests/lib/DB/MigrationsTest.php +++ b/tests/lib/DB/MigrationsTest.php @@ -20,6 +20,7 @@ use OC\DB\Connection; use OC\DB\MigrationService; use OC\DB\SchemaWrapper; use OC\Migration\MetadataManager; +use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\IDBConnection; use OCP\Migration\Attributes\AddColumn; @@ -81,10 +82,10 @@ class MigrationsTest extends \Test\TestCase { public function testUnknownApp(): void { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('App not found'); + $this->expectException(AppPathNotFoundException::class); + $this->expectExceptionMessage('Could not find path for unknown_bloody_app'); - $migrationService = new MigrationService('unknown-bloody-app', $this->db); + $migrationService = new MigrationService('unknown_bloody_app', $this->db); } diff --git a/tests/lib/IntegrityCheck/CheckerTest.php b/tests/lib/IntegrityCheck/CheckerTest.php index a8a2596a3d8..823258cab10 100644 --- a/tests/lib/IntegrityCheck/CheckerTest.php +++ b/tests/lib/IntegrityCheck/CheckerTest.php @@ -11,7 +11,6 @@ namespace Test\IntegrityCheck; use OC\Core\Command\Maintenance\Mimetype\GenerateMimetypeFileBuilder; use OC\Files\Type\Detection; use OC\IntegrityCheck\Checker; -use OC\IntegrityCheck\Helpers\AppLocator; use OC\IntegrityCheck\Helpers\EnvironmentHelper; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OC\Memcache\NullCache; @@ -29,10 +28,6 @@ class CheckerTest extends TestCase { private $serverVersion; /** @var EnvironmentHelper|\PHPUnit\Framework\MockObject\MockObject */ private $environmentHelper; - /** @var AppLocator|\PHPUnit\Framework\MockObject\MockObject */ - private $appLocator; - /** @var Checker */ - private $checker; /** @var FileAccessHelper|\PHPUnit\Framework\MockObject\MockObject */ private $fileAccessHelper; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ @@ -46,12 +41,13 @@ class CheckerTest extends TestCase { /** @var \OC\Files\Type\Detection|\PHPUnit\Framework\MockObject\MockObject */ private $mimeTypeDetector; + private Checker $checker; + protected function setUp(): void { parent::setUp(); $this->serverVersion = $this->createMock(ServerVersion::class); $this->environmentHelper = $this->createMock(EnvironmentHelper::class); $this->fileAccessHelper = $this->createMock(FileAccessHelper::class); - $this->appLocator = $this->createMock(AppLocator::class); $this->config = $this->createMock(IConfig::class); $this->appConfig = $this->createMock(IAppConfig::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); @@ -71,7 +67,6 @@ class CheckerTest extends TestCase { $this->serverVersion, $this->environmentHelper, $this->fileAccessHelper, - $this->appLocator, $this->config, $this->appConfig, $this->cacheFactory, @@ -186,7 +181,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->once()) ->method('getAppPath') ->with('SomeApp') @@ -221,7 +216,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->once()) ->method('getAppPath') ->with('SomeApp') @@ -262,7 +257,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->once()) ->method('getAppPath') ->with('SomeApp') @@ -319,7 +314,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->never()) ->method('getAppPath') ->with('SomeApp'); @@ -374,7 +369,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->once()) ->method('getAppPath') ->with('SomeApp') @@ -415,7 +410,7 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->willReturn(false); - $this->appLocator + $this->appManager ->expects($this->once()) ->method('getAppPath') ->with('SomeApp') @@ -984,7 +979,6 @@ class CheckerTest extends TestCase { $this->serverVersion, $this->environmentHelper, $this->fileAccessHelper, - $this->appLocator, $this->config, $this->appConfig, $this->cacheFactory, @@ -1032,7 +1026,7 @@ class CheckerTest extends TestCase { $this->assertSame($expected, $app); return []; }); - $this->appLocator + $this->appManager ->expects($this->exactly(2)) ->method('getAppPath') ->willReturnMap([ diff --git a/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php b/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php deleted file mode 100644 index 837b397ac1f..00000000000 --- a/tests/lib/IntegrityCheck/Helpers/AppLocatorTest.php +++ /dev/null @@ -1,34 +0,0 @@ -locator = new AppLocator(); - } - - public function testGetAppPath(): void { - $this->assertSame(\OC_App::getAppPath('files'), $this->locator->getAppPath('files')); - } - - - public function testGetAppPathNotExistentApp(): void { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('App not found'); - - $this->locator->getAppPath('aTotallyNotExistingApp'); - } -}