Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
21e3c5e to
74d4a3f
Compare
74d4a3f to
5a21506
Compare
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
2bbd9d8 to
10b5284
Compare
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/go/model_simple.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/go/model_simple.mustache:510">
P2: For a required `Foo` with a default, `{"foo":""}` reaches this branch and leaves both `foo` and the defaulted `Foo` in `allProperties`; unmarshalling the marshaled map can let the alias overwrite the default. Remove or normalize the matched alias before adding the canonical default.</violation>
</file>
| @@ -0,0 +1,106 @@ | |||
| openapi: 3.1.0 | |||
There was a problem hiding this comment.
with the change in this PR, i generated a go client using this spec but got the following errors when running go test:
.\model_optional_enum_child.go:63:16: o._ undefined (type *OptionalEnumChild has no field or method _)
.\model_optional_enum_child.go:70:4: o._ undefined (type *OptionalEnumChild has no field or method _)
.\model_optional_enum_child.go:70:16: decoded._ undefined (type struct{Kind *string "json:\"kind,omitempty\""; _ *string "json:\"-,omitempty\""; Label *string "json:\"label,omitempty\""} has no field or method _)
.\model_optional_enum_parent.go:77:25: o._ undefined (type *OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:81:12: o._ undefined (type *OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:87:25: o._ undefined (type *OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:90:11: o._ undefined (type *OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:120:14: o._ undefined (type OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:121:24: o._ undefined (type OptionalEnumParent has no field or method _)
.\model_optional_enum_parent.go:90:11: too many errors
did you encounter similar errors when testing the change locally?
There was a problem hiding this comment.
fixing. Agree
There was a problem hiding this comment.
Fixed. The - property was only exercising the datatag path and caused the generated Go identifier _, so I replaced it with the regular dash property and removed the now-unnecessary name mapping. The generated sample now compiles successfully, including go test ./... and go test -count=1 ./....
| @@ -0,0 +1,40 @@ | |||
| package openapi | |||
There was a problem hiding this comment.
i believe this file should be put in the auto-generated go client using the test spec oneof-not-enum.yaml, right?
if that's the case, please add a config similar to ./bin/configs/go-regex-test.yaml and update the go github workflow to have it tested moving forward: https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-go-client.yaml#L12
There was a problem hiding this comment.
Done. I added a dedicated oneof-not-enum Go sample config and included it in the Go client workflow paths and matrix. The runtime test is now generated into the sample as oneof-not-enum_test.go
| return objs; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
suggestion would be nice to add more comments in the new code block (line 606 - 708)
There was a problem hiding this comment.
Added concise comments around the non-obvious parts of the new logic: scoping allowed-enum checks to the affected oneOf variants, handling inherited allOf properties, and matching the case-insensitive JSON field behavior used by encoding/json.
| return objs; | ||
| } | ||
|
|
||
| private static Map<String, CodegenProperty> effectiveVars(CodegenModel model) { |
There was a problem hiding this comment.
please add docstrings to newly-created functions effectiveVars, stringEnumComparison
There was a problem hiding this comment.
Added Javadocs for both effectiveVars and stringEnumComparison, describing the effective/inherited property handling and the generated string/null enum comparison behavior.
There was a problem hiding this comment.
All reported issues were addressed across 52 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
@cubic-dev-ai review this PR |
@AndreyVMarkelov I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 54 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The sample on master is out of date after OpenAPITools#25064 and OpenAPITools#25089 were merged concurrently, which fails the "Samples up-to-date" check.
…#25138) * [Java][webclient/restclient/resttemplate] Simplify generated API code - Only null-check optional header, cookie and form parameters (and use braces); required parameters are already validated to be non-null - Pass the request body to invokeAPI directly instead of through a redundant postBody local variable - Use the diamond operator for generic instantiations; for anonymous ParameterizedTypeReference classes only when targeting Java 17 - Set the java17 flag for resttemplate when using Jakarta EE and use it to select the Java version in its pom.xml and build.gradle (which also removes duplicated source/target elements with Spring Boot 4) * Update stale go-oneof-not-enum sample The sample on master is out of date after #25064 and #25089 were merged concurrently, which fails the "Samples up-to-date" check.
Description
Fixes #25090
Fix Go client generation for string properties constrained by
enumandnot: { enum: [...] }.Previously, generated models could accept values excluded by
not: enumduring JSON unmarshaling. This could makeoneOfvariants overlap when they were intended to be mutually exclusive.This change adds validation during generated model unmarshaling so:
nullsemantics;not: enumdoes not incorrectly force a string type;encoding/jsoncase-insensitive field matching.Existing
UnmarshalJSONpaths for required andadditionalPropertiesmodels are preserved.Related reports: #25065 (Java), #25069 (Python).
Tests
Added focused Go generator coverage for:
not: enumUnmarshalJSONgenerationGenerated Go code compiles and passes runtime validation with
go test ./....