feature symfony#53191 [HttpKernel] Allow #[WithHttpStatus] and #[WithLogLevel] to take effect on interfaces (priyadi)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpKernel] Allow `#[WithHttpStatus]` and `#[WithLogLevel]` to take effect on interfaces

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        |
| License       | MIT

This change lets adding `#[WithHttpStatus]` and `#[WithLogLevel]` to an interface, and exceptions implementing such interface will behave as if `#[WithHttpStatus]` and `#[WithLogLevel]` is attached to itself.

`#[WithHttpStatus]` and `#[WithLogLevel]` were created to have configuration like `config/packages/exceptions.yaml` but using attributes: https://symfony.com/blog/new-in-symfony-6-3-http-exception-attributes

However, unlike `exceptions.yaml`, `#[WithHttpStatus]` and `#[WithLogLevel]` did not previously work with interfaces. Thus, the two ways of configuring have slightly different semantics. This PR fixes the issue.

Commits
-------

19de235 [HttpKernel] Allow `#[WithHttpStatus]` and `#[WithLogLevel]` to take effect on interfaces

Author: nicolas-grekas

src/Symfony/Component/HttpKernel/EventListener/ErrorListener.php

@@ -70,19 +70,9 @@ public function logKernelException(ExceptionEvent $event): void
         }
 
         // There's no specific status code defined in the configuration for this exception
-        if (!$throwable instanceof HttpExceptionInterface) {
-            $class = new \ReflectionClass($throwable);
-
-            do {
-                if ($attributes = $class->getAttributes(WithHttpStatus::class, \ReflectionAttribute::IS_INSTANCEOF)) {
-                    /** @var WithHttpStatus $instance */
-                    $instance = $attributes[0]->newInstance();
-
-                    $throwable = HttpException::fromStatusCode($instance->statusCode, $throwable->getMessage(), $throwable, $instance->headers);
-                    $event->setThrowable($throwable);
-                    break;
-                }
-            } while ($class = $class->getParentClass());
+        if (!$throwable instanceof HttpExceptionInterface && $withHttpStatus = $this->getInheritedAttribute($throwable::class, WithHttpStatus::class)) {
+            $throwable = HttpException::fromStatusCode($withHttpStatus->statusCode, $throwable->getMessage(), $throwable, $withHttpStatus->headers);
+            $event->setThrowable($throwable);
         }
 
         $e = FlattenException::createFromThrowable($throwable);
@@ -200,16 +190,9 @@ private function resolveLogLevel(\Throwable $throwable): string
             }
         }
 
-        $class = new \ReflectionClass($throwable);
-
-        do {
-            if ($attributes = $class->getAttributes(WithLogLevel::class)) {
-                /** @var WithLogLevel $instance */
-                $instance = $attributes[0]->newInstance();
-
-                return $instance->level;
-            }
-        } while ($class = $class->getParentClass());
+        if ($withLogLevel = $this->getInheritedAttribute($throwable::class, WithLogLevel::class)) {
+            return $withLogLevel->level;
+        }
 
         if (!$throwable instanceof HttpExceptionInterface || $throwable->getStatusCode() >= 500) {
             return LogLevel::CRITICAL;
@@ -233,4 +216,45 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
 
         return $request;
     }
+
+    /**
+     * @template T
+     *
+     * @param class-string<T> $attribute
+     *
+     * @return T|null
+     */
+    private function getInheritedAttribute(string $class, string $attribute): ?object
+    {
+        $class = new \ReflectionClass($class);
+        $interfaces = [];
+        $attributeReflector = null;
+        $parentInterfaces = [];
+        $ownInterfaces = [];
+
+        do {
+            if ($attributes = $class->getAttributes($attribute, \ReflectionAttribute::IS_INSTANCEOF)) {
+                $attributeReflector = $attributes[0];
+                $parentInterfaces = class_implements($class->name);
+                break;
+            }
+
+            $interfaces[] = class_implements($class->name);
+        } while ($class = $class->getParentClass());
+
+        while ($interfaces) {
+            $ownInterfaces = array_diff_key(array_pop($interfaces), $parentInterfaces);
+            $parentInterfaces += $ownInterfaces;
+
+            foreach ($ownInterfaces as $interface) {
+                $class = new \ReflectionClass($interface);
+
+                if ($attributes = $class->getAttributes($attribute, \ReflectionAttribute::IS_INSTANCEOF)) {
+                    $attributeReflector = $attributes[0];
+                }
+            }
+        }
+
+        return $attributeReflector?->newInstance();
+    }
 }

src/Symfony/Component/HttpKernel/Tests/EventListener/ErrorListenerTest.php

@@ -139,6 +139,21 @@ public function testHandleWithLogLevelAttribute()
         $this->assertCount(1, $logger->getLogs('warning'));
     }
 
+    public function testHandleClassImplementingInterfaceWithLogLevelAttribute()
+    {
+        $request = new Request();
+        $event = new ExceptionEvent(new TestKernel(), $request, HttpKernelInterface::MAIN_REQUEST, new ImplementingInterfaceWithLogLevelAttribute());
+        $logger = new TestLogger();
+        $l = new ErrorListener('not used', $logger);
+
+        $l->logKernelException($event);
+        $l->onKernelException($event);
+
+        $this->assertEquals(0, $logger->countErrors());
+        $this->assertCount(0, $logger->getLogs('critical'));
+        $this->assertCount(1, $logger->getLogs('warning'));
+    }
+
     public function testHandleWithLogLevelAttributeAndCustomConfiguration()
     {
         $request = new Request();
@@ -298,6 +313,7 @@ public static function exceptionWithAttributeProvider()
         yield [new WithCustomUserProvidedAttribute(), 208, ['name' => 'value']];
         yield [new WithGeneralAttribute(), 412, ['some' => 'thing']];
         yield [new ChildOfWithGeneralAttribute(), 412, ['some' => 'thing']];
+        yield [new ImplementingInterfaceWithGeneralAttribute(), 412, ['some' => 'thing']];
     }
 }
 
@@ -359,6 +375,20 @@ class WithGeneralAttribute extends \Exception
 {
 }
 
+#[WithHttpStatus(
+    statusCode: Response::HTTP_PRECONDITION_FAILED,
+    headers: [
+        'some' => 'thing',
+    ]
+)]
+interface InterfaceWithGeneralAttribute
+{
+}
+
+class ImplementingInterfaceWithGeneralAttribute extends \Exception implements InterfaceWithGeneralAttribute
+{
+}
+
 class ChildOfWithGeneralAttribute extends WithGeneralAttribute
 {
 }
@@ -371,3 +401,12 @@ class WarningWithLogLevelAttribute extends \Exception
 class ChildOfWarningWithLogLevelAttribute extends WarningWithLogLevelAttribute
 {
 }
+
+#[WithLogLevel(LogLevel::WARNING)]
+interface InterfaceWithLogLevelAttribute
+{
+}
+
+class ImplementingInterfaceWithLogLevelAttribute extends \Exception implements InterfaceWithLogLevelAttribute
+{
+}