swagger-api / swagger-api/swagger-codegen

[JAVA Spring] Discriminator field comes twice in response if specified as a field

Open
#9,603 0 comments 3 reactions 0 assignees View on GitHub

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_PROPERTY instead of JsonTypeInfo.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_PROPERTY value 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.