swagger-api / swagger-api/swagger-codegen
[JAVA Spring] Discriminator field comes twice in response if specified as a field
Nobody has claimed this yet.
- Dominant language
- Mustache
- Stars
- 17.8k
- Forks
- 6k
- PR merge metrics
- No merged PRs in 30d
Description
Description
I am generating a server stub for Spring Boot and using the discriminator functionality to support polymorphism.
If I define the discriminator also in the list of properties, then I get two fields with the same in the response (one populated, one null). This is clearly wrong (technically valid JSON maybe, but liable to cause problems and confusing). If I define the discriminator as just discriminator and required then I get a warning on the YAML spec that I have a required property that I have not defined.
The Swagger spec says, about the discriminator:
The property name used MUST be defined at this schema and it MUST be in the required property list
So it is technically wrong not to define it in the properties list. The warning is correct.
The spec also says:
When used, the value MUST be the name of this schema or any schema that inherits it.
So I think the approach that sets it automagically and doesn't let you manually set/override the value is correct.
Swagger-codegen version
2.4.7
Swagger declaration file content or url
swagger: '2.0'
info:
title: Bug Demo API
description: Demo of discriminator issue
version: 1
host: api.example.com
schemes:
- https
basePath: /
tags:
- name: demo
description: Demo
produces:
- application/json
- application/xml
definitions:
AbstractParentObjWithClassNameField:
type: object
required:
- '_className'
discriminator: '_className'
properties:
'_className':
type: string
description: Internal class name. Used to support polymorphism.
id:
type: integer
ChildObjectAForParentWithClassname:
allOf:
- $ref: '#/definitions/AbstractParentObjWithClassNameField'
- type: object
properties:
foo:
type: string
AbstractParentObjWithoutClassNameField:
type: object
required:
- '_className'
discriminator: '_className'
properties:
id:
type: integer
ChildObjectAForParentWithoutClassname:
allOf:
- $ref: '#/definitions/AbstractParentObjWithoutClassNameField'
- type: object
properties:
foo:
type: string
paths:
/foo:
get:
operationId: getFoo
tags:
- demo
responses:
'200':
description: Foo
schema:
$ref: '#/definitions/ChildObjectAForParentWithClassname'
/bar:
get:
operationId: getBar
tags:
- demo
responses:
'200':
description: Bar
schema:
$ref: '#/definitions/ChildObjectAForParentWithoutClassname'
Command line used for generation
I am generating with the Maven plugin with the following options:
<language>spring</language>
<withXml>true</withXml>
<configOptions>
<serializableModel>true</serializableModel>
<dateLibrary>java8</dateLibrary>
<java8>true</java8>
<async>true</async>
<library>spring-boot</library>
<delegatePattern>true</delegatePattern>
<useBeanValidation>true</useBeanValidation>
<useOptional>true</useOptional>
<hideGenerationTimestamp>true</hideGenerationTimestamp>
<useTags>true</useTags>
</configOptions>
Steps to reproduce
Generate the API stub described above and implement the methods as:
@Override
public CompletableFuture<ResponseEntity<ChildObjectAForParentWithClassname>> getFoo() {
ChildObjectAForParentWithClassname obj = new ChildObjectAForParentWithClassname().foo("abc");
obj.setId(1);
return CompletableFuture.completedFuture(new ResponseEntity<>(
obj,
HttpStatus.OK));
}
@Override
public CompletableFuture<ResponseEntity<ChildObjectAForParentWithoutClassname>> getBar() {
ChildObjectAForParentWithoutClassname obj = new ChildObjectAForParentWithoutClassname().foo("123");
obj.setId(2);
return CompletableFuture.completedFuture(new ResponseEntity<>(
obj,
HttpStatus.OK));
}
Calling the /foo endpoint (asking for JSON response) gives:
{
"_className": "ChildObjectAForParentWithClassname",
"_className": null,
"id": 1,
"foo": "abc"
}
With the double field. Calling the /bar endpoint produces an acceptable response:
{
"_className": "ChildObjectAForParentWithoutClassname",
"id": 2,
"foo": "123"
}
But it gives a warning in development on the swagger spec. I think the warning is correct - my reading of the spec quoted above is that the property should be included - but for now I am ignoring it.
If I ask for XML response, then /foo produces:
<ChildObjectAForParentWithClassname _className="ChildObjectAForParentWithClassname">
<_className/>
<id>1</id>
<foo>abc</foo>
</ChildObjectAForParentWithClassname>
Which is even more wrong, IMO. /bar then produces
<ChildObjectAForParentWithoutClassname _className="ChildObjectAForParentWithoutClassname">
<id>2</id>
<foo>123</foo>
</ChildObjectAForParentWithoutClassname>
Which is definitely wrong - the spec says the field must be included!
Related issues/PRs
#9487 might be relevant
Suggest a fix/enhancement
The generated code for the parent produces:
@Validated
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "_className", visible = true )
@JsonSubTypes({
@JsonSubTypes.Type(value = ChildObjectAForParentWithClassname.class, name = "ChildObjectAForParentWithClassname"),
})
The difference (apart from the @JsonSubTypes values of course) is that one then also includes
@JsonProperty("_className")
private String className = null;
public AbstractParentObjWithClassNameField className(String className) {
this.className = className;
return this;
}
/**
* Internal class name. Used to support polymorphism.
* @return className
**/
@ApiModelProperty(required = true, value = "Internal class name. Used to support polymorphism.")
@NotNull
public String getClassName() {
return className;
}
public void setClassName(String className) {
this.className = className;
}
while the other does not.
I see a few options:
- Define as
JsonTypeInfo.As.EXISTING_PROPERTYinstead ofJsonTypeInfo.As.PROPERTY.- This would mean it would have to be set also in code (at least, if I try to just make this change in the generated code, it comes out only once but as null)
- Could allow you to override the value and set it in correctly (or leave null)
- I guess you could define the field with only a getter and set it in the constructor?
- If the discriminator field is present in properties then do not generate a field in the class
- This means you only get it once
- It also means it is automatically set correctly
- But it means the field isn't available within the code you then write.
To be honest, I'm struggling to see how to solve this in a backwardly compatible way. I was leaning towards not generating the field when I had only looked at the JSON response, but then the XML makes me go the other way. I think at this point I'd be leaning to:
- Generate the field but as don't have a public setter (maybe a protected)
- Use the
include = JsonTypeInfo.As.EXISTING_PROPERTYvalue on the annotation - Set it in the constructor
So, the parent class would be something like:
@Validated
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "_className", visible = true )
@JsonSubTypes({
@JsonSubTypes.Type(value = ChildObjectAForParentWithClassname.class, name = "ChildObjectAForParentWithClassname"),
})
public class AbstractParentObjWithClassNameField implements Serializable {
private static final long serialVersionUID = 1L;
public AbstractParentObjWithClassNameField() {
this.setClassName("AbstractParentObjWithClassNameField");
}
@JsonProperty("_className")
private String className = null;
/**
* Internal class name. Used to support polymorphism.
* @return className
**/
@ApiModelProperty(required = true, value = "Internal class name. Used to support polymorphism.")
@NotNull
public String getClassName() {
return className;
}
protected void setClassName(final String className) {
this.className = className;
}
But then we'd need to work out that the parent had a discriminator when generating the child classes and set it
public class ChildObjectAForParentWithClassname extends AbstractParentObjWithClassNameField implements Serializable {
private static final long serialVersionUID = 1L;
public ChildObjectAForParentWithClassname() {
this.setClassName("ChildObjectAForParentWithClassname");
}
Which is far from ideal.
It seems to produce valid responses:
{
"_className": "ChildObjectAForParentWithClassname",
"id": 1,
"foo": "abc"
}
Although the _classname attribute is missing from the root object in the XML version (is that a problem?):
<ChildObjectAForParentWithClassname>
<_className>ChildObjectAForParentWithClassname</_className>
<id>1</id>
<foo>abc</foo>
</ChildObjectAForParentWithClassname>
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Run the Maven Spring generator with the supplied Swagger 2.0 definition and compare the generated AbstractParentObjWithClassNameField and AbstractParentObjWithoutClassNameField models. Start by examining the discriminator annotations and generated _className properties, then verify JSON and XML responses for duplicate or missing discriminator fields. Done means the generated models produce correct discriminator output in both formats while preserving the required-property behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring-boot
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100