[php-nextgen] Add anyof Support#24306
Conversation
There was a problem hiding this comment.
1 issue found across 35 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PhpNextgenClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PhpNextgenClientCodegen.java:175">
P1: `anyOf` values that satisfy multiple member schemas lose data: this makes their API type a single-member union and the dispatcher returns the first matching member. Represent `anyOf` as a value capable of retaining all applicable constraints/properties, rather than reusing `oneOf`'s exclusive-branch union/dispatch behavior.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| members.addAll(composed.getOneOf()); | ||
| } | ||
| if (composed.getAnyOf() != null) { | ||
| members.addAll(composed.getAnyOf()); |
There was a problem hiding this comment.
P1: anyOf values that satisfy multiple member schemas lose data: this makes their API type a single-member union and the dispatcher returns the first matching member. Represent anyOf as a value capable of retaining all applicable constraints/properties, rather than reusing oneOf's exclusive-branch union/dispatch behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PhpNextgenClientCodegen.java, line 175:
<comment>`anyOf` values that satisfy multiple member schemas lose data: this makes their API type a single-member union and the dispatcher returns the first matching member. Represent `anyOf` as a value capable of retaining all applicable constraints/properties, rather than reusing `oneOf`'s exclusive-branch union/dispatch behavior.</comment>
<file context>
@@ -143,40 +143,47 @@ public void processOpts() {
+ members.addAll(composed.getOneOf());
+ }
+ if (composed.getAnyOf() != null) {
+ members.addAll(composed.getAnyOf());
+ }
+ if (members.isEmpty()) {
</file context>
There was a problem hiding this comment.
This was on purpose, anyOf uses the same composed dispatcher as oneOf, so for a discriminated anyOf the discriminator selects exactly one model. Only non-discriminated anyOfs matching multiple models fall back to first-match, which I think is how some other generators handle this.
Could look into the work to fix this but it'd probably be a fairly large redesign, lmk
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| supportingFiles.add(new SupportingFile("ObjectSerializer.mustache", toSrcPath(invokerPackage, srcBasePath), "ObjectSerializer.php")); | ||
| supportingFiles.add(new SupportingFile("ModelInterface.mustache", toSrcPath(modelPackage, srcBasePath), "ModelInterface.php")); | ||
| supportingFiles.add(new SupportingFile("OneOfInterface.mustache", toSrcPath(modelPackage, srcBasePath), "OneOfInterface.php")); | ||
| supportingFiles.add(new SupportingFile("ComposedInterface.mustache", toSrcPath(modelPackage, srcBasePath), "ComposedInterface.php")); |
There was a problem hiding this comment.
What about keeping the OneOfInterface and adding a AnyOfInterface instead?
Renaming OneOfInterface to ComposedInterface is a breaking change if I'm not mistaken.
For other generators we do maintain separate implementations (mustache files) for anyOf and oneOf.
There was a problem hiding this comment.
I'm definitely not opposed to splitting them out, I implemented it this way since the two are so similar to avoid some more duplication. But if we want to match some of the other generators we can definitely migrate to 2 separate models.
This is technically breaking, but the original oneOf implementation hasn't been released yet, so the only people this would break is those using 7.24.0-SNAPSHOT. If that's an issue then we can definitely break these out into oneOf/anyOf. Let me know and I can start on that
There was a problem hiding this comment.
@coffeemakr can you please also review this change when you've time since this is a follow-up PR to #23985 authored by you?
There was a problem hiding this comment.
Personally I prefer making the implementation more consistent to other generators. In addition, the term Composed may not be familiar to developers who just start using OpenAPI in their workflow.
|
also cc @coffeemakr @JulianVennen (top contributors to php-nextgen client generator) |
Follow up to #23985
This PR adds support for anyOf models with and without discriminators. It copies the implementation from the oneOf model and updates some of the naming/logic to refer to
Composedinstead ofoneOforanyOfspecifically.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
@jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ybelenko (2018/07), @renepardon (2018/12)
Summary by cubic
Adds
anyOfsupport to the PHP Nextgen generator and unifiesoneOf/anyOfunder a sharedComposedInterface. Updates deserialization to dispatch composed types before normalization to keep arrays and scalars intact.New Features
anyOfdispatcher models viamodel_anyof.mustache.OneOfInterfacewithComposedInterfaceand rename methods togetComposed*; update templates, samples, and tests.deserializeComposedinObjectSerializerand route composed models usingComposedInterface.anyOfmembers and discriminator selection.anyOftests; regenerate samples/docs/tests.Bug Fixes
ObjectSerializer, dispatch composed models before the array-to-object cast and normalization to avoid corrupting arrays/scalars.Throwableduring deserialization to preserve error handling.Written for commit d95922f. Summary will update on new commits.