Merge pull request #36417 from nextcloud/fix/psr-container

Make the container fully fulfill PSR container interface
nextcloud · Feb 7, 2023 · dcfc96f · dcfc96f
2 parents 6077003 + 3b2d01f
commit dcfc96f
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 6 deletions.
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -720,6 +720,7 @@
     'OC\\AppFramework\\Services\\AppConfig' => $baseDir . '/lib/private/AppFramework/Services/AppConfig.php',
     'OC\\AppFramework\\Services\\InitialState' => $baseDir . '/lib/private/AppFramework/Services/InitialState.php',
     'OC\\AppFramework\\Utility\\ControllerMethodReflector' => $baseDir . '/lib/private/AppFramework/Utility/ControllerMethodReflector.php',
+    'OC\\AppFramework\\Utility\\QueryNotFoundException' => $baseDir . '/lib/private/AppFramework/Utility/QueryNotFoundException.php',
     'OC\\AppFramework\\Utility\\SimpleContainer' => $baseDir . '/lib/private/AppFramework/Utility/SimpleContainer.php',
     'OC\\AppFramework\\Utility\\TimeFactory' => $baseDir . '/lib/private/AppFramework/Utility/TimeFactory.php',
     'OC\\AppScriptDependency' => $baseDir . '/lib/private/AppScriptDependency.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -753,6 +753,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\AppFramework\\Services\\AppConfig' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Services/AppConfig.php',
         'OC\\AppFramework\\Services\\InitialState' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Services/InitialState.php',
         'OC\\AppFramework\\Utility\\ControllerMethodReflector' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/ControllerMethodReflector.php',
+        'OC\\AppFramework\\Utility\\QueryNotFoundException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/QueryNotFoundException.php',
         'OC\\AppFramework\\Utility\\SimpleContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/SimpleContainer.php',
         'OC\\AppFramework\\Utility\\TimeFactory' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Utility/TimeFactory.php',
         'OC\\AppScriptDependency' => __DIR__ . '/../../..' . '/lib/private/AppScriptDependency.php',

diff --git a/lib/private/AppFramework/Utility/QueryNotFoundException.php b/lib/private/AppFramework/Utility/QueryNotFoundException.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023, Ferdinand Thiessen <rpm@fthiessen.de>
+ *
+ * @author Ferdinand Thiessen <rpm@fthiessen.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\AppFramework\Utility;
+
+use OCP\AppFramework\QueryException;
+use Psr\Container\NotFoundExceptionInterface;
+
+/**
+ * Private implementation of the `Psr\Container\NotFoundExceptionInterface`
+ *
+ * QueryNotFoundException is a simple wrapper over the `QueryException`
+ * to fulfill the PSR Container interface.
+ *
+ * You should not catch this class directly but the `NotFoundExceptionInterface`.
+ */
+class QueryNotFoundException extends QueryException implements NotFoundExceptionInterface {
+}
diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php
@@ -127,7 +127,8 @@ public function resolve($name) {
 					' Class can not be instantiated');
 			}
 		} catch (ReflectionException $e) {
-			throw new QueryException($baseMsg . ' ' . $e->getMessage());
+			// Class does not exist
+			throw new QueryNotFoundException($baseMsg . ' ' . $e->getMessage());
 		}
 	}
 
@@ -145,7 +146,7 @@ public function query(string $name, bool $autoload = true) {
 			return $object;
 		}
 
-		throw new QueryException('Could not resolve ' . $name . '!');
+		throw new QueryNotFoundException('Could not resolve ' . $name . '!');
 	}
 
 	/**

diff --git a/lib/public/AppFramework/QueryException.php b/lib/public/AppFramework/QueryException.php
@@ -32,7 +32,7 @@
 /**
  * Class QueryException
  *
- * The class extends `NotFoundExceptionInterface` since 20.0.0
+ * The class extends `ContainerExceptionInterface` since 20.0.0
  *
  * @since 8.1.0
  * @deprecated 20.0.0 catch \Psr\Container\ContainerExceptionInterface

diff --git a/lib/public/IContainer.php b/lib/public/IContainer.php
@@ -33,6 +33,7 @@
 use OCP\AppFramework\QueryException;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\ContainerInterface;
+use Psr\Container\NotFoundExceptionInterface;
 
 /**
  * Class IContainer
@@ -70,6 +71,7 @@ public function resolve($name);
 	 * @return mixed
 	 * @psalm-return ($name is class-string ? T : mixed)
 	 * @throws ContainerExceptionInterface if the query could not be resolved
+	 * @throws NotFoundExceptionInterface if the name could not be found within the container
 	 * @throws QueryException if the query could not be resolved
 	 * @since 6.0.0
 	 * @deprecated 20.0.0 use \Psr\Container\ContainerInterface::get

diff --git a/tests/lib/AppFramework/Utility/SimpleContainerTest.php b/tests/lib/AppFramework/Utility/SimpleContainerTest.php
@@ -26,6 +26,7 @@
 namespace Test\AppFramework\Utility;
 
 use OC\AppFramework\Utility\SimpleContainer;
+use Psr\Container\NotFoundExceptionInterface;
 
 interface TestInterface {
 }
@@ -76,11 +77,31 @@ public function testRegister() {
 	}
 
 
-
+	/**
+	 * Test querying a class that is not registered without autoload enabled
+	 */
 	public function testNothingRegistered() {
-		$this->expectException(\OCP\AppFramework\QueryException::class);
+		try {
+			$this->container->query('something really hard', false);
+			$this->fail('Expected `QueryException` exception was not thrown');
+		} catch (\Throwable $exception) {
+			$this->assertInstanceOf(\OCP\AppFramework\QueryException::class, $exception);
+			$this->assertInstanceOf(NotFoundExceptionInterface::class, $exception);
+		}
+	}
+
 
-		$this->container->query('something really hard');
+	/**
+	 * Test querying a class that is not registered with autoload enabled
+	 */
+	public function testNothingRegistered_autoload() {
+		try {
+			$this->container->query('something really hard');
+			$this->fail('Expected `QueryException` exception was not thrown');
+		} catch (\Throwable $exception) {
+			$this->assertInstanceOf(\OCP\AppFramework\QueryException::class, $exception);
+			$this->assertInstanceOf(NotFoundExceptionInterface::class, $exception);
+		}
 	}