Skip to content

Add isDiscriminator to CodegenProperty and use it to mark discriminator properties with @JsonIgnore #4588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

keenanpepper
Copy link
Contributor

@keenanpepper keenanpepper commented Nov 22, 2019

…for JavaSpring. Fixes #3796

This may also be appropriate for other generators but I am only changing it for the JavaSpring template now. Ran bin/spring-all-petstore.sh and all the changes are expected.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10)

Copy link
Contributor

@lwlee2608 lwlee2608 left a comment

Choose a reason for hiding this comment

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

I do not think it is OpenAPI's intention to remove discriminator object from the payload.

@keenanpepper
Copy link
Contributor Author

I was going along with this comment from the issue #3796: "If removing discriminator property addresses the issue, we can certainly do that."

@davinchia
Copy link

Anyone else want to weigh in on this? The proposed fix seems sensible as the discriminator is mainly an artifact of strongly-typed/object-oriented languages (we are using Java). Since information is within a type, persisting the discriminator field seems extraneous.

Copy link

@jeff9finger jeff9finger left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@davinchia
Copy link

@keenanpepper i took a second look at your changes since my comment yesterday and i believe we also need to remove the visible jackson annotation here https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/JavaSpring/typeInfoAnnotation.mustache#L3. Otherwise the generated code attempts to pass the discriminator to the deserialiser and fails as it is no longer a field on the model.

@keenanpepper keenanpepper force-pushed the remove-discriminator-duplicate branch from 95f25ff to f7f212c Compare December 13, 2019 22:52
@keenanpepper
Copy link
Contributor Author

@davinchia done

@davinchia
Copy link

davinchia commented Dec 17, 2019

Much much appreciated @keenanpepper, I definitely didn't mean to be 'assigning' you work :)

Two questions from me:

  1. Any reason why we would not also include the changes in the Java client? Seems strange/misleading to fix this on the server-side and not on the client side. e.g. as a dev I am now able to implement this correctly, but my clients still send out slightly incorrectly serialised json, which can lead to confusion when debugging. If there aren't good reasons, I'd be happy to do so either in this same PR, or in a separate PR.
  2. What are conventions around merging/releasing this? If a branch passes all tests and has an approval, is it on maintainers or patchers to make sure the PR gets merged in? Furthermore, when can we expect the patch to be released? @jeff9finger can you shed some light?

@keenanpepper
Copy link
Contributor Author

  1. Yes, that's the main thing I'm uncertain of, since there are so many different templates for all the different generators, I don't know which should have this change and which shouldn't. It's possible this bug even affects languages other than Java. I figured it was better to fix it for just one generator, and it can always be fixed in other generators later.

@keenanpepper keenanpepper force-pushed the remove-discriminator-duplicate branch from f7f212c to 5b6be27 Compare December 19, 2019 20:33
@keenanpepper
Copy link
Contributor Author

keenanpepper commented Dec 19, 2019

I just found and fixed an issue with this PR regarding hasMore for the new nonDiscriminatorVars. We need hasMore to be computed independently for them, which means we also need to clone the properties themselves.

I could add a test for this case (discriminator property happens to be the last, so hasMore is incorrectly true on the non-discrimaintor property that becomes the last after the discriminator is removed), but I thought it better to get the code fixed first.

@keenanpepper
Copy link
Contributor Author

Okay I'm having a different thought about this now after we're trying to use my fix internally. We still want to have a getter method for the discriminator property. So my new idea is to scratch this approach (nonDiscriminatorVars), and instead use the @JsonIgnore annotation to mark the discriminator as ignored for serialization.

Any comments on that?

by marking the discriminator property as @JsonIgnored so it will not be
serialized twice.
@keenanpepper keenanpepper force-pushed the remove-discriminator-duplicate branch from 5b6be27 to 88194ff Compare January 10, 2020 03:20
@keenanpepper keenanpepper changed the title Add nonDiscriminatorVars to CodegenModel and use it instead of vars… Add isDiscriminator to CodegenProperty and use it to mark discriminator properties with @JsonIgnore Jan 10, 2020
@keenanpepper
Copy link
Contributor Author

Okay I updated this code review with the new approach (to preserve the context & discussion) and edited the subject since nonDiscriminatorVars is no longer a part of it. Let me know what you think.

@keenanpepper
Copy link
Contributor Author

@davinchia @jeff9finger what do you think of this (somewhat less invasive) approach?

@jeff9finger
Copy link

The Java templates use the x-discriminator-value vendor extension to allow definition of discriminator values. This is a useful feature for me at least.

Marking a discriminator property as ignored in all cases seems to invasive. There are times when the discriminator is needed and other times when it isn't. One example I can think of is when the discriminator has meaning other than just a discriminator. Would it make sense to only ignore the property when the x-discriminator-value vendor extension is null? (If the property is ignored, would the getter for the discriminator still provide the discriminator value?)

@keenanpepper
Copy link
Contributor Author

(If the property is ignored, would the getter for the discriminator still provide the discriminator value?)

Yes, the @JsonIgnore annotation only affects serialization, not deserialization. So if the descriminator is PetType, you can still call instance.getPetType() and get the value of the pet type that was passed in.

That was actually the main reason for me updating this PR to use @JsonIgnore instead of just removing the discriminator from the generated model.

@jeff9finger
Copy link

Yes, the @JsonIgnore annotation only affects serialization, not deserialization.

OK. That is good.

IIRC, the @JsonTypeInfo annotation instructs jersey to add the discriminator to the payload on serialization. Is that correct?

@keenanpepper
Copy link
Contributor Author

Yes, the @JsonTypeInfo stuff causes the correct subtype to be instantiated during deserialization, and also causes the discriminator property to be added based on the concrete type during serialization. The discriminator field of the object will be ignored in favor of the concrete type during serialization.

@jeff9finger
Copy link

Great.
LGTM 👍

@nickshoe
Copy link

nickshoe commented Jan 19, 2021

Another possibility is to remove the discriminator property from the generated class altogether.

This would initially lead to the following exception during deserialization:
com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: ...

But this can be easily fixed by configuring the Jackson ObjectMapper to ignore non-existing class properties when it encounters the respective key in JSON, during deserialization.

    ObjectMapper om = new ObjectMapper();
    om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
    ...

Please see the following code example.

Event.java

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;

@JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.PROPERTY,
        property = "type",
        visible = true
)
@JsonSubTypes({
        @JsonSubTypes.Type(value = CreationEvent.class, name = "CreationEvent"),
        @JsonSubTypes.Type(value = DeletionEvent.class, name = "DeletionEvent")
})
public class Event {

    private String name;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

}

CreationEvent.java

public class CreationEvent extends Event {
}

DeletionEvent.java

public class DeletionEvent extends Event {
}

JacksonExperiment.java

    public static void main(String[] args) {
        ObjectMapper om = new ObjectMapper();
        om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

        try {
            Event deserializedCreationEvent = om.readValue("{ \"type\": \"CreationEvent\", \"name\": \"Foo\" }", Event.class);
            System.out.println(deserializedCreationEvent.getClass().getName());

            Event deserializedDeletionEvent = om.readValue("{ \"type\": \"DeletionEvent\", \"name\": \"Bar\" }", Event.class);
            System.out.println(deserializedDeletionEvent.getClass().getName());

            CreationEvent creationEvent = new CreationEvent();
            creationEvent.setName("Foo");
            String serializedCreationEvent = om.writeValueAsString(creationEvent);
            System.out.println(serializedCreationEvent);

            DeletionEvent deletionEvent = new DeletionEvent();
            deletionEvent.setName("Bar");
            String serializedDeletionEvent = om.writeValueAsString(deletionEvent);
            System.out.println(serializedDeletionEvent);
        } catch (JsonProcessingException e) {
            e.printStackTrace();
        }
    }

This code will produce the following output:

CreationEvent
DeletionEvent
{"type":"CreationEvent","name":"Foo"}
{"type":"DeletionEvent","name":"Bar"}

@nickshoe
Copy link

If adding the om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); config might be an hassle, we could open an issue on Jackson lib and ask them not to raise an error when a JSON key is used as the discriminator property (i.e. it is specified in the @JsonTypeInfo( ..., property = "type", ... )) and the class is missing the respective property.

@holomekc
Copy link
Contributor

holomekc commented Feb 4, 2021

This issue is not only related to JavaSpring, because it is an overall Jackson topic. As mentioned by @nickshoe the it must not be JsonTypeInfo.As.EXISTING_PROPERTY because due to Jackson documentation would result in client being responsible for setting appropriate type values and this would make JsonSubTypes annotation useless. So the changes I can see in the commit mentioned in this issue are not enough.

We are using jersey2 and what works for is is to overwrite typeInfoAnnotation.mustache and pojo.mustache:

typeInfoAnnotation.mustache:

{{#jackson}}

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "{{{discriminator.propertyBaseName}}}", visible = true)
{{#discriminator.mappedModels}}
{{#-first}}
@JsonSubTypes({
{{/-first}}
  @JsonSubTypes.Type(value = {{modelName}}.class, name = "{{^vendorExtensions.x-discriminator-value}}{{mappingName}}{{/vendorExtensions.x-discriminator-value}}{{#vendorExtensions.x-discriminator-value}}{{{vendorExtensions.x-discriminator-value}}}{{/vendorExtensions.x-discriminator-value}}"),
{{#-last}}
})
{{/-last}}
{{/discriminator.mappedModels}}
{{#isClassnameSanitized}}
@JsonTypeName("{{name}}")
{{/isClassnameSanitized}}
{{/jackson}}

pojo.mustache:

{{#jackson}}
import com.fasterxml.jackson.annotation.JsonIgnore;
{{/jackson}}
/**
 * {{#description}}{{.}}{{/description}}{{^description}}{{classname}}{{/description}}
 */{{#description}}
@ApiModel(description = "{{{description}}}"){{/description}}
{{#jackson}}
@JsonPropertyOrder({
{{#vars}}
  {{classname}}.JSON_PROPERTY_{{nameInSnakeCase}}{{^-last}},{{/-last}}
{{/vars}}
})
{{/jackson}}
{{>additionalModelTypeAnnotations}}{{>generatedAnnotation}}{{#discriminator}}{{>typeInfoAnnotation}}{{/discriminator}}{{>xmlAnnotation}}
public class {{classname}} {{#parent}}extends {{{parent}}} {{/parent}}{{#vendorExtensions.x-implements}}{{#-first}}implements {{{.}}}{{/-first}}{{^-first}}, {{{.}}}{{/-first}}{{#-last}} {{/-last}}{{/vendorExtensions.x-implements}}{
{{#serializableModel}}
  private static final long serialVersionUID = 1L;

{{/serializableModel}}
  {{#vars}}
    {{#isEnum}}
    {{^isContainer}}
    {{^vendorExtensions.x-enum-as-string}}
{{>modelInnerEnum}}
    {{/vendorExtensions.x-enum-as-string}}
    {{/isContainer}}
    {{#isContainer}}
    {{#mostInnerItems}}
{{>modelInnerEnum}}
    {{/mostInnerItems}}
    {{/isContainer}}
    {{/isEnum}}
  {{#gson}}
  public static final String SERIALIZED_NAME_{{nameInSnakeCase}} = "{{baseName}}";
  {{/gson}}
  {{#jackson}}
  public static final String JSON_PROPERTY_{{nameInSnakeCase}} = "{{baseName}}";
  {{/jackson}}
  {{#withXml}}
  {{#isXmlAttribute}}
  @XmlAttribute(name = "{{#xmlName}}{{xmlName}}{{/xmlName}}{{^xmlName}}{{baseName}}{{/xmlName}}")
  {{/isXmlAttribute}}
  {{^isXmlAttribute}}
    {{^isContainer}}
  @XmlElement({{#xmlNamespace}}namespace="{{xmlNamespace}}", {{/xmlNamespace}}name = "{{#xmlName}}{{xmlName}}{{/xmlName}}{{^xmlName}}{{baseName}}{{/xmlName}}")
    {{/isContainer}}
    {{#isContainer}}
  // Is a container wrapped={{isXmlWrapped}}
      {{#items}}
  // items.name={{name}} items.baseName={{baseName}} items.xmlName={{xmlName}} items.xmlNamespace={{xmlNamespace}}
  // items.example={{example}} items.type={{dataType}}
  @XmlElement({{#xmlNamespace}}namespace="{{xmlNamespace}}", {{/xmlNamespace}}name = "{{#xmlName}}{{xmlName}}{{/xmlName}}{{^xmlName}}{{baseName}}{{/xmlName}}")
      {{/items}}
      {{#isXmlWrapped}}
  @XmlElementWrapper({{#xmlNamespace}}namespace="{{xmlNamespace}}", {{/xmlNamespace}}name = "{{#xmlName}}{{xmlName}}{{/xmlName}}{{^xmlName}}{{baseName}}{{/xmlName}}")
      {{/isXmlWrapped}}
    {{/isContainer}}
  {{/isXmlAttribute}}
  {{/withXml}}
  {{#gson}}
  @SerializedName(SERIALIZED_NAME_{{nameInSnakeCase}})
  {{/gson}}
  {{#vendorExtensions.x-is-jackson-optional-nullable}}
  {{#isContainer}}
  private JsonNullable<{{{datatypeWithEnum}}}> {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>undefined();
  {{/isContainer}}
  {{^isContainer}}
  private JsonNullable<{{{datatypeWithEnum}}}> {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>{{#defaultValue}}of({{{.}}}){{/defaultValue}}{{^defaultValue}}undefined(){{/defaultValue}};
  {{/isContainer}}
  {{/vendorExtensions.x-is-jackson-optional-nullable}}
  {{^vendorExtensions.x-is-jackson-optional-nullable}}
  {{#isContainer}}
  private {{{datatypeWithEnum}}} {{name}}{{#required}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/required}}{{^required}} = null{{/required}};
  {{/isContainer}}
  {{^isContainer}}
  private {{{datatypeWithEnum}}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};
  {{/isContainer}}
  {{/vendorExtensions.x-is-jackson-optional-nullable}}

  {{/vars}}
  {{#parcelableModel}}
  public {{classname}}() {
  {{#parent}}
    super();
  {{/parent}}
  {{#gson}}
  {{#discriminator}}
    this.{{{discriminatorName}}} = this.getClass().getSimpleName();
  {{/discriminator}}
  {{/gson}}
  }
  {{/parcelableModel}}
  {{^parcelableModel}}
  {{#gson}}
  {{#discriminator}}
  public {{classname}}() {
    this.{{{discriminatorName}}} = this.getClass().getSimpleName();
  }
  {{/discriminator}}
  {{/gson}}
  {{/parcelableModel}}
  {{#vars}}

  {{^isReadOnly}}
  {{#vendorExtensions.x-enum-as-string}}
  public static final Set<String> {{{nameInSnakeCase}}}_VALUES = new HashSet<>(Arrays.asList(
    {{#allowableValues}}{{#enumVars}}{{{value}}}{{^-last}}, {{/-last}}{{/enumVars}}{{/allowableValues}}
  ));

  {{/vendorExtensions.x-enum-as-string}}
  public {{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
    {{#vendorExtensions.x-enum-as-string}}
    if (!{{{nameInSnakeCase}}}_VALUES.contains({{name}})) {
      throw new IllegalArgumentException({{name}} + " is invalid. Possible values for {{name}}: " + String.join(", ", {{{nameInSnakeCase}}}_VALUES));
    }

    {{/vendorExtensions.x-enum-as-string}}
    {{#vendorExtensions.x-is-jackson-optional-nullable}}
    this.{{name}} = JsonNullable.<{{{datatypeWithEnum}}}>of({{name}});
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    {{^vendorExtensions.x-is-jackson-optional-nullable}}
    this.{{name}} = {{name}};
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    return this;
  }
  {{#isArray}}

  public {{classname}} add{{nameInCamelCase}}Item({{{items.datatypeWithEnum}}} {{name}}Item) {
    {{#vendorExtensions.x-is-jackson-optional-nullable}}
    if (this.{{name}} == null || !this.{{name}}.isPresent()) {
      this.{{name}} = JsonNullable.<{{{datatypeWithEnum}}}>of({{{defaultValue}}});
    }
    try {
      this.{{name}}.get().add({{name}}Item);
    } catch (java.util.NoSuchElementException e) {
      // this can never happen, as we make sure above that the value is present
    }
    return this;
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    {{^vendorExtensions.x-is-jackson-optional-nullable}}
    {{^required}}
    if (this.{{name}} == null) {
      this.{{name}} = {{{defaultValue}}};
    }
    {{/required}}
    this.{{name}}.add({{name}}Item);
    return this;
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
  }
  {{/isArray}}
  {{#isMap}}

  public {{classname}} put{{nameInCamelCase}}Item(String key, {{{items.datatypeWithEnum}}} {{name}}Item) {
    {{#vendorExtensions.x-is-jackson-optional-nullable}}
    if (this.{{name}} == null || !this.{{name}}.isPresent()) {
      this.{{name}} = JsonNullable.<{{{datatypeWithEnum}}}>of({{{defaultValue}}});
    }
    try {
      this.{{name}}.get().put(key, {{name}}Item);
    } catch (java.util.NoSuchElementException e) {
      // this can never happen, as we make sure above that the value is present
    }
    return this;
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    {{^vendorExtensions.x-is-jackson-optional-nullable}}
    {{^required}}
    if (this.{{name}} == null) {
      this.{{name}} = {{{defaultValue}}};
    }
    {{/required}}
    this.{{name}}.put(key, {{name}}Item);
    return this;
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
  }
  {{/isMap}}

  {{/isReadOnly}}
   /**
  {{#description}}
   * {{description}}
  {{/description}}
  {{^description}}
   * Get {{name}}
  {{/description}}
  {{#minimum}}
   * minimum: {{minimum}}
  {{/minimum}}
  {{#maximum}}
   * maximum: {{maximum}}
  {{/maximum}}
   * @return {{name}}
  **/
{{#required}}
{{#isNullable}}
  @javax.annotation.Nullable
{{/isNullable}}
{{/required}}
{{^required}}
  @javax.annotation.Nullable
{{/required}}
{{#useBeanValidation}}{{>beanValidation}}{{/useBeanValidation}}  @ApiModelProperty({{#example}}example = "{{{example}}}", {{/example}}{{#required}}required = {{required}}, {{/required}}value = "{{{description}}}")
{{#vendorExtensions.x-extra-annotation}}
  {{{vendorExtensions.x-extra-annotation}}}
{{/vendorExtensions.x-extra-annotation}}
{{#vendorExtensions.x-is-jackson-optional-nullable}}
  {{!unannotated, Jackson would pick this up automatically and add it *in addition* to the _JsonNullable getter field}}
  @JsonIgnore
{{/vendorExtensions.x-is-jackson-optional-nullable}}
{{#jackson}}{{#isDiscriminator}}
  @JsonIgnore
{{/isDiscriminator}}{{/jackson}}
{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#jackson}}{{> jackson_annotations}}{{/jackson}}{{/vendorExtensions.x-is-jackson-optional-nullable}}
  public {{{datatypeWithEnum}}} {{getter}}() {
    {{#vendorExtensions.x-is-jackson-optional-nullable}}
    {{#isReadOnly}}{{! A readonly attribute doesn't have setter => jackson will set null directly if explicitly returned by API, so make sure we have an empty JsonNullable}}
    if ({{name}} == null) {
      {{name}} = JsonNullable.<{{{datatypeWithEnum}}}>{{#defaultValue}}of({{{.}}}){{/defaultValue}}{{^defaultValue}}undefined(){{/defaultValue}};
    }
    {{/isReadOnly}}
    return {{name}}.orElse(null);
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    {{^vendorExtensions.x-is-jackson-optional-nullable}}
    return {{name}};
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
  }
  {{#vendorExtensions.x-is-jackson-optional-nullable}}
{{> jackson_annotations}}
  public JsonNullable<{{{datatypeWithEnum}}}> {{getter}}_JsonNullable() {
    return {{name}};
  }
  {{/vendorExtensions.x-is-jackson-optional-nullable}}{{#vendorExtensions.x-is-jackson-optional-nullable}}
  @JsonProperty(JSON_PROPERTY_{{nameInSnakeCase}})
  {{#isReadOnly}}private{{/isReadOnly}}{{^isReadOnly}}public{{/isReadOnly}} void {{setter}}_JsonNullable(JsonNullable<{{{datatypeWithEnum}}}> {{name}}) {
    {{! For getters/setters that have name differing from attribute name, we must include setter (albeit private) for jackson to be able to set the attribute}}
    this.{{name}} = {{name}};
  }
  {{/vendorExtensions.x-is-jackson-optional-nullable}}
  {{^isReadOnly}}
  public void {{setter}}({{{datatypeWithEnum}}} {{name}}) {
    {{#vendorExtensions.x-enum-as-string}}
    if (!{{{nameInSnakeCase}}}_VALUES.contains({{name}})) {
      throw new IllegalArgumentException({{name}} + " is invalid. Possible values for {{name}}: " + String.join(", ", {{{nameInSnakeCase}}}_VALUES));
    }
    {{/vendorExtensions.x-enum-as-string}}
    {{#vendorExtensions.x-is-jackson-optional-nullable}}
    this.{{name}} = JsonNullable.<{{{datatypeWithEnum}}}>of({{name}});
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
    {{^vendorExtensions.x-is-jackson-optional-nullable}}
    this.{{name}} = {{name}};
    {{/vendorExtensions.x-is-jackson-optional-nullable}}
  }
  {{/isReadOnly}}
  {{/vars}}
{{>libraries/jersey2/additional_properties}}
  /**
   * Return true if this {{name}} object is equal to o.
   */
  @Override
  public boolean equals(Object o) {
  {{#useReflectionEqualsHashCode}}
    return EqualsBuilder.reflectionEquals(this, o, false, null, true);
  {{/useReflectionEqualsHashCode}}
  {{^useReflectionEqualsHashCode}}
    if (this == o) {
      return true;
    }
    if (o == null || getClass() != o.getClass()) {
      return false;
    }{{#hasVars}}
    {{classname}} {{classVarName}} = ({{classname}}) o;
    return {{#vars}}{{#isByteArray}}Arrays{{/isByteArray}}{{^isByteArray}}Objects{{/isByteArray}}.equals(this.{{name}}, {{classVarName}}.{{name}}){{^-last}} &&
        {{/-last}}{{/vars}}{{#additionalPropertiesType}}&&
        Objects.equals(this.additionalProperties, {{classVarName}}.additionalProperties){{/additionalPropertiesType}}{{#parent}} &&
        super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
    return {{#parent}}super.equals(o){{/parent}}{{^parent}}true{{/parent}};{{/hasVars}}
  {{/useReflectionEqualsHashCode}}
  }
  @Override
  public int hashCode() {
  {{#useReflectionEqualsHashCode}}
    return HashCodeBuilder.reflectionHashCode(this);
  {{/useReflectionEqualsHashCode}}
  {{^useReflectionEqualsHashCode}}
    return Objects.hash({{#vars}}{{^isByteArray}}{{name}}{{/isByteArray}}{{#isByteArray}}Arrays.hashCode({{name}}){{/isByteArray}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}}{{#additionalPropertiesType}}{{#hasVars}}, {{/hasVars}}{{^hasVars}}{{#parent}}, {{/parent}}{{/hasVars}}additionalProperties{{/additionalPropertiesType}});
  {{/useReflectionEqualsHashCode}}
  }
  @Override
  public String toString() {
    StringBuilder sb = new StringBuilder();
    sb.append("class {{classname}} {\n");
    {{#parent}}
    sb.append("    ").append(toIndentedString(super.toString())).append("\n");
    {{/parent}}
    {{#vars}}
    sb.append("    {{name}}: ").append(toIndentedString({{name}})).append("\n");
    {{/vars}}
    {{#additionalPropertiesType}}
    sb.append("    additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
    {{/additionalPropertiesType}}
    sb.append("}");
    return sb.toString();
  }
  /**
   * Convert the given object to string with each line indented by 4 spaces
   * (except the first line).
   */
  private String toIndentedString(Object o) {
    if (o == null) {
      return "null";
    }
    return o.toString().replace("\n", "\n    ");
  }
{{#parcelableModel}}
  public void writeToParcel(Parcel out, int flags) {
{{#model}}
{{#isArray}}
    out.writeList(this);
{{/isArray}}
{{^isArray}}
{{#parent}}
    super.writeToParcel(out, flags);
{{/parent}}
{{#vars}}
    out.writeValue({{name}});
{{/vars}}
{{/isArray}}
{{/model}}
  }
  {{classname}}(Parcel in) {
{{#isArray}}
    in.readTypedList(this, {{arrayModelType}}.CREATOR);
{{/isArray}}
{{^isArray}}
{{#parent}}
    super(in);
{{/parent}}
{{#vars}}
{{#isPrimitiveType}}
    {{name}} = ({{{datatypeWithEnum}}})in.readValue(null);
{{/isPrimitiveType}}
{{^isPrimitiveType}}
    {{name}} = ({{{datatypeWithEnum}}})in.readValue({{complexType}}.class.getClassLoader());
{{/isPrimitiveType}}
{{/vars}}
{{/isArray}}
  }
  public int describeContents() {
    return 0;
  }
  public static final Parcelable.Creator<{{classname}}> CREATOR = new Parcelable.Creator<{{classname}}>() {
    public {{classname}} createFromParcel(Parcel in) {
{{#model}}
{{#isArray}}
      {{classname}} result = new {{classname}}();
      result.addAll(in.readArrayList({{arrayModelType}}.class.getClassLoader()));
      return result;
{{/isArray}}
{{^isArray}}
      return new {{classname}}(in);
{{/isArray}}
{{/model}}
    }
    public {{classname}}[] newArray(int size) {
      return new {{classname}}[size];
    }
  };
{{/parcelableModel}}
{{#discriminator}}
static {
  // Initialize and register the discriminator mappings.
  Map<String, Class<?>> mappings = new HashMap<String, Class<?>>();
  {{#mappedModels}}
  mappings.put("{{mappingName}}", {{modelName}}.class);
  {{/mappedModels}}
  mappings.put("{{name}}", {{classname}}.class);
  JSON.registerDiscriminator({{classname}}.class, "{{propertyBaseName}}", mappings);
}
{{/discriminator}}
}

I mean for Jackson it seems quite pointless to integrate the discriminator but I guess this is the easiest fix. So basically we tell Jackson: Set type due to annotations (which is what we want) and then we tell Jackson ignore one property (on getter) which is the discriminator and we do not care about.

@nickshoe
Copy link

Okay I'm having a different thought about this now after we're trying to use my fix internally. We still want to have a getter method for the discriminator property. So my new idea is to scratch this approach (nonDiscriminatorVars), and instead use the @JsonIgnore annotation to mark the discriminator as ignored for serialization.

Any comments on that?

Hey @keenanpepper, could you give an example where you need a getter for the discriminator property?
You could devise the type of an object by its class.

@nickshoe
Copy link

@keenanpepper please, see also this comment #3796 (comment).

It should be the approach proposed by @davinchia at the beginning of the thread.

We just have to figure out why we would need a discriminator property in the POJO. I cant' imagine a real need for that, or I can think of writing code that uses the object class to infer the discriminator property value.

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.

[BUG] [JavaSpring] Discriminator property gets serialized twice
6 participants