Skip to content

Commit

Permalink
remove annotation reader from FilteredRouteCollectionBuilder
Browse files Browse the repository at this point in the history
  • Loading branch information
@DjordyKoert
DjordyKoert committed Oct 25, 2024
1 parent de2984a commit aa59bb4
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 186 deletions.
3 changes: 2 additions & 1 deletion UPGRADE-5.0.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ public int $negative;
...
```

This causes the following breaking changes in classes that used on annotations:
This causes the following breaking changes in classes that used annotations:
- BC BREAK: Removed 3rd parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\Describer\OpenApiPhpDescriber::__construct()`
- BC BREAK: Removed 2nd parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\ModelDescriber\FormModelDescriber::__construct()`
- BC BREAK: Removed 2nd parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\ModelDescriber\JMSModelDescriber::__construct()`
- BC BREAK: Removed 2nd parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\ModelDescriber\ObjectModelDescriber::__construct()`
- BC BREAK: Removed 1st parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\RouteDescriber\FosRestDescriber::__construct()`
- BC BREAK: Removed 1st parameter `?Reader $annotationReader` from `Nelmio\ApiDocBundle\Routing\FilteredRouteCollectionBuilder::__construct()`
2 changes: 0 additions & 2 deletions src/DependencyInjection/NelmioApiDocExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
Expand Down Expand Up @@ -143,7 +142,6 @@ public function load(array $configs, ContainerBuilder $container): void
(new Definition(FilteredRouteCollectionBuilder::class))
->setArguments(
[
new Reference('annotation_reader', ContainerInterface::NULL_ON_INVALID_REFERENCE), // Here we use the cached version as we don't deal with @OA annotations in this service
new Reference('nelmio_api_doc.controller_reflector'),
$area,
$areaConfig,
Expand Down
37 changes: 6 additions & 31 deletions src/Routing/FilteredRouteCollectionBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Nelmio\ApiDocBundle\Routing;

use Doctrine\Common\Annotations\Reader;
use Nelmio\ApiDocBundle\Annotation\Areas;
use Nelmio\ApiDocBundle\Util\ControllerReflector;
use OpenApi\Annotations\AbstractAnnotation;
Expand All @@ -21,8 +20,6 @@

final class FilteredRouteCollectionBuilder
{
private ?Reader $annotationReader;

private ControllerReflector $controllerReflector;

private string $area;
Expand All @@ -36,7 +33,6 @@ final class FilteredRouteCollectionBuilder
* @param array<mixed> $options
*/
public function __construct(
?Reader $annotationReader,
ControllerReflector $controllerReflector,
string $area,
array $options = []
Expand Down Expand Up @@ -64,7 +60,6 @@ public function __construct(
$options = $normalizedOptions;
}

$this->annotationReader = $annotationReader;
$this->controllerReflector = $controllerReflector;
$this->area = $area;
$this->options = $resolver->resolve($options);
Expand Down Expand Up @@ -132,27 +127,11 @@ private function matchAnnotation(Route $route): bool
return false;
}

/** @var Areas|null $areas */
$areas = $this->getAttributesAsAnnotation($reflectionMethod, Areas::class)[0] ?? null;

if (null === $areas) {
/** @var Areas|null $areas */
$areas = $this->getAttributesAsAnnotation($reflectionMethod->getDeclaringClass(), Areas::class)[0] ?? null;
$areas = $this->getAttributesAsAnnotation($reflectionMethod, Areas::class)[0]
?? $this->getAttributesAsAnnotation($reflectionMethod->getDeclaringClass(), Areas::class)[0]
?? null;

Check warning on line 132 in src/Routing/FilteredRouteCollectionBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/Routing/FilteredRouteCollectionBuilder.php#L130-L132 

Added lines #L130 - L132 were not covered by tests

if (null === $areas && null !== $this->annotationReader) {
/** @var Areas|null $areas */
$areas = $this->annotationReader->getMethodAnnotation(
$reflectionMethod,
Areas::class
);

if (null === $areas) {
$areas = $this->annotationReader->getClassAnnotation($reflectionMethod->getDeclaringClass(), Areas::class);
}
}
}

return (null !== $areas) ? $areas->has($this->area) : false;
return null !== $areas && $areas->has($this->area);

Check warning on line 134 in src/Routing/FilteredRouteCollectionBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/Routing/FilteredRouteCollectionBuilder.php#L134 

Added line #L134 was not covered by tests
}

private function defaultRouteDisabled(Route $route): bool
Expand All @@ -169,13 +148,9 @@ private function defaultRouteDisabled(Route $route): bool
return false;
}

$annotations = null !== $this->annotationReader
? $this->annotationReader->getMethodAnnotations($method)
: [];

$annotations = array_merge($annotations, array_map(function (\ReflectionAttribute $attribute) {
$annotations = array_map(function (\ReflectionAttribute $attribute) {

Check warning on line 151 in src/Routing/FilteredRouteCollectionBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/Routing/FilteredRouteCollectionBuilder.php#L151 

Added line #L151 was not covered by tests
return $attribute->newInstance();
}, $method->getAttributes(AbstractAnnotation::class, \ReflectionAttribute::IS_INSTANCEOF)));
}, $method->getAttributes(AbstractAnnotation::class, \ReflectionAttribute::IS_INSTANCEOF));

Check warning on line 153 in src/Routing/FilteredRouteCollectionBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/Routing/FilteredRouteCollectionBuilder.php#L153 

Added line #L153 was not covered by tests

foreach ($annotations as $annotation) {
if (false !== strpos(get_class($annotation), 'Nelmio\\ApiDocBundle\\Annotation')
Expand Down
152 changes: 0 additions & 152 deletions tests/Routing/FilteredRouteCollectionBuilderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,8 @@

namespace Nelmio\ApiDocBundle\Tests\Routing;

use Doctrine\Common\Annotations\AnnotationReader;
use Doctrine\Common\Annotations\Reader;
use Nelmio\ApiDocBundle\Annotation\Areas;
use Nelmio\ApiDocBundle\Annotation\Operation;
use Nelmio\ApiDocBundle\Routing\FilteredRouteCollectionBuilder;
use Nelmio\ApiDocBundle\Util\ControllerReflector;
use OpenApi\Annotations\Parameter;
use OpenApi\Context;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Container;
Expand All @@ -31,16 +25,6 @@
*/
class FilteredRouteCollectionBuilderTest extends TestCase
{
/**
* @var AnnotationReader|null
*/
private $doctrineAnnotations;

protected function setUp(): void
{
$this->doctrineAnnotations = class_exists(AnnotationReader::class) ? new AnnotationReader() : null;
}

public function testFilter(): void
{
$options = [
Expand All @@ -60,7 +44,6 @@ public function testFilter(): void
}

$routeBuilder = new FilteredRouteCollectionBuilder(
$this->doctrineAnnotations,
$this->createControllerReflector(),
'areaName',
$options
Expand Down Expand Up @@ -88,7 +71,6 @@ public function testFilterWithDeprecatedArgument(): void
}

$routeBuilder = new FilteredRouteCollectionBuilder(
$this->doctrineAnnotations,
$this->createControllerReflector(),
'areaName',
$pathPattern
Expand All @@ -107,7 +89,6 @@ public function testFilterWithInvalidOption(array $options): void
$this->expectException(InvalidArgumentException::class);

new FilteredRouteCollectionBuilder(
$this->doctrineAnnotations,
$this->createControllerReflector(),
'areaName',
$options
Expand Down Expand Up @@ -162,7 +143,6 @@ public function testMatchingRoutes(string $name, Route $route, array $options =
$routes->add($name, $route);

$routeBuilder = new FilteredRouteCollectionBuilder(
$this->doctrineAnnotations,
$this->createControllerReflector(),
'area',
$options
Expand All @@ -188,72 +168,6 @@ public static function getMatchingRoutes(): \Generator
}
}

/**
* @param array<string, mixed> $options
*/
#[DataProvider('getMatchingRoutesWithAnnotation')]
public function testMatchingRoutesWithAnnotation(string $name, Route $route, array $options = []): void
{
$routes = new RouteCollection();
$routes->add($name, $route);
$area = 'area';

$reflectionMethodStub = $this->createMock(\ReflectionMethod::class);
$controllerReflectorStub = $this->createMock(ControllerReflector::class);
$controllerReflectorStub->method('getReflectionMethod')->willReturn($reflectionMethodStub);

$annotationReader = null;
if (interface_exists(Reader::class)) {
$annotationReader = $this->createMock(Reader::class);
$annotationReader
->method('getMethodAnnotation')
->with($reflectionMethodStub, Areas::class)
->willReturn(new Areas(['value' => [$area]]))
;
}

$routeBuilder = new FilteredRouteCollectionBuilder(
$annotationReader,
$controllerReflectorStub,
$area,
$options
);
$filteredRoutes = $routeBuilder->filter($routes);

self::assertCount(1, $filteredRoutes);
}

public static function getMatchingRoutesWithAnnotation(): \Generator
{
yield from [
'with annotation only' => [
'r10',
new Route('/api/areas/new', ['_controller' => 'ApiController::newAreaAction']),
['with_annotation' => true],
],
'with annotation and path patterns' => [
'r10',
new Route('/api/areas/new', ['_controller' => 'ApiController::newAreaAction']),
['path_patterns' => ['^/api'], 'with_annotation' => true],
],
];

if (\PHP_VERSION_ID < 80000) {
yield from [
'with attribute only' => [
'r10',
new Route('/api/areas_attributes/new', ['_controller' => 'ApiController::newAreaActionAttributes']),
['with_annotation' => true],
],
'with attribute and path patterns' => [
'r10',
new Route('/api/areas_attributes/new', ['_controller' => 'ApiController::newAreaActionAttributes']),
['path_patterns' => ['^/api'], 'with_annotation' => true],
],
];
}
}

/**
* @param array<string, mixed> $options
*/
Expand All @@ -264,7 +178,6 @@ public function testNonMatchingRoutes(string $name, Route $route, array $options
$routes->add($name, $route);

$routeBuilder = new FilteredRouteCollectionBuilder(
$this->doctrineAnnotations,
$this->createControllerReflector(),
'areaName',
$options
Expand All @@ -284,71 +197,6 @@ public static function getNonMatchingRoutes(): \Generator
yield ['r6_non_matching_path_and_non_matching_host', new Route('/admin/bar/action1', [], [], [], 'www.example.com'), ['path_patterns' => ['^/api/'], 'host_patterns' => ['^api\.ex']]];
}

/**
* @param array<Operation|Parameter> $annotations
* @param array<string|bool> $options
*/
#[DataProvider('getRoutesWithDisabledDefaultRoutes')]
public function testRoutesWithDisabledDefaultRoutes(
string $name,
Route $route,
array $annotations,
array $options,
int $expectedRoutesCount
): void {
$routes = new RouteCollection();
$routes->add($name, $route);
$area = 'area';

$reflectionMethodStub = $this->createMock(\ReflectionMethod::class);
$controllerReflectorStub = $this->createMock(ControllerReflector::class);
$controllerReflectorStub->method('getReflectionMethod')->willReturn($reflectionMethodStub);

$annotationReader = null;
if (interface_exists(Reader::class)) {
$annotationReader = $this->createMock(Reader::class);
$annotationReader
->method('getMethodAnnotations')
->willReturn($annotations)
;
}

$routeBuilder = new FilteredRouteCollectionBuilder(
$annotationReader,
$controllerReflectorStub,
$area,
$options
);
$filteredRoutes = $routeBuilder->filter($routes);

self::assertCount($expectedRoutesCount, $filteredRoutes);
}

public static function getRoutesWithDisabledDefaultRoutes(): \Generator
{
yield 'non matching route without Annotation' => [
'r10',
new Route('/api/foo', ['_controller' => 'ApiController::fooAction']),
[],
['disable_default_routes' => true],
0,
];
yield 'matching route with Nelmio Annotation' => [
'r10',
new Route('/api/foo', ['_controller' => 'ApiController::fooAction']),
[new Operation(['_context' => new Context()])],
['disable_default_routes' => true],
1,
];
yield 'matching route with Swagger Annotation' => [
'r10',
new Route('/api/foo', ['_controller' => 'ApiController::fooAction']),
[new Parameter(['_context' => new Context()])],
['disable_default_routes' => true],
1,
];
}

private function createControllerReflector(): ControllerReflector
{
return new ControllerReflector(new Container());
Expand Down

0 comments on commit aa59bb4

Please sign in to comment.