10000 feature #53191 [HttpKernel] Allow `#[WithHttpStatus]` and `#[WithLogL… · symfony/symfony@5c25195 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5c25195

Browse files
nicolas-grekasnicolas-grekas
committed
feature #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
2 parents 1d35045 + 19de235 commit 5c25195

File tree

2 files changed

+86
-23
lines changed

2 files changed

+86
-23
lines changed

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

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,9 @@ public function logKernelException(ExceptionEvent $event): void
7070
}
7171

7272
// There's no specific status code defined in the configuration for this exception
73-
if (!$throwable instanceof HttpExceptionInterface) {
74-
$class = new \ReflectionClass($throwable);
75-
76-
do {
77-
if ($attributes = $class->getAttributes(WithHttpStatus::class, \ReflectionAttribute::IS_INSTANCEOF)) {
78-
/** @var WithHttpStatus $instance */
79-
$instance = $attributes[0]->newInstance();
80-
81-
$throwable = HttpException::fromStatusCode($instance->statusCode, $throwable->getMessage(), $throwable, $instance->headers);
82-
$event->setThrowable($throwable);
83-
break;
84-
}
85-
} while ($class = $class->getParentClass());
73+
if (!$throwable instanceof HttpExceptionInterface && $withHttpStatus = $this->getInheritedAttribute($throwable::class, WithHttpStatus::class)) {
74+
$throwable = HttpException::fromStatusCode($withHttpStatus->statusCode, $throwable->getMessage(), $throwable, $withHttpStatus->headers);
75+
$event->setThrowable($throwable);
8676
}
8777

8878
$e = FlattenException::createFromThrowable($throwable);
@@ -200,16 +190,9 @@ private function resolveLogLevel(\Throwable $throwable): string
200190
}
201191
}
202192

203-
$class = new \ReflectionClass($throwable);
204-
205-
do {
206-
if ($attributes = $class->getAttributes(WithLogLevel::class)) {
207-
/** @var WithLogLevel $instance */
208-
$instance = $attributes[0]->newInstance();
209-
210-
return $instance->level;
211-
}
212-
} while ($class = $class->getParentClass());
193+
if ($withLogLevel = $this->getInheritedAttribute($throwable::class, WithLogLevel::class)) {
194+
return $withLogLevel->level;
195+
}
213196

214197
if (!$throwable instanceof HttpExceptionInterface || $throwable->getStatusCode() >= 500) {
215198
return LogLevel::CRITICAL;
@@ -233,4 +216,45 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
233216

234217
return $request;
235218
}
219+
220+
/**
221+
* @template T
222+
*
223+
* @param class-string<T> $attribute
224+
*
225+
* @return T|null
226+
*/
227+
private function getInheritedAttribute(string $class, string $attribute): ?object
228+
{
229+
$class = new \ReflectionClass($class);
230+
$interfaces = [];
231+
$attributeReflector = null;
232+
$parentInterfaces = [];
233+
$ownInterfaces = [];
234+
235+
do {
236+
if ($attributes = $class->getAttributes($attribute, \ReflectionAttribute::IS_INSTANCEOF)) {
237+
$attributeReflector = $attributes[0];
238+
$parentInterfaces = class_implements($class->name);
239+
break;
240+
}
241+
242+
$interfaces[] = class_implements($class->name);
243+
} while ($class = $class->getParentClass());
244+
245+
while ($interfaces) {
246+
$ownInterfaces = array_diff_key(array_pop($interfaces), $parentInterfaces);
247+
$parentInterfaces += $ownInterfaces;
248+
249+
foreach ($ownInterfaces as $interface) {
250+
$class = new \ReflectionClass($interface);
251+
252+
if ($attributes = $class->getAttributes($attribute, \ReflectionAttribute::IS_INSTANCEOF)) {
253+
$attributeReflector = $attributes[0];
254+
}
255+
}
256+
}
257+
258+
return $attributeReflector?->newInstance();
259+
}
236260
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,21 @@ public function testHandleWithLogLevelAttribute()
139139
$this->assertCount(1, $logger->getLogs('warning'));
140140
}
141141

142+
public function testHandleClassImplementingInterfaceWithLogLevelAttribute()
143+
{
144+
$request = new Request();
145+
$event = new ExceptionEvent(new TestKernel(), $request, HttpKernelInterface::MAIN_REQUEST, new ImplementingInterfaceWithLogLevelAttribute());
146+
$logger = new TestLogger();
147+
$l = new ErrorListener('not used', $logger);
148+
149+
$l->logKernelException($event);
150+
$l->onKernelException($event);
151+
152+
$this->assertEquals(0, $logger->countErrors());
153+
$this->assertCount(0, $logger->getLogs('critical'));
154+
$this->assertCount(1, $logger->getLogs('warning'));
155+
}
156+
142157
public function testHandleWithLogLevelAttributeAndCustomConfiguration()
143158
{
144159
$request = new Request();
@@ -298,6 +313,7 @@ public static function exceptionWithAttributeProvider()
298313
yield [new WithCustomUserProvidedAttribute(), 208, ['name' => 'value']];
299314
yield [new WithGeneralAttribute(), 412, ['some' => 'thing']];
300315
yield [new ChildOfWithGeneralAttribute(), 412, ['some' => 'thing']];
316+
yield [new ImplementingInterfaceWithGeneralAttribute(), 412, ['some' => 'thing']];
301317
}
302318
}
303319

@@ -359,6 +375,20 @@ class WithGeneralAttribute extends \Exception
359375
{
360376
}
361377

378+
#[WithHttpStatus(
379+
statusCode: Response::HTTP_PRECONDITION_FAILED,
380+
headers: [
381+
'some' => 'thing',
382+
]
383+
)]
384+
interface InterfaceWithGeneralAttribute
385+
{
386+
}
387+
388+
class ImplementingInterfaceWithGeneralAttribute extends \Exception implements InterfaceWithGeneralAttribute
389+
{
390+
}
391+
362392
class ChildOfWithGeneralAttribute extends WithGeneralAttribute
363393
{
364394
}
@@ -371,3 +401,12 @@ class WarningWithLogLevelAttribute extends \Exception
371401
class ChildOfWarningWithLogLevelAttribute extends WarningWithLogLevelAttribute
372402
{
373403
}
404+
405+
#[WithLogLevel(LogLevel::WARNING)]
406+
interface InterfaceWithLogLevelAttribute
407+
{
408+
}
409+
410+
class ImplementingInterfaceWithLogLevelAttribute extends \Exception implements InterfaceWithLogLevelAttribute
411+
{
412+
}

0 commit comments

Comments
 (0)
0