Skip to content

Commit 721240b

Browse files
ycerutoyceruto
committed
Implement RemoveScannedQueryParameters processor to clean up unnecessary QueryParameter annotations
1 parent 3da7dee commit 721240b

7 files changed

Lines changed: 159 additions & 50 deletions

File tree

‎src/OpenApi/OpenApiGeneratorFactory.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OpenApi\Processors\AugmentRefs;
1111
use OpenApi\Processors\AugmentRequestBody;
1212
use OpenApi\Processors\BuildPaths;
13+
use OpenApi\Processors\MergeIntoComponents;
1314
use OpenSolid\Api\OpenApi\Processor\AugmentOperations;
1415
use OpenSolid\Api\OpenApi\Processor\AugmentQueryParameters;
1516
use OpenSolid\Api\OpenApi\Processor\AugmentQueryParameterSets;
@@ -18,6 +19,7 @@
1819
use OpenSolid\Api\OpenApi\Processor\AugmentSchemas;
1920
use OpenSolid\Api\OpenApi\Processor\GenerateOperationsFromApiRoutes;
2021
use OpenSolid\Api\OpenApi\Processor\MergeMethodAnnotationsIntoOperations;
22+
use OpenSolid\Api\OpenApi\Processor\RemoveScannedQueryParameters;
2123
use Symfony\Component\Validator\Constraint;
2224
use Psr\Log\LoggerInterface;
2325
use Symfony\Component\TypeInfo\TypeResolver\TypeResolverInterface;
@@ -43,6 +45,7 @@ public function __invoke(): OpenApiGenerator
4345
$pl->insert(new AugmentOperations($this->config['media_type'], $this->typeResolver), BuildPaths::class);
4446
$pl->insert(new AugmentRequestBodies($this->config['media_type']), BuildPaths::class);
4547
$pl->insert(new AugmentQueryParameterSets(), AugmentParameters::class);
48+
$pl->insert(new RemoveScannedQueryParameters(), MergeIntoComponents::class);
4649
$pl->insert(new AugmentQueryParameters($this->pathParameterSchemaResolvers), AugmentRefs::class);
4750
$pl->insert(new AugmentSchemas(), AugmentRequestBody::class);
4851
if (class_exists(Constraint::class)) {

‎src/OpenApi/Processor/AugmentQueryParameterSets.php‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -78,19 +78,6 @@ public function __invoke(Analysis $analysis): void
7878
$operation->parameters[] = $queryParam;
7979
$analysis->addAnnotation($queryParam, $queryParam->_context);
8080
}
81-
82-
// Remove originally-scanned QueryParameter annotations for this class
83-
// to prevent MergeIntoComponents from promoting them into Components::parameters.
84-
// The scanner sets nested=false on these; our new instances above have nested=true.
85-
foreach ($analysis->getAnnotationsOfType(OAT\QueryParameter::class) as $annotation) {
86-
if ($annotation->_context->nested) {
87-
continue;
88-
}
89-
$ref = $annotation->_context->reflector ?? null;
90-
if ($ref instanceof \ReflectionProperty && $ref->getDeclaringClass()->getName() === $className) {
91-
$analysis->annotations->offsetUnset($annotation);
92-
}
93-
}
9481
}
9582
}
9683
}

‎src/OpenApi/Processor/RemoveScannedQueryParameters.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenSolid\Api\OpenApi\Processor;
6+
7+
use OpenApi\Analysis;
8+
use OpenApi\Attributes as OAT;
9+
10+
/**
11+
* Removes QueryParameter annotations that swagger-php's scanner picks up from class properties.
12+
*
13+
* These scanned annotations have nested=false in their context and a ReflectionProperty reflector.
14+
* Without removal, MergeIntoComponents promotes them into Components::parameters where they fail
15+
* validation (missing parameter key-field, missing name). They're not needed because
16+
* AugmentQueryParameterSets re-creates them from reflection and attaches them directly to operations.
17+
*/
18+
final readonly class RemoveScannedQueryParameters
19+
{
20+
public function __invoke(Analysis $analysis): void
21+
{
22+
foreach ($analysis->getAnnotationsOfType(OAT\QueryParameter::class) as $annotation) {
23+
if ($annotation->_context->nested) {
24+
continue;
25+
}
26+
27+
if (($annotation->_context->reflector ?? null) instanceof \ReflectionProperty) {
28+
$analysis->annotations->offsetUnset($annotation);
29+
}
30+
}
31+
}
32+
}

‎tests/Fixtures/App/Model/FindOrdersByProductQuery.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,6 @@ final class FindOrdersByProductQuery
1414
#[OA\QueryParameter(description: 'Filter by external system ID')]
1515
public ?string $externalId = null;
1616

17-
#[OA\QueryParameter(description: 'Filter by product ID', schema: new OA\Schema(format: 'uuid'))]
17+
#[OA\QueryParameter(description: 'Filter by product ID')]
1818
public ?string $productId = null;
1919
}

‎tests/OpenApi/Processor/AugmentQueryParameterSetsTest.php‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -96,38 +96,6 @@ public function itSkipsBuiltinParameterType(): void
9696
self::assertTrue(Generator::isDefault($operation->parameters));
9797
}
9898

99-
#[Test]
100-
public function itRemovesOriginallyScannedAnnotationsFromAnalysis(): void
101-
{
102-
$operation = $this->createOperation(ActionWithMapQueryString::class);
103-
104-
// Simulate swagger-php scanning: add QueryParameter annotations with nested=false
105-
// (exactly as AttributeAnnotationFactory does for property-level attributes)
106-
$scannedAnnotations = [];
107-
$class = new \ReflectionClass(QueryStringParams::class);
108-
foreach ($class->getProperties() as $property) {
109-
$attrs = $property->getAttributes(OAT\QueryParameter::class, \ReflectionAttribute::IS_INSTANCEOF);
110-
if ($attrs !== []) {
111-
$scanned = $attrs[0]->newInstance();
112-
$scanned->_context = new Context([
113-
'nested' => false,
114-
'property' => $property->getName(),
115-
'reflector' => $property,
116-
]);
117-
$scannedAnnotations[] = $scanned;
118-
}
119-
}
120-
121-
$analysis = new Analysis(array_merge([$operation], $scannedAnnotations), new Context());
122-
123-
($this->processor)($analysis);
124-
125-
// The originally-scanned (nested=false) QueryParameter annotations should be removed
126-
foreach ($analysis->getAnnotationsOfType(OAT\QueryParameter::class) as $annotation) {
127-
self::assertTrue((bool) $annotation->_context->nested, 'Expected all remaining QueryParameter annotations to have nested=true');
128-
}
129-
}
130-
13199
#[Test]
132100
public function itUsesCustomParameterName(): void
133101
{

‎tests/OpenApi/Processor/RemoveScannedQueryParametersTest.php‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenSolid\Api\Tests\OpenApi\Processor;
6+
7+
use OpenApi\Analysis;
8+
use OpenApi\Attributes as OAT;
9+
use OpenApi\Context;
10+
use OpenSolid\Api\OpenApi\Processor\RemoveScannedQueryParameters;
11+
use PHPUnit\Framework\Attributes\Test;
12+
use PHPUnit\Framework\TestCase;
13+
14+
class RemoveScannedQueryParametersTest extends TestCase
15+
{
16+
private RemoveScannedQueryParameters $processor;
17+
18+
protected function setUp(): void
19+
{
20+
$this->processor = new RemoveScannedQueryParameters();
21+
}
22+
23+
#[Test]
24+
public function itRemovesScannedQueryParametersWithReflectionPropertyContext(): void
25+
{
26+
// Simulate swagger-php scanning: QueryParameter annotations with nested=false
27+
// (exactly as AttributeAnnotationFactory does for property-level attributes)
28+
$scannedAnnotations = [];
29+
$class = new \ReflectionClass(ScannedParams::class);
30+
foreach ($class->getProperties() as $property) {
31+
$attrs = $property->getAttributes(OAT\QueryParameter::class, \ReflectionAttribute::IS_INSTANCEOF);
32+
if ($attrs !== []) {
33+
$scanned = $attrs[0]->newInstance();
34+
$scanned->_context = new Context([
35+
'nested' => false,
36+
'property' => $property->getName(),
37+
'reflector' => $property,
38+
]);
39+
$scannedAnnotations[] = $scanned;
40+
}
41+
}
42+
43+
$analysis = new Analysis($scannedAnnotations, new Context());
44+
45+
self::assertNotEmpty($analysis->getAnnotationsOfType(OAT\QueryParameter::class));
46+
47+
($this->processor)($analysis);
48+
49+
self::assertEmpty($analysis->getAnnotationsOfType(OAT\QueryParameter::class));
50+
}
51+
52+
#[Test]
53+
public function itKeepsNestedQueryParameters(): void
54+
{
55+
// Simulate a QueryParameter created by AugmentQueryParameterSets (nested=true)
56+
$property = (new \ReflectionClass(ScannedParams::class))->getProperty('name');
57+
$queryParam = new OAT\QueryParameter(description: 'Filter by name');
58+
$queryParam->_context = new Context([
59+
'nested' => true,
60+
'property' => 'name',
61+
'reflector' => $property,
62+
]);
63+
64+
$analysis = new Analysis([$queryParam], new Context());
65+
66+
($this->processor)($analysis);
67+
68+
self::assertCount(1, $analysis->getAnnotationsOfType(OAT\QueryParameter::class));
69+
}
70+
71+
#[Test]
72+
public function itRemovesDuplicateScannedParametersAcrossClasses(): void
73+
{
74+
// Two different classes both with an 'externalId' parameter — the real-world scenario
75+
$scannedAnnotations = [];
76+
foreach ([ScannedParams::class, OtherScannedParams::class] as $className) {
77+
$class = new \ReflectionClass($className);
78+
foreach ($class->getProperties() as $property) {
79+
$attrs = $property->getAttributes(OAT\QueryParameter::class, \ReflectionAttribute::IS_INSTANCEOF);
80+
if ($attrs !== []) {
81+
$scanned = $attrs[0]->newInstance();
82+
$scanned->_context = new Context([
83+
'nested' => false,
84+
'property' => $property->getName(),
85+
'reflector' => $property,
86+
]);
87+
$scannedAnnotations[] = $scanned;
88+
}
89+
}
90+
}
91+
92+
$analysis = new Analysis($scannedAnnotations, new Context());
93+
94+
// Both classes contribute annotations, including duplicate 'externalId'
95+
self::assertGreaterThan(2, count($analysis->getAnnotationsOfType(OAT\QueryParameter::class)));
96+
97+
($this->processor)($analysis);
98+
99+
self::assertEmpty($analysis->getAnnotationsOfType(OAT\QueryParameter::class));
100+
}
101+
}
102+
103+
// Test fixtures
104+
105+
class ScannedParams
106+
{
107+
#[OAT\QueryParameter(description: 'Filter by name')]
108+
public ?string $name = null;
109+
110+
#[OAT\QueryParameter(description: 'Filter by external ID')]
111+
public ?string $externalId = null;
112+
}
113+
114+
class OtherScannedParams
115+
{
116+
#[OAT\QueryParameter(description: 'Filter by external ID')]
117+
public ?string $externalId = null;
118+
119+
#[OAT\QueryParameter(description: 'Filter by status')]
120+
public ?string $status = null;
121+
}

‎tests/OpenApi/expected_openapi.json‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,7 @@
415415
"description": "Filter by product ID",
416416
"required": false,
417417
"schema": {
418-
"type": "string",
419-
"format": "uuid"
418+
"type": "string"
420419
}
421420
},
422421
{
@@ -678,8 +677,7 @@
678677
},
679678
"type": "object"
680679
}
681-
},
682-
"parameters": {}
680+
}
683681
},
684682
"tags": [
685683
{

0 commit comments

Comments
 (0)