chore: add support for non-discriminator, oneOf query parameters#216
Merged
Conversation
- Update `api` template to use union types for request parameters that use the `oneOf` construct without a discriminator rather than a generated wrapper model. This ensures that `oneOf` can be used to expand a parameters types without breaking backward compatibility. - Update `ObjectSerializer` template to support the `int|\DateTime` type, which is will be used to by the pending `start` and `end` types. New combinations of `oneOf` schemas will require similar changes in the future. - Update `README.mustache` to skip references to models without variables. The referenced models will be ignored in .openapi-generator-ignore moving forward. - Update `api_doc` template to generate correct examples and type references for `oneOf` request parameters.
Contributor
|
🟢 Coverage increased by 86.26% Code Coverage ReportCoverage Report
Files Coverage
|
There was a problem hiding this comment.
Pull request overview
This PR updates the PHP SDK’s OpenAPI Generator templates and generated serializer to better support oneOf request parameters without a discriminator, primarily to allow query parameters to accept expanded types (e.g., int|\DateTime) without requiring wrapper models.
Changes:
- Adjusts API templates to alter how parameter types are emitted when
oneOfis present without a discriminator. - Extends
ObjectSerializer::toQueryValue()to format\DateTimewhen the OpenAPI type is the unionint|\DateTime. - Updates README/API docs templates to avoid referencing empty models and to improve
oneOfparameter examples/type rendering. - Updates generated
src/ObjectSerializer.phpaccordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| template/README.mustache | Skips listing models with no variables in the README model section. |
| template/ObjectSerializer.mustache | Adds query serialization handling for `int |
| template/api.mustache | Changes emitted param doc/signature typing for non-discriminator oneOf parameters. |
| template/api_doc.mustache | Adjusts generated docs/examples for oneOf request parameters. |
| src/ObjectSerializer.php | Regenerates serializer with `int |
Comments suppressed due to low confidence (2)
template/api.mustache:112
- In the method signature, non-discriminator oneOf parameters are typed as the first schema only ({{schema.dataType}}). This will still reject the additional oneOf alternatives at runtime (PHP type checks) and undermines the goal of accepting expanded types; generate a proper union type (e.g., int|\DateTime) or otherwise loosen the type when hasDiscriminatorWithNonEmptyMapping is false.
public function {{operationId}}({{#allParams}}{{#isArray}}array{{/isArray}}{{#isMap}}array{{/isMap}}{{^isArray}}{{^isMap}}{{^composedSchemas.oneOf}}{{{dataType}}}{{/composedSchemas.oneOf}}{{#composedSchemas.oneOf}}{{#-first}}{{^hasDiscriminatorWithNonEmptyMapping}}{{{schema.dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{#hasDiscriminatorWithNonEmptyMapping}}{{{dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{/-first}}{{/composedSchemas.oneOf}}{{/isMap}}{{/isArray}} ${{paramName}}{{^required}} = {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}): {{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}
{
src/ObjectSerializer.php:198
- The new "int|\DateTime" branch is executed after the emptiness check. Since isEmptyValue() doesn’t understand union OpenAPI types, integer 0 will be considered empty for openApiType "int|\DateTime", which can incorrectly drop/blank query values. Update the emptiness logic to handle union types (or normalize based on runtime value type).
# Handle int and DateTime union in query
if ('int|\DateTime' === $openApiType || '\DateTime|int' === $openApiType) {
if ($value instanceof \DateTime) {
return ["$paramName" => $value->format(self::$dateTimeFormat)];
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The last commit just updated the generated code, not the template.
Contributor
pnpm exec changesetto create a changeset. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
template/api.mustache:145
- Same issue as above for the WithHttpInfo method signature: oneOf parameters use schema.dataType/schema.complexType but the signature still defaults optional params to null. Ensure the type hint includes nullability for optional/nullable oneOf params to avoid a PHP fatal error in generated clients.
* @deprecated
{{/isDeprecated}}
*/
public function {{operationId}}WithHttpInfo({{#allParams}}{{#isArray}}array{{/isArray}}{{#isMap}}array{{/isMap}}{{^isArray}}{{^isMap}}{{^composedSchemas.oneOf}}{{{dataType}}}{{/composedSchemas.oneOf}}{{#composedSchemas.oneOf}}{{#-first}}{{^hasDiscriminatorWithNonEmptyMapping}}{{{schema.dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{#hasDiscriminatorWithNonEmptyMapping}}{{{schema.complexType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{/-first}}{{/composedSchemas.oneOf}}{{/isMap}}{{/isArray}} ${{paramName}}{{^required}} = {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}): array
{
template/api.mustache:222
- Same issue as above for the Async method signature: oneOf parameters emitted via schema.dataType/schema.complexType need to include nullability when the parameter is optional/nullable, otherwise the default null will be invalid for the union/class type.
* @deprecated
{{/isDeprecated}}
*/
public function {{operationId}}Async({{#allParams}}{{#isArray}}array{{/isArray}}{{#isMap}}array{{/isMap}}{{^isArray}}{{^isMap}}{{^composedSchemas.oneOf}}{{{dataType}}}{{/composedSchemas.oneOf}}{{#composedSchemas.oneOf}}{{#-first}}{{^hasDiscriminatorWithNonEmptyMapping}}{{{schema.dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{#hasDiscriminatorWithNonEmptyMapping}}{{{schema.complexType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{/-first}}{{/composedSchemas.oneOf}}{{/isMap}}{{/isArray}} ${{paramName}}{{^required}} = {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}): PromiseInterface
{
template/api.mustache:256
- Same issue as above for the AsyncWithHttpInfo method signature: optional oneOf params default to null, so the emitted union/class type must include null (e.g., add "|null" / nullable handling) to prevent fatal errors in generated code.
* @deprecated
{{/isDeprecated}}
*/
public function {{operationId}}AsyncWithHttpInfo({{#allParams}}{{#isArray}}array{{/isArray}}{{#isMap}}array{{/isMap}}{{^isArray}}{{^isMap}}{{^composedSchemas.oneOf}}{{{dataType}}}{{/composedSchemas.oneOf}}{{#composedSchemas.oneOf}}{{#-first}}{{^hasDiscriminatorWithNonEmptyMapping}}{{{schema.dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{#hasDiscriminatorWithNonEmptyMapping}}{{{schema.complexType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{/-first}}{{/composedSchemas.oneOf}}{{/isMap}}{{/isArray}} ${{paramName}}{{^required}} = {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}): PromiseInterface
{
template/api.mustache:299
- Same issue as above for the Request method signature: optional oneOf params still default to null, but the type hint for oneOf comes from schema.dataType/schema.complexType and may be non-nullable. Ensure nullable handling is included for optional/nullable oneOf parameters so generated code parses and runs.
* @deprecated
{{/isDeprecated}}
*/
public function {{operationId}}Request({{#allParams}}{{#isArray}}array{{/isArray}}{{#isMap}}array{{/isMap}}{{^isArray}}{{^isMap}}{{^composedSchemas.oneOf}}{{{dataType}}}{{/composedSchemas.oneOf}}{{#composedSchemas.oneOf}}{{#-first}}{{^hasDiscriminatorWithNonEmptyMapping}}{{{schema.dataType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{#hasDiscriminatorWithNonEmptyMapping}}{{{schema.complexType}}}{{/hasDiscriminatorWithNonEmptyMapping}}{{/-first}}{{/composedSchemas.oneOf}}{{/isMap}}{{/isArray}} ${{paramName}}{{^required}} = {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}): Request
{
mcnulty-fp
requested review from
Orkuncakilkaya,
TheUnderScorer,
erayaydin and
ilfa
as code owners
TheUnderScorer
approved these changes
May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates the handling for
oneOfconstructs that do not use a discriminator to allow query parameters to use a union type to expand the accepted types for a parameter without breaking backward compatibility. This functionality will be used for the pending changes to expandstartandendto accept adate-timestring.#217 contains the generated code changes that result from these templates when using a schema that includes the changes for
startand `end.These template changes will also require additions to
.openapi-generator-ignoreto ignore the models that would be generated for theoneOfschema construct but aren't needed. #217 shows what that will look like.Itemized changes:
apitemplate to use union types for request parameters that use theoneOfconstruct without a discriminator rather than a generated wrapper model. This ensures thatoneOfcan be used to expand a parameters types without breaking backward compatibility.ObjectSerializertemplate to support theint|\DateTimetype, which is will be used to by the pendingstartandendtypes. New combinations ofoneOfschemas will require similar changes in the future.README.mustacheto skip references to models without variables. The referenced models will be ignored in .openapi-generator-ignore moving forward.api_doctemplate to generate correct examples and type references foroneOfrequest parameters.ObjectSerializer.phpObjectSerializer