Discriminators of type string not generated correctly

[t-evers]

If discriminator property for mapping types is of type string and not defined to be required, generation of go-code fails for MergeXYZ and FromXYZ methods.

In both methods, default values are assigned as a string literal but the property is of type string: ` func (t Bar) FromFooA(v FooA) error { v.Kind = "A" b, err := json.Marshal(v) t.union = b return err }`

OpenAPI-Yaml and generated code is attached as an example. mapping_example.zip

[mismithhisler]

According to the swagger docs, all models referenced by the schema that contains the discriminator must require that property.

Are you proposing that setting a default should function similarly to require, in that the property would be a value instead of a pointer?

I wonder if it would be best here to just make the field required, or use the x-go-type-skip-optional-pointer feature.

[t-evers]

No, it is just not possible in Go to assing a literal value to a pointer directly. So instead of having v.Kind = "A" It should be generated this way:

aValue:="A"
v.Kind = &aValue

[mismithhisler]

You are correct, that is not possible.

However your suggestion on how the code should be generated is not an actual "fix" for this issue, because you have not required the discriminator property. You have simply coded around the fact that you are breaking an OpenAPI rule.

[t-evers]

Sorry but I don't understand which rule I might have broken? I simply refactored the string constant out of that assignment. That changes nothing to any OpenAPI specific stuff, it just corrects the syntax. If you have another way of fixing that, so that it compiles, please go ahead. It hast just been a proposal on how to solve that.

[mismithhisler]

From the swagger docs, a discriminator property has to be required. Here is the example from the swagger docs on Discriminators:

I could be wrong but I think the real issue here is that oapi-codegen is not validating the discriminators before generating the code, which is causing it to create code that doesn't compile.

There are libraries that can do OpenAPI validation (the library oapi-codegen uses to load the swagger can do validation, https://github.com/getkin/kin-openapi) but this library, nor any of the others I found, seem to support validating Discriminators.

[t-evers]

Now we are getting somewhere ;-) So we talked about different things. I talked about the generated implementation and you talked about the spec beeing wrong. Well...problem is, my example is just simplified from a real world example where it is exactly like in my example. That might be an error in that spec and I'll for sure report that to the team who created that spec but what I suggested is a very simple fix working around that kind of issues. It is on you whether you close the issue or implement a fix.

[john-tipper]

I encountered similar issues and the fix was as per @mismithhisler's suggestion, which was to rigorously ensure that all properties used as a discriminator were marked as required in the spec.

The spec I was using (Gravitee management API) was producing code that looked like this:

type DefinitionVersion string
type ApiV4 struct {
    DefinitionVersion *DefinitionVersion `json:"definitionVersion,omitEmpty"`
}

// then erroring code looked like
v.DefinitionVersion = "ApiV1"

As @t-evers points out, it's possible to fix the above like this:

aDefinitionVersion := DefinitionVersion("ApiV1")
v.DefinitionVersion = &aDefinitionVersion

However, some other parts of the spec resulted in code that looked like this, notice that there is not a pointer:

type CreateApiV4 struct {
    DefinitionVersion DefinitionVersion `json:"definitionVersion"`
}

In this instance, the generated code is perfectly fine:

v.DefinitionVersion = "ApiV1"

The reason for this different code being generated is that in the latter, the discriminator field is marked as required in all cases.

I tried using the --templates option to override the behaviour of the union.tmpl file to generate code as @t-evers mentioned. However, I was not able to see how to get the type of the discriminator field, so that I could determine what form the generated code was supposed to look like.

For me, the fix was to recognise that if a field is not required, it is generated as a pointer, and that causes the generated code to not compile. The spec I was using has errors in it, where the discriminator properties were not marked as required for all possible use cases. For instance, the generator will not view myDiscriminatorField as being required and therefore you'll see a pointer being created resulting in uncompilable code.

allOf:
  - $ref: '#/components/schemas/fooThatRequiresMyDiscriminatorField'
  - type: object
     properties:
       myDiscriminatorField:
         type: string

It's not ideal that specs can be invalid because discriminator fields are not marked required, yet still pass validation in various tools, including the Swagger editor. However, it does look like codegen will generate correct code if proper attention to detail is paid to marking all discriminator properties as required in all cases.