chore: Get rid of AppLocator helper

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

Author: come-nc

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 @@
 class CheckApp extends Base {
 	public function __construct(
 		private Checker $checker,
-		private AppLocator $appLocator,
 		private FileAccessHelper $fileAccessHelper,
 		private IAppManager $appManager,
 	) {
@@ -70,7 +69,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		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')) {

lib/private/App/AppManager.php

@@ -252,9 +252,14 @@ public function loadApps(array $types = []): bool {
 		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 @@ public function loadApp(string $app): void {
 			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);

lib/private/DB/MigrationService.php

@@ -13,8 +13,8 @@
 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 @@ public function __construct(string $appName, Connection $connection, ?IOutput $o
 			$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 @@ protected function logErrorOrWarning(string $log): void {
 		}
 	}
 
-	private function ensureMigrationsAreLoaded() {
+	private function ensureMigrationsAreLoaded(): void {
 		if (empty($this->migrations)) {
 			$this->migrations = $this->findMigrations();
 		}

lib/private/Installer.php

@@ -18,7 +18,6 @@
 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;

lib/private/IntegrityCheck/Checker.php

@@ -10,7 +10,6 @@
 
 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 @@ public function __construct(
 		private ServerVersion $serverVersion,
 		private EnvironmentHelper $environmentHelper,
 		private FileAccessHelper $fileAccessHelper,
-		private AppLocator $appLocator,
 		private ?IConfig $config,
 		private ?IAppConfig $appConfig,
 		ICacheFactory $cacheFactory,
@@ -460,7 +458,7 @@ private function cleanResults() {
 	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 @@ public function runInstanceVerification() {
 			$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;
 			}

lib/private/IntegrityCheck/Helpers/AppLocator.php

@@ -65,7 +65,6 @@
 use OC\Http\Client\ClientService;
 use OC\Http\Client\NegativeDnsCache;
 use OC\IntegrityCheck\Checker;
-use OC\IntegrityCheck\Helpers\AppLocator;
 use OC\IntegrityCheck\Helpers\EnvironmentHelper;
 use OC\IntegrityCheck\Helpers\FileAccessHelper;
 use OC\KnownUser\KnownUserService;
@@ -839,7 +838,6 @@ public function __construct($webRoot, \OC\Config $config) {
 				$c->get(ServerVersion::class),
 				$c->get(EnvironmentHelper::class),
 				new FileAccessHelper(),
-				new AppLocator(),
 				$config,
 				$appConfig,
 				$c->get(ICacheFactory::class),

lib/private/Server.php

@@ -20,6 +20,7 @@
 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 @@ public function testExecuteUnknownStep(): void {
 
 
 	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);
 	}
 
 

tests/lib/DB/MigrationsTest.php

@@ -11,7 +11,6 @@
 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 @@ protected function setUp(): void {
 			$this->serverVersion,
 			$this->environmentHelper,
 			$this->fileAccessHelper,
-			$this->appLocator,
 			$this->config,
 			$this->appConfig,
 			$this->cacheFactory,
@@ -186,7 +181,7 @@ public function testVerifyAppSignatureWithValidSignatureData(): void {
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
 			->method('getAppPath')
 			->with('SomeApp')
@@ -221,7 +216,7 @@ public function testVerifyAppSignatureWithTamperedSignatureData(): void {
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
 			->method('getAppPath')
 			->with('SomeApp')
@@ -262,7 +257,7 @@ public function testVerifyAppSignatureWithTamperedFiles(): void {
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
 			->method('getAppPath')
 			->with('SomeApp')
@@ -319,7 +314,7 @@ public function testVerifyAppSignatureWithTamperedFilesAndAlternatePath(): void
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->never())
 			->method('getAppPath')
 			->with('SomeApp');
@@ -374,7 +369,7 @@ public function testVerifyAppWithDifferentScope(): void {
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
 			->method('getAppPath')
 			->with('SomeApp')
@@ -415,7 +410,7 @@ public function testVerifyAppWithDifferentScopeAndAlwaysTrustedCore(): void {
 			->with('integrity.check.disabled', false)
 			->willReturn(false);
 
-		$this->appLocator
+		$this->appManager
 			->expects($this->once())
 			->method('getAppPath')
 			->with('SomeApp')
@@ -984,7 +979,6 @@ public function testRunInstanceVerification(): void {
 				$this->serverVersion,
 				$this->environmentHelper,
 				$this->fileAccessHelper,
-				$this->appLocator,
 				$this->config,
 				$this->appConfig,
 				$this->cacheFactory,
@@ -1032,7 +1026,7 @@ public function testRunInstanceVerification(): void {
 				$this->assertSame($expected, $app);
 				return [];
 			});
-		$this->appLocator
+		$this->appManager
 			->expects($this->exactly(2))
 			->method('getAppPath')
 			->willReturnMap([