Repository navigation
Upgrade to swagger-php v6 and infer more of the spec from Symfony - #5
Conversation
Upgrade zircote/swagger-php to ^6.12 and PHPUnit to ^11.5|^12.5|^13.0, and resolve the swagger-php, Symfony 8.1 and PHP 8.5 deprecations. Port the OpenAPI inference of the api library: - native #[MapRequestPayload], #[MapQueryString] and #[MapQueryParameter] - query parameter type, format, items, enum, default, description and required - Symfony Validator constraints as schema keywords - required fields, enums and promoted constructor defaults on schemas - merge of declared and inferred responses, 401/403 for secured operations - statusCode option, #[Head] and #[Options], items from @return docblocks Fix the Path "required" default, the Path format placement, the 204 for empty arrays, multi-method controllers and the runtime "type"/"key" options. Add unit and functional tests for 100% line coverage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 527ddc7aa9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bool $mapWhenEmpty = false, | ||
| ) { | ||
| parent::__construct($acceptFormat, $serializationContext, $validationGroups, $resolver, $validationFailedStatusCode); | ||
| parent::__construct($acceptFormat, $serializationContext, $validationGroups, $resolver, $validationFailedStatusCode, $type, $mapWhenEmpty); |
There was a problem hiding this comment.
Adding mapWhenEmpty here exposes an option that the bundle's RequestPayloadArrayResolver never reads: its mapRequestPayload() still returns null unconditionally for an empty body, after which a required argument produces a 422. Thus #[Payload(mapWhenEmpty: true)] (and native #[MapRequestPayload(mapWhenEmpty: true)], which this resolver also intercepts) does not get Symfony's requested empty-payload mapping behavior.
Useful? React with 👍 / 👎.
| if ('path' !== $parameter->in | ||
| && (Undefined::isDefault($parameter->required) || ($parameter->required && $isOptional)) | ||
| ) { | ||
| $parameter->required = !$isOptional; |
There was a problem hiding this comment.
For a nullable or defaulted DTO property annotated with #[QueryParameter(required: true)], this condition treats the explicit true as inferred noise and overwrites it with false. Unless a separate NotNull/NotBlank constraint later restores it, the generated contract silently loses the author's explicit required flag; inference should only fill an undefined value or distinguish generated values from declared ones.
Useful? React with 👍 / 👎.
| if (Undefined::isDefault($schema->default) && $hasDefault) { | ||
| $default = $reflector->getDefaultValue(); | ||
| if (null !== $default) { | ||
| $schema->default = $default; | ||
| } |
There was a problem hiding this comment.
When a newly inferred query parameter has a PHP enum default, this assigns the enum object directly to the OpenAPI schema. In particular, a pure enum default such as Kind::Foo cannot be JSON-encoded, so generating /openapi.json fails rather than emitting the case name; convert backed enums to their value and pure enums to their name as AugmentSchemas already does.
Useful? React with 👍 / 👎.
| if (($annotation->_context->reflector ?? null) instanceof \ReflectionProperty) { | ||
| $analysis->annotations->offsetUnset($annotation); |
There was a problem hiding this comment.
This removes every top-level query-parameter attribute attached to a property, not only unnamed parameters belonging to a MapQueryString DTO. A legitimate property-level #[QueryParameter(parameter: 'Filter', ...)] is therefore discarded before MergeIntoComponents, leaving any #/components/parameters/Filter reference unresolved; restrict the removal to parameters that are actually regenerated for mapped DTOs or at least exclude named component declarations.
Useful? React with 👍 / 👎.
…enEmpty - Merge the inferred body into the first declared 2xx response, which is the status code ControllerResultSubscriber sends at runtime, instead of adding an unused 200 next to it - Store backed enum defaults of query parameters as their value and pure enum defaults as their name, so the spec can be JSON-encoded - Keep an explicit required: true on query parameters backed by a property - Honor mapWhenEmpty (Symfony 8.1+) for #[MapRequestPayload] and #[MapQueryString] in RequestPayloadArrayResolver Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Summary
Upgrade
zircote/swagger-phpfrom^4.7to^6.12, resolve the deprecations, and infer more of the OpenAPI spec from Symfony attributes, validator constraints and PHP reflection.Upgrade and migration
zircote/swagger-php^6.12,phpunit/phpunit^11.5|^12.5|^13.0, newsymfony/type-inforequirement,symfony/intlas dev dependency.Property,Pathand the route attributes pass named arguments to swagger-php (v6 added new constructor parameters).OpenSolid\OpenApiBundle\OpenApi\Processor\ProcessorInterface, autoconfigured with theopenapi.processortag (swagger-php removed its own interface in v5).Deprecations resolved
Generator::isDefault()/Generator::UNDEFINED→Undefined;DocBlockAnnotationFactoryremoved.ViewEvent::$controllerArgumentsEvent,PartialDenormalizationException::getErrors(),defaultPriorityMethodon tagged iterators.SplObjectStorage::detach().New features
#[MapRequestPayload](withtype:),#[MapQueryString](withkey:) and#[MapQueryParameter]in the spec.Length,Count,Range, comparisons,Regex,Choice,All).required, backed and pure enums, and promoted constructor defaults.statusCode:option on the route attributes, for the spec and the response.#[Head]and#[Options]; one controller class can declare several HTTP methods.@return list<View>and@param list<Item>docblocks.Bug fixes
requireddefault of#[Path]was never applied.#[Path(format: ...)]was written on the parameter object instead of its schema.[].TypeError.type:andkey:options.Behavior changes
requiredproperties.schemawithdefaultinstead ofexample.@OAannotations are not read anymore.See the "Upgrade to swagger-php v6" section in the README.
Tests
🤖 Generated with Claude Code