[JAVA] oneOf implementation: wrong discriminator values and discriminator field

[jmzc]

Description

When JSON.java code is generated for an oneOf property, it is set wrong discriminator values,
and discriminator field is set empty

openapi-generator version

openapi-generator-cli-3.0.0 (SNAPSHOT 20180531.120113-109)

OpenAPI declaration file content or url

Person:
      type: object
      properties:
         genre:
            oneOf:
               - $ref: '#/components/schemas/Male'
               - $ref: '#/components/schemas/Female'
            discriminator:
               propertyName: genretype
               mapping:
                  M: '#/components/schemas/Male'
                  F: '#/components/schemas/Female'

Genre:
      type: object
      required:
        - genretype
      properties:
         genretype:
           type: string 
           enum:
            - M
            - F

Male:
     allOf:
      - $ref: '#/components/schemas/Genre'
      - type: object
    
Female:
     allOf:
      - $ref: '#/components/schemas/Genre'
      - type: object

Command line used for generation

java -jar openapi-generator-cli.jar generate \
-i openapi-3.0.yaml \
-l java \
-c java-config.json \
-o ./output

> cat java-config.json
{
  "modelPackage" : "com.test.ws.client.model",
  "apiPackage"   : "com.test.ws.client.api",
  "groupId" : "com.test.ws",
  "invokerPackage" : "com.test.ws.client",
  "artifactId"  : "testws-client",
  "artifact-version": "0.0.1-SNAPSHOT",
  "dateLibrary" : "threetenbp"
}

Steps to reproduce

After generation according step before , JSON.java class includes the next code

GsonFireBuilder fireBuilder = new GsonFireBuilder()
          .registerTypeSelector(Genre.class, new TypeSelector() {
            @Override
            public Class getClassForElement(JsonElement readElement) {
                Map classByDiscriminatorValue = new HashMap();
                classByDiscriminatorValue.put("Male".toUpperCase(), Male.class);
                classByDiscriminatorValue.put("Female".toUpperCase(),Female.class);
                classByDiscriminatorValue.put("Genre".toUpperCase(), Genre.class);
                return getClassByDiscriminator(
                                           classByDiscriminatorValue,
                                           getDiscriminatorValue(readElement, ""));
            }
          })

Related issues/PRs

Improve handling of oneOf #15

Suggest a fix/enhancement

I think that it should be:

• to remove classByDiscriminatorValue.put("Genre".toUpperCase(), Genre.class);
• to set discriminator values ( "M" and "F" , in this case )
• to set a discriminator field ( "genretype" in this case )

So,

GsonFireBuilder fireBuilder = new GsonFireBuilder()
          .registerTypeSelector(Genre.class, new TypeSelector() {
            @Override
            public Class getClassForElement(JsonElement readElement) {
                Map classByDiscriminatorValue = new HashMap();
                classByDiscriminatorValue.put("M".toUpperCase(), Male.class);
                classByDiscriminatorValue.put("F".toUpperCase(),Female.class);
                return getClassByDiscriminator(
                                           classByDiscriminatorValue,
                                           getDiscriminatorValue(readElement, "genretype"));
            }
          })

[jmini]

Thank you a lot for this report!

Maybe we should try to just fix it as you have proposed.

On the long run, as I told in #15 I think that Genre should be an interface. There are also cases where the discriminator is not set at all. (If you have an opinion participate on the long vision discussion).

[jmzc]

On the long run, as I told in #15 I think that Genre should be an interface.

Well, in my example, Genre is a superclass cause by "allOf" property
And openapi-generator do it right

On the other side , I agree with you: "oneOf" should be implemented with an interface

GenreType , in my example (names could not be the best choice )

So:

public interface GenreType {

	  @JsonAdapter(GenreTypeEnum.Adapter.class)
	  public enum GenreTypeEnum {
	    M("M"),
	    F("F");

	    private String value;

	    GenreTypeEnum(String value) {
	      this.value = value;
	    }

	    public String getValue() {
	      return value;
	    }

	    @Override
	    public String toString() {
	      return String.valueOf(value);
	    }

	    public static GenreTypeEnum fromValue(String text) {
	      for (GenreTypeEnum b : GenreTypeEnum.values()) {
	        if (String.valueOf(b.value).equals(text)) {
	          return b;
	        }
	      }
	      return null;
	    }

	    public static class Adapter extends TypeAdapter<GenreTypeEnum> {
	      @Override
	      public void write(final JsonWriter jsonWriter, final GenreEnum enumeration) throws IOException {
	        jsonWriter.value(enumeration.getValue());
	      }

	      @Override
	      public GenreTypeEnum read(final JsonReader jsonReader) throws IOException {
	        String value = jsonReader.nextString();
	        return GenreTypeEnum.fromValue(String.valueOf(value));
	      }
	    }
	  }

	  
	  public GenreTypeEnum getGenretype();
}

And

public class Male extends Genre implements GenreType { 

/*** Other fields ***/

  public static final String SERIALIZED_NAME_GENRETYPE = "genretype";
  @SerializedName(SERIALIZED_NAME_GENRETYPE)
  private GenreTypeEnum genretype = GenreTypeEnum.M;

  public GenreTypeEnum getGenretype() {
    return genretype;
  }

 }

/** getters & setters */

}

I think the discriminator field shouldn't have a settter method ( maybe, discriminator field should be final )

Same for Female clase, except setting

private GenreTypeEnum genretype = GenreTypeEnum.M;

And Person.java

 public static final String SERIALIZED_NAME_GENRE = "genre";
 @SerializedName(SERIALIZED_NAME_GENRE)
 private GenreType genre = null;

And JSON.java

GsonFireBuilder fireBuilder = new GsonFireBuilder()
          .registerTypeSelector(GenreType.class, new TypeSelector() {
            @Override
            public Class getClassForElement(JsonElement readElement) {
                Map classByDiscriminatorValue = new HashMap();
                classByDiscriminatorValue.put("M".toUpperCase(), Male.class);
                classByDiscriminatorValue.put("F".toUpperCase(),Female.class);
                return getClassByDiscriminator(
                                           classByDiscriminatorValue,
                                           getDiscriminatorValue(readElement, "genretype"));
            }
          })

[jmini]

I think the discriminator field shouldn't have a settter method ( maybe, discriminator field should be final )

Well from a OpenAPI Specification point of view, there is nothing that specify that genretype is a constant. For that OAI/OpenAPI-Specification#1313 is requested.