Allow Struct Embedding with allOf#2086
Conversation
This is useful when working with extending APIs Signed-off-by: Charly Molter <[email protected]>
Kusari Analysis Results:
No pinned version dependency changes, code issues or exposed secrets detected! Note View full detailed analysis result for more information on the output and the checks that were run.
Found this helpful? Give it a 👍 or 👎 reaction! |
|
Would love to see this merged, good job! |
Greptile SummaryThis PR adds support for struct embedding in Key Changes:
Potential Issue:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| pkg/codegen/extension.go | Added new extension constant and helper function for x-go-allof-embed-refs feature |
| pkg/codegen/merge_schemas.go | Core implementation of embedding feature with validation for overlapping fields and required field modifications |
| pkg/codegen/schema.go | Added call to ReadExtGoAllOfEmbedRefs to enable the feature when processing allOf schemas |
| internal/test/all_of/embed-openapi.yaml | Test schema with potential overlapping FirstName field in Person allOf that may violate validation rules |
Last reviewed commit: b109546
| allOf: | ||
| - $ref: "#/components/schemas/PersonProperties" | ||
| - required: [ FirstName, LastName ] | ||
| - $ref: "#/components/schemas/FirstName" |
There was a problem hiding this comment.
Potential overlapping field issue: both PersonProperties (line 34) and FirstName (line 36) define a FirstName property. The validation logic in merge_schemas.go:215-219 checks for overlapping fields and should reject this when x-go-allof-embed-refs: true is set. Verify that this test case generates correctly or if it needs to be updated.
|
This is a very tricky change, despite seeming simple, and I think we need lots more testing to ensure it doesn't break behavior. When you embed structs, you also embed their methods. In your example, say, Due to this embedding, when the json library is doing its unarshaling, it will find that I've hit these problems before. Dynamic typing in a stricly typed language is quite hard, and allOf/anyOf/oneOf has been the most difficult thing to support in oapi-codegen. |
A rebase of 2 year old PR (#1295) (main code courtesy of Lahabana!) to the latest main branch with a very small tweak to error handling. This is a feature I find very useful. Also addresses #1622 (comment), where the original draft PR was linked.
Using one of my own app examples, adding
x-go-allof-embed-refs: truetoallOfcomponents:to:
This helps build reusable functions for "extendable" types - e.g. I have a function to populate
WorkItem- now if I extend work item to add other fields for other API paths (like above example), I can reuse the same function to manage theWorkItemnested struct, rather than rebuilding an entire new function for the extended type.