Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: generate enum definitions for schemas that use anyOf #1522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thedadams
Copy link
Contributor

Fixes: #1429

@thedadams
Copy link
Contributor Author

@jamietanna I know there are a few issues similar to this one, but this particular one was easy to address. I looked at the other related issues, but they were not as straightforward.

@grongor
Copy link

grongor commented Apr 3, 2024

Hi :) I was trying out your patch to see if it fixes my issues, but not quite. I have anyOf inside a response definition, and when debugging the issue it was present in the schema.AdditionalTypes field, but never got written out. This fixes it:

diff --git a/pkg/codegen/codegen.go b/pkg/codegen/codegen.go
index a006335..2f5f7ab 100644
--- a/pkg/codegen/codegen.go
+++ b/pkg/codegen/codegen.go
@@ -428,6 +428,10 @@ func GenerateTypeDefinitions(t *template.Template, swagger *openapi3.T, ops []Op
 			return "", fmt.Errorf("error generating Go types for component request bodies: %w", err)
 		}
 		allTypes = append(allTypes, bodyTypes...)
+
+		for _, typ := range allTypes {
+			allTypes = append(allTypes, typ.Schema.GetAdditionalTypeDefs()...)
+		}
 	}

 	// Go through all operations, and add their types to allTypes, so that we can
@@ -529,8 +533,6 @@ func GenerateTypesForSchemas(t *template.Template, schemas map[string]*openapi3.
 			TypeName: goTypeName,
 			Schema:   goSchema,
 		})
-
-		types = append(types, goSchema.GetAdditionalTypeDefs()...)
 	}
 	return types, nil
 }

Not sure if this is the right approach...I've never contributed here, I'm using this package for the first time and I'm also OpenAPI novice, so...yeah 😆
Even with these changes it still doesn't produce working code with my schema (the discriminator field has a custom type, and the generated From* and Merge* methods try to assign a plain string), but I'm again not sure if this is even relevant, or yet another issue...
edit: it was schema issue on my end (optional discriminator)
image

Let me know if you include this or if I should create a separate issue/PR. Cheers!

@thedadams
Copy link
Contributor Author

@grongor Thanks for your input here. It looks like your diff is very similar to the one I have created.

Regarding the discriminator: I haven't looked at this at all, so I am not sure what is going on there. There are a few issues around this type of problem that I found here, but this one fixes #1429 (which is the same as the specific issue I was having). I know @jamietanna was working on a fix to a similar, but different problem, too.

Signed-off-by: Donnie Adams <donnie@acorn.io>
@grongor
Copy link

grongor commented Apr 8, 2024

Sorry for the delay. You can ignore the enum thing (it was an issue with the schema, where the discriminator was marked as optional).

As for the rest, if I understand it correctly, your changes make sure that all additional types are returned on call to GetAdditionalTypeDefs (recursivelly), e.g. fixing a case when a type had additional type that also had additional type (and this final type was previously not generated).
My changes fix other issue: it seems like the method GetAdditionalTypeDefs is not used everywhere where it should be (check the usage of the method), so in my case the types were not generated because the method GenerateTypesForResponses didn't call this GetAdditionalTypeDefs.
That's why I moved the call to GetAdditionalTypeDefs after all types are generated (GenerateTypesForSchemas, GenerateTypesForParameters, GenerateTypesForResponses and GenerateTypesForRequestBodies).

Here is my fork https://github.com/grongor/oapi-codegen/tree/fix-1429, and the commit specifically 5706088.

@thedadams
Copy link
Contributor Author

Are you saying that your approach fixes both issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enum not generated when used with anyof
2 participants