diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php index 1989ef5d4c128..2e4e6a3bf2cf3 100644 --- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php +++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php @@ -11,6 +11,7 @@ use OCA\Provisioning_API\Middleware\Exceptions\NotSubAdminException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\AppFramework\OCS\OCSException; @@ -40,7 +41,7 @@ public function __construct( */ public function beforeController($controller, $methodName) { // If AuthorizedAdminSetting, the check will be done in the SecurityMiddleware - if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { throw new NotSubAdminException(); } } diff --git a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php index c027e518a3d63..6044e2bb9f5d3 100644 --- a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php +++ b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php @@ -53,10 +53,14 @@ public function testBeforeController(bool $subadminRequired, bool $isAdmin, bool ); $this->reflector->method('hasAnnotation') - ->willReturnCallback(function ($annotation) use ($subadminRequired, $hasSettingAuthorizationAnnotation) { + ->willReturnCallback(function ($annotation) use ($subadminRequired) { if ($annotation === 'NoSubAdminRequired') { return !$subadminRequired; } + return false; + }); + $this->reflector->method('hasAnnotationOrAttribute') + ->willReturnCallback(function ($annotation, $attribute) use ($hasSettingAuthorizationAnnotation) { if ($annotation === 'AuthorizedAdminSetting') { return $hasSettingAuthorizationAnnotation; } diff --git a/apps/settings/lib/Middleware/SubadminMiddleware.php b/apps/settings/lib/Middleware/SubadminMiddleware.php index 02d68e138dae4..eb27e27c74cca 100644 --- a/apps/settings/lib/Middleware/SubadminMiddleware.php +++ b/apps/settings/lib/Middleware/SubadminMiddleware.php @@ -14,6 +14,7 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Middleware; use OCP\Group\ISubAdmin; @@ -48,7 +49,7 @@ private function isSubAdmin(): bool { * @throws \Exception */ public function beforeController($controller, $methodName) { - if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { if (!$this->isSubAdmin()) { throw new NotAdminException($this->l10n->t('Logged in account must be a sub admin')); } diff --git a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php index 37cfb5ccc5969..489359c34693e 100644 --- a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php +++ b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php @@ -14,6 +14,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector; use OCA\Settings\Middleware\SubadminMiddleware; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\TemplateResponse; use OCP\Group\ISubAdmin; use OCP\IL10N; @@ -62,11 +63,16 @@ public function testBeforeControllerAsUserWithoutAnnotation(): void { $this->expectException(NotAdminException::class); $this->reflector - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('hasAnnotation') ->willReturnMap([ ['NoSubAdminRequired', false], - ['AuthorizedAdminSetting', false], + ]); + $this->reflector + ->expects($this->exactly(1)) + ->method('hasAnnotationOrAttribute') + ->willReturnMap([ + ['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false], ]); $this->subAdminManager @@ -94,11 +100,16 @@ public function testBeforeControllerWithAnnotation(): void { public function testBeforeControllerAsSubAdminWithoutAnnotation(): void { $this->reflector - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('hasAnnotation') ->willReturnMap([ ['NoSubAdminRequired', false], - ['AuthorizedAdminSetting', false], + ]); + $this->reflector + ->expects($this->exactly(1)) + ->method('hasAnnotationOrAttribute') + ->willReturnMap([ + ['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false], ]); $this->subAdminManager diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index b8805ae77cf25..c8e9ce2ca8b58 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1657,9 +1657,6 @@ - - - @@ -2010,9 +2007,6 @@ - - - @@ -3213,9 +3207,6 @@ - - - diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php index 115a823598367..658a120320e2f 100644 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -18,7 +18,6 @@ use OCP\ISession; use OCP\IUserSession; use Psr\Log\LoggerInterface; -use ReflectionMethod; // Will close the session if the user session is ephemeral. // Happens when the user logs in via the login flow v2. @@ -61,12 +60,7 @@ public function beforeController(Controller $controller, string $methodName) { return; } - $reflectionMethod = new ReflectionMethod($controller, $methodName); - if (!empty($reflectionMethod->getAttributes(PublicPage::class))) { - return; - } - - if ($this->reflector->hasAnnotation('PublicPage')) { + if ($this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class)) { return; } diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 0facbffe504ea..718dabb2c70d3 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -45,9 +45,7 @@ public function __construct( * @throws NotConfirmedException */ public function beforeController(Controller $controller, string $methodName) { - $reflectionMethod = new ReflectionMethod($controller, $methodName); - - if (!$this->needsPasswordConfirmation($reflectionMethod)) { + if (!$this->needsPasswordConfirmation()) { return; } @@ -78,6 +76,7 @@ public function beforeController(Controller $controller, string $methodName) { return; } + $reflectionMethod = new ReflectionMethod($controller, $methodName); if ($this->isPasswordConfirmationStrict($reflectionMethod)) { $authHeader = $this->request->getHeader('Authorization'); if (!str_starts_with(strtolower($authHeader), 'basic ')) { @@ -100,18 +99,8 @@ public function beforeController(Controller $controller, string $methodName) { } } - private function needsPasswordConfirmation(ReflectionMethod $reflectionMethod): bool { - $attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class); - if (!empty($attributes)) { - return true; - } - - if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { - $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . 'PasswordConfirmationRequired' . ' annotation and should use the #[PasswordConfirmationRequired] attribute instead'); - return true; - } - - return false; + private function needsPasswordConfirmation(): bool { + return $this->reflector->hasAnnotationOrAttribute('PasswordConfirmationRequired', PasswordConfirmationRequired::class); } private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod): bool { diff --git a/lib/private/AppFramework/Middleware/SessionMiddleware.php b/lib/private/AppFramework/Middleware/SessionMiddleware.php index b7b0fb118c2d3..84f6c4ddb45d4 100644 --- a/lib/private/AppFramework/Middleware/SessionMiddleware.php +++ b/lib/private/AppFramework/Middleware/SessionMiddleware.php @@ -14,7 +14,6 @@ use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\ISession; -use ReflectionMethod; class SessionMiddleware extends Middleware { /** @var ControllerMethodReflector */ @@ -34,18 +33,7 @@ public function __construct(ControllerMethodReflector $reflector, * @param string $methodName */ public function beforeController($controller, $methodName) { - /** - * Annotation deprecated with Nextcloud 26 - */ - $hasAnnotation = $this->reflector->hasAnnotation('UseSession'); - if ($hasAnnotation) { - $this->session->reopen(); - return; - } - - $reflectionMethod = new ReflectionMethod($controller, $methodName); - $hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class)); - if ($hasAttribute) { + if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) { $this->session->reopen(); } } @@ -57,18 +45,7 @@ public function beforeController($controller, $methodName) { * @return Response */ public function afterController($controller, $methodName, Response $response) { - /** - * Annotation deprecated with Nextcloud 26 - */ - $hasAnnotation = $this->reflector->hasAnnotation('UseSession'); - if ($hasAnnotation) { - $this->session->close(); - return $response; - } - - $reflectionMethod = new ReflectionMethod($controller, $methodName); - $hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class)); - if ($hasAttribute) { + if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) { $this->session->close(); } diff --git a/lib/private/AppFramework/Utility/ControllerMethodReflector.php b/lib/private/AppFramework/Utility/ControllerMethodReflector.php index 568e037d5cd6d..59d5d49b1e882 100644 --- a/lib/private/AppFramework/Utility/ControllerMethodReflector.php +++ b/lib/private/AppFramework/Utility/ControllerMethodReflector.php @@ -9,23 +9,34 @@ namespace OC\AppFramework\Utility; use OCP\AppFramework\Utility\IControllerMethodReflector; +use Psr\Log\LoggerInterface; /** * Reads and parses annotations from doc comments */ class ControllerMethodReflector implements IControllerMethodReflector { - public $annotations = []; - private $types = []; - private $parameters = []; + public array $annotations = []; + private array $types = []; + private array $parameters = []; private array $ranges = []; + private ?\ReflectionMethod $reflectionMethod = null; + + public function __construct( + private readonly LoggerInterface $logger, + ) { + } /** * @param object $object an object or classname * @param string $method the method which we want to inspect */ public function reflect($object, string $method) { - $reflection = new \ReflectionMethod($object, $method); - $docs = $reflection->getDocComment(); + $this->annotations = []; + $this->types = []; + $this->parameters = []; + $this->ranges = []; + $this->reflectionMethod = new \ReflectionMethod($object, $method); + $docs = $this->reflectionMethod->getDocComment(); if ($docs !== false) { // extract everything prefixed by @ and first letter uppercase @@ -64,7 +75,7 @@ public function reflect($object, string $method) { } } - foreach ($reflection->getParameters() as $param) { + foreach ($this->reflectionMethod->getParameters() as $param) { // extract type information from PHP 7 scalar types and prefer them over phpdoc annotations $type = $param->getType(); if ($type instanceof \ReflectionNamedType) { @@ -109,6 +120,24 @@ public function getParameters(): array { return $this->parameters; } + /** + * @template T + * + * @param class-string $attributeClass + */ + public function hasAnnotationOrAttribute(?string $annotationName, string $attributeClass): bool { + if (!empty($this->reflectionMethod->getAttributes($attributeClass))) { + return true; + } + + if ($annotationName && $this->hasAnnotation($annotationName)) { + $this->logger->debug($this->reflectionMethod->getDeclaringClass()->getName() . '::' . $this->reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); + return true; + } + + return false; + } + /** * Check if a method contains an annotation * @param string $name the name of the annotation diff --git a/lib/public/AppFramework/Utility/IControllerMethodReflector.php b/lib/public/AppFramework/Utility/IControllerMethodReflector.php index 95d7fbebb561d..d4e36e310e084 100644 --- a/lib/public/AppFramework/Utility/IControllerMethodReflector.php +++ b/lib/public/AppFramework/Utility/IControllerMethodReflector.php @@ -11,22 +11,13 @@ /** * Interface ControllerMethodReflector * - * Reads and parses annotations from doc comments + * You can inject this interface in your Middleware, and it will be prefilled with information related to the called controller method + * + * Reads and parses annotations from doc comments (deprecated) and PHP attributes * * @since 8.0.0 - * @deprecated 22.0.0 will be obsolete with native attributes in PHP8 - * @see https://help.nextcloud.com/t/how-should-we-use-php8-attributes/104278 */ interface IControllerMethodReflector { - /** - * @param object $object an object or classname - * @param string $method the method which we want to inspect - * @return void - * @since 8.0.0 - * @deprecated 17.0.0 Reflect should not be called multiple times and only be used internally. This will be removed in Nextcloud 18 - */ - public function reflect($object, string $method); - /** * Inspects the PHPDoc parameters for types * @@ -56,4 +47,15 @@ public function getParameters(): array; * @see https://help.nextcloud.com/t/how-should-we-use-php8-attributes/104278 */ public function hasAnnotation(string $name): bool; + + /** + * @template T + * + * Check if a method contains an annotation or an attribute. + * Log a debug line if the annotation is used. + * + * @param class-string $attributeClass + * @since 34.0.0 + */ + public function hasAnnotationOrAttribute(?string $annotationName, string $attributeClass): bool; } diff --git a/tests/lib/AppFramework/Http/DispatcherTest.php b/tests/lib/AppFramework/Http/DispatcherTest.php index 6e431fff08cd0..133458ffb138c 100644 --- a/tests/lib/AppFramework/Http/DispatcherTest.php +++ b/tests/lib/AppFramework/Http/DispatcherTest.php @@ -123,7 +123,7 @@ protected function setUp(): void { $this->request = $this->createMock(Request::class); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->dispatcher = new Dispatcher( $this->http, diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php index 3fd2cb38a33e1..e29691683ee78 100644 --- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php @@ -51,7 +51,7 @@ class BruteForceMiddlewareTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->throttler = $this->createMock(IThrottler::class); $this->request = $this->createMock(IRequest::class); $this->logger = $this->createMock(LoggerInterface::class); diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php index c325ae638fb96..8ffe343effc48 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -36,7 +36,7 @@ class CORSMiddlewareTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->session = $this->createMock(Session::class); $this->throttler = $this->createMock(IThrottler::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -82,6 +82,7 @@ public function testNoAnnotationNoCORSHEADER(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterController($this->controller, __FUNCTION__, new Response()); @@ -303,6 +304,7 @@ public function testAfterExceptionWithSecurityExceptionNoStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception')); @@ -319,6 +321,7 @@ public function testAfterExceptionWithSecurityExceptionWithStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception', 501)); @@ -338,6 +341,7 @@ public function testAfterExceptionWithRegularException(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $middleware->afterException($this->controller, __FUNCTION__, new \Exception('A regular exception')); } diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 90e801ca47174..e0d1c254c9dbf 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -45,7 +45,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { private Manager $userManager; protected function setUp(): void { - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->session = $this->createMock(ISession::class); $this->userSession = $this->createMock(IUserSession::class); $this->user = $this->createMock(IUser::class); diff --git a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php index 358d24fa4ec11..5406ccb7d06ec 100644 --- a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php @@ -76,7 +76,7 @@ protected function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->userSession = $this->createMock(IUserSession::class); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->limiter = $this->createMock(Limiter::class); $this->session = $this->createMock(ISession::class); $this->appConfig = $this->createMock(IAppConfig::class); diff --git a/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php index 7800371f68f33..f86a05069b986 100644 --- a/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php @@ -46,7 +46,8 @@ public function testBeforeControllerIndexHasAnnotation(): void { $this->request->method('getScriptName') ->willReturn('/index.php'); - $this->reflector->method('hasAnnotation') + $this->reflector->expects(self::once()) + ->method('hasAnnotation') ->with('NoSameSiteCookieRequired') ->willReturn(true); @@ -58,7 +59,8 @@ public function testBeforeControllerIndexNoAnnotationPassingCheck(): void { $this->request->method('getScriptName') ->willReturn('/index.php'); - $this->reflector->method('hasAnnotation') + $this->reflector->expects(self::once()) + ->method('hasAnnotation') ->with('NoSameSiteCookieRequired') ->willReturn(false); @@ -75,7 +77,8 @@ public function testBeforeControllerIndexNoAnnotationFailingCheck(): void { $this->request->method('getScriptName') ->willReturn('/index.php'); - $this->reflector->method('hasAnnotation') + $this->reflector->expects(self::once()) + ->method('hasAnnotation') ->with('NoSameSiteCookieRequired') ->willReturn(false); diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 0c6fc21357d9a..b7d5f17cf057f 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -83,7 +83,7 @@ protected function setUp(): void { 'test', $this->request ); - $this->reader = new ControllerMethodReflector(); + $this->reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->logger = $this->createMock(LoggerInterface::class); $this->navigationManager = $this->createMock(INavigationManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); @@ -444,6 +444,7 @@ public function testCsrfOcsController(string $controllerClass, bool $hasOcsApiHe ->willReturn(true); $controller = new $controllerClass('test', $this->request); + $this->reader->reflect($controller, 'foo'); try { $this->middleware->beforeController($controller, 'foo'); diff --git a/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php b/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php index 8ecc73791c902..2e45bea3d0d21 100644 --- a/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php @@ -19,8 +19,8 @@ use Test\TestCase; class SessionMiddlewareTest extends TestCase { - private ControllerMethodReflector|MockObject $reflector; - private ISession|MockObject $session; + private ControllerMethodReflector&MockObject $reflector; + private ISession&MockObject $session; private Controller $controller; private SessionMiddleware $middleware; @@ -50,70 +50,39 @@ public function without() { public function testSessionNotClosedOnBeforeController(): void { $this->configureSessionMock(0, 1); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->beforeController($this->controller, 'withAnnotation'); } - public function testSessionNotClosedOnBeforeControllerWithAttribute(): void { - $this->configureSessionMock(0, 1); - $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(false); - - $this->middleware->beforeController($this->controller, 'withAttribute'); - } - public function testSessionClosedOnAfterController(): void { $this->configureSessionMock(1); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->afterController($this->controller, 'withAnnotation', new Response()); } - public function testSessionClosedOnAfterControllerWithAttribute(): void { - $this->configureSessionMock(1); - $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(true); - - $this->middleware->afterController($this->controller, 'withAttribute', new Response()); - } - public function testSessionReopenedAndClosedOnBeforeController(): void { $this->configureSessionMock(1, 1); $this->reflector->expects(self::exactly(2)) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->beforeController($this->controller, 'withAnnotation'); $this->middleware->afterController($this->controller, 'withAnnotation', new Response()); } - public function testSessionReopenedAndClosedOnBeforeControllerWithAttribute(): void { - $this->configureSessionMock(1, 1); - $this->reflector->expects(self::exactly(2)) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(false); - - $this->middleware->beforeController($this->controller, 'withAttribute'); - $this->middleware->afterController($this->controller, 'withAttribute', new Response()); - } - public function testSessionClosedOnBeforeController(): void { $this->configureSessionMock(0); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(false); $this->middleware->beforeController($this->controller, 'without'); @@ -122,8 +91,8 @@ public function testSessionClosedOnBeforeController(): void { public function testSessionNotClosedOnAfterController(): void { $this->configureSessionMock(0); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(false); $this->middleware->afterController($this->controller, 'without', new Response()); diff --git a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php index c0f0833fb6982..145964cc4edf7 100644 --- a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php +++ b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php @@ -9,6 +9,7 @@ namespace Test\AppFramework\Utility; use OC\AppFramework\Utility\ControllerMethodReflector; +use Psr\Log\LoggerInterface; class BaseController { /** @@ -69,7 +70,7 @@ class ControllerMethodReflectorTest extends \Test\TestCase { * @Annotation */ public function testReadAnnotation(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadAnnotation' @@ -82,7 +83,7 @@ public function testReadAnnotation(): void { * @Annotation(parameter=value) */ public function testGetAnnotationParameterSingle(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( self::class, __FUNCTION__ @@ -95,7 +96,7 @@ public function testGetAnnotationParameterSingle(): void { * @Annotation(parameter1=value1, parameter2=value2,parameter3=value3) */ public function testGetAnnotationParameterMultiple(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( self::class, __FUNCTION__ @@ -111,7 +112,7 @@ public function testGetAnnotationParameterMultiple(): void { * @param test */ public function testReadAnnotationNoLowercase(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadAnnotationNoLowercase' @@ -127,7 +128,7 @@ public function testReadAnnotationNoLowercase(): void { * @param int $test */ public function testReadTypeIntAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeIntAnnotations' @@ -148,7 +149,7 @@ public function arguments3($a, float $b, int $c, $d) { * @requires PHP 7 */ public function testReadTypeIntAnnotationsScalarTypes(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments3' @@ -166,7 +167,7 @@ public function testReadTypeIntAnnotationsScalarTypes(): void { * @param double $test something special */ public function testReadTypeDoubleAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeDoubleAnnotations' @@ -180,7 +181,7 @@ public function testReadTypeDoubleAnnotations(): void { * @param string $foo */ public function testReadTypeWhitespaceAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeWhitespaceAnnotations' @@ -193,7 +194,7 @@ public function testReadTypeWhitespaceAnnotations(): void { public function arguments($arg, $arg2 = 'hi') { } public function testReflectParameters(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments' @@ -206,7 +207,7 @@ public function testReflectParameters(): void { public function arguments2($arg) { } public function testReflectParameters2(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments2' @@ -217,7 +218,7 @@ public function testReflectParameters2(): void { public function testInheritance(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test'); $this->assertTrue($reader->hasAnnotation('Annotation')); @@ -225,7 +226,7 @@ public function testInheritance(): void { public function testInheritanceOverride(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test2'); $this->assertTrue($reader->hasAnnotation('NoAnnotation')); @@ -234,14 +235,14 @@ public function testInheritanceOverride(): void { public function testInheritanceOverrideNoDocblock(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test3'); $this->assertFalse($reader->hasAnnotation('Annotation')); } public function testRangeDetectionPsalm(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test4'); $rangeInfo1 = $reader->getRange('rangedOne'); @@ -262,7 +263,7 @@ public function testRangeDetectionPsalm(): void { } public function testRangeDetectionNative(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test5'); $rangeInfo1 = $reader->getRange('rangedOne');