Open 0v1se opened 6 years ago
According to you having the name
in the @JsonSubTypes.Type
annotation should be sufficient to make the JSON-Deserializer work?
Where is the information about the field name (petType
in your example) that is used to determinate which kind of object should be created?
@jmini no, it's not sufficient Also @JsonTypeInfo should be present:
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "petType", visible = true )
I didn't notice it because it's generated correctly.
One more thing, petType property is also generated in the Pet model.
@JsonProperty("petType")
private String petType = null;
This way wrong json is produced:
{"petType":"Dog","name":null,"petType":null,"packSize":10}
@jmini I can fix this and change jackson annotations to
@JsonSubTypes({
@JsonSubTypes.Type(value = Cat.class, name = "catttt"),
@JsonSubTypes.Type(value = Dog.class, name = "dogggg"),
})
Also, I can remove property identifying type (in my example is 'petType') from the pojo. I think it won't break anything.
What do you think? This way basic polymorphism will be supported at least for Java clients
Yes, I have started to work on it:
fix_discriminator. I have introduced a DiscriminatorCodegen
used in CodegenModel
and in CodegenOperation
. This is a drop in replacement for io.swagger.v3.oas.models.media.Discriminator
that was used before.
This is work in progress, but my Idea was to edit the templates so that the cases described here are covered.
So, this method (modelInheritanceSupportInGson) won't be needed? Am I right? There will support for inheritance for every generator.
And, mappedModels
will be used insted of children
in (https://github.com/jmini/openapi-generator/blob/6e2ca294b52e55c381eb8dae1fb01ccf86acb754/modules/openapi-generator/src/main/resources/Java/typeInfoAnnotation.mustache#L4)
I know it's WIP, but a small note:
equals should be implemented for DiscriminatorCodegen
, it's used in the code
Thank you a lot for this feedback.
And, mappedModels will be used insted of children
Yes this was my idea.
And an other template change is needed to remove the creation of the discriminator as field:
@JsonProperty("petType")
private String petType = null;
(did not investigate yet)
So, this method (modelInheritanceSupportInGson) won't be needed?
Nice catch, I did not analyze who was computing the "children" map.
equals should be implemented for DiscriminatorCodegen, it's used in the code
Nice catch!
Do you want to take over the PR? I have not that much time right now. If you finish it, I will review it.
There is also a small spec modules/openapi-generator/src/test/resources/3_0/allOf.yaml
to test that everything is working well. A small unit test in DefaultCodegenTest
would be great. Minimum would be to parse the spec and to ensure that the discriminator is set in the CodegenOperation
.
I will try, but I'm not very comfortable with the code right now (I mean, not very familiar with). Also, I won't be able to implement this feature for other languages/libraries.
Will get back if I have questions.
I won't be able to implement this feature for other languages/libraries.
This is always the case. I just fix stuff in for the Java generators and I help at model level (the codegen classes that are exposed in the templates). Then I consider that people with knowledge of other languages needs to consume the elements in their models.
I can help you if you need to know something, but from the feedback you gave me on my WIP commit, I think you know enough to create a great PR.
I almost finished. Will submit PR soon (I had only to change templates and do some minor stuff). But it won't cover one aspect: discriminator field won't be removed from the model.
It is a little bit hard to do this. I will write some thoughts after I submit PR
Sure feel free to do stuff in multiple small steps...
@jmini submitted https://github.com/OpenAPITools/openapi-generator/pull/536
some concerns about the PR:
modelInheritanceSupportInGson
cause it doesn't do anything besides calculating children (for annotation) - not 100% sure about this. Pls, reviewRegarding not including discriminator field from model (petType from this issue)
CodegenModel.vars
(currently I think it's best solution, but I can't see all the consequences of this).
2.2 flag the property with isDiscriminator
and filter it out in templates
2.3 create one more collection with properties and use it in pojo.mustache (and create hasMore, hasVars analogs for this collection)I tried to implement 2.2, but it's not that easy: I needed to change hasMore, hasVars and possibly other auxiliary fields from CodegenModel
to correctly generate java files.
What do you think about this?
I will come back to you with the problem I see with the discriminator in the properties. I am no longer sure what we need.
I will give you an example that is not working at all (using getClass().getSimpleName()
as value). We need to fix it.
In the mean time, I think it is important to document changes like this in the migration guide, I have opened #587 for that.
I have some cases that produce compile error when prefixes are used (all my models are using a common prefix). I will document this.
Other interesting case, @yuanpli reported a compile error when an enum is used: https://github.com/OpenAPITools/openapi-generator/issues/806
@0v1se thanks for your work on the discriminator's mapping.
I've filed https://github.com/OpenAPITools/openapi-generator/pull/1360 with better support for composition/inheritance. Please give it a try and me know if you've any feedback.
Thank you! Will give it a try
The fix for this may also be needed for Java Spring target? Maybe related to #3796
According to the status of the PR, I assume that this has been fixed since 3.2.x
correct?
Because I'm still facing the same issue when generating from swagger. Although one difference is that my discriminator
is in the definition. (And in the definitions, it is only allowed to be a string value)
I have:
definitions:
Instruction:
type: "object"
discriminator: "instructionType"
properties:
instructionType:
description: "Type of instruction"
enum:
- "EMAIL"
- "SOME_OTHER"
EmailInstruction:
allOf:
- $ref: "#/definitions/Instruction"
- type: "object"
properties:
name:
type: "string"
example: "Name of reciever"
description: "Name of the receiver"
emailAddress:
type: "string"
example: "info@stackoverflow.com"
description: "Email address of the receiver"
SomeOtherInstruction:
allOf:
- $ref: "#/definitions/Instruction"
- type: "object"
properties:
anotherone:
type: "string"
example: "Name of reciever"
description: "Name of the receiver"
And in turn the Instruction
is wrapped in another object. When generating, it still uses the class name for the JsonSubType
annotation:
Actual:
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "instructionType", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = SomeOtherInstruction.class, name = "SomeOtherInstruction"),
@JsonSubTypes.Type(value = EmailInstruction.class, name = "EmailInstruction"),
})
Expected:
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "instructionType", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = SomeOtherInstruction.class, name = "SOME_OTHER"),
@JsonSubTypes.Type(value = EmailInstruction.class, name = "EMAIL"),
})
This is also biting me with the following spec and 4.2.2:
OutputFormat:
type: object
required:
- type
properties:
type:
type: string
discriminator:
propertyName: type
mapping:
outputKeyValueLabel: '#/components/schemas/OutputKeyValueLabel'
outputIdLabel: '#/components/schemas/OutputIdLabel'
OutputKeyValueLabel:
allOf:
- $ref: '#/components/schemas/OutputFormat'
- type: object
properties:
keyLabel:
type: string
example: PETS
valueLabel:
type: string
example: DOGS
required:
- keyLabel
OutputIdLabel:
allOf:
- $ref: '#/components/schemas/OutputFormat'
- type: object
properties:
idLabel:
type: string
example: MM1234
required:
- idLabel
this gives
public class OutputFormat {
public static final String SERIALIZED_NAME_TYPE = "type";
@SerializedName(SERIALIZED_NAME_TYPE)
private String type;
public OutputFormat() {
this.type = this.getClass().getSimpleName();
}
public OutputFormat type(String type) {
this.type = type;
return this;
}
...
and the discriminator is set to the class's name instead of the specified mapping. There also isn't any JsonTypeInfo
annotations on the classes.
Note that if you upgrade to an OpenAPI specification instead of swagger 2.0 and use f.e. the <library>jersey2</library>
then it does work. Also check out this stackoverflow post: https://stackoverflow.com/questions/59252417/generate-a-property-as-schema-definition-in-openapi-3-0
Might be mistaken - this should be on OpenApi 3.0.
Silly me. The JsonTypeInfo
annotations are Jackson, while the default uses Gson, which has terrible polymorphic support and probably explains why this is misbehaving. Confirm changing to Jersey2 fixed it. Thanks @ivarreukers!
Just looking at some examples in this conversation that worked (e.g @ivarreukers example), do we really need a self reference from the child to the parent (thus creating that dependency between discriminator and its implementation classes)
According to the example of discriminator in inheritance and polymorphism, the following should work (without cyclic reference between child back to parent)
parent:
oneOf:
- $ref: '#/components/schemas/simpleObject'
- $ref: '#/components/schemas/complexObject'
discriminator:
propertyName: objectType
mapping:
Simple: '#/components/schemas/simpleObject'
Complex: '#/components/schemas/complexObject'
simpleObject:
type: object
required:
- objectType
properties:
objectType:
type: string
complexObject:
type: object
required:
- objectType
properties:
objectType:
type: string
This generated the right JsonSubTypes
on the parent
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "objectType", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = SimpleObjectDto.class, name = "Simple"),
@JsonSubTypes.Type(value = ComplexObjectDto.class, name = "Complex"),
})
@com.fasterxml.jackson.annotation.JsonIgnoreProperties(ignoreUnknown=true)
public class ParentDto { .... }
but the child does not have the extension to the parent
public class SimpleObjectDto { ... } // missing extends ParentDto
but the child does not have the extension to the parent
Thats because of #8495
Description
spec: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/ mapping can be specified in discriminator:
Currently it's not taken in account when generating model (it's not for Java, may be for every other language too)
openapi-generator version
3.1.0-SNAPSHOT
OpenAPI declaration file content or url
Command line used for generation
java -jar ... generate -i api.yaml -g java --library resttemplate
Steps to reproduce
Generate model Here is what's generated in Pet.java:
Related issues/PRs
Related to https://github.com/OpenAPITools/openapi-generator/issues/197
Suggest a fix/enhancement
Should generate model like this: