Skip to content

fix: #1530 use discriminator with several mappings for one schema#1534

Closed
jKiler wants to merge 1 commit into
oapi-codegen:mainfrom
jKiler:fix-1530
Closed

fix: #1530 use discriminator with several mappings for one schema#1534
jKiler wants to merge 1 commit into
oapi-codegen:mainfrom
jKiler:fix-1530

Conversation

@jKiler

@jKiler jKiler commented Apr 8, 2024

Copy link
Copy Markdown

I've decided I'll give it a try but as long as fixing ValueByDiscriminator seems pretty straightforward, it requires some more changes to support proper generation of FromX and MergeX methods.

Let's assume our Dog has breeds [ beagle, poodle ]. It will produce a code:

func (t *Pet) FromDog(v Dog) error {
	v.Breed = "beagle"
	v.Breed = "poodle"
	b, err := json.Marshal(v)
	t.union = b
	return err
}

which seems fair if you're not planning to use it, but it compiles only if we specify our breed as required.

I am not fully aware of the intentions behind setting the mapping field in these methods, so I'm not sure if resigning from setting the field is the solution here.

Another pair of 👀 would be much appreciated. Cheers!

Comment on lines +85 to +86
v.Breed = "maine_coon"
v.Breed = "ragdoll"

@mgabeler-lee-6rs mgabeler-lee-6rs Oct 2, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR goes a long way to fixing the issue for me, but it doesn't address this part of the codegen

Edit: I'm looking through how union.tmpl works to see if there is an (easy) way to determine if the type of v has the discriminator field within it. I think I may have found a solution.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've submitted an enhanced version of this PR -- #1789 -- which I think should address this issue

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.

2 participants