Swagger-codegen: Java: enum element names need to be sanitized

[ePaul]

Summary
When generating Java classes from swagger, for String properties marked with enum a (local) enum class will be generated, with the enum constant names taken from the swagger enum values.
The result does not compile when using names which are not valid Java identifiers.

Reproducible

swagger: '2.0'
info:
  title: Swagger Codegen bug trigger
  description: |
     This demonstrates a bug in swagger-codegen.
  version: 0.0.1
basePath: /api
definitions:
  example:
    type: object
    properties:
      foo:
        type: string
        format: uri
        enum:
          - https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL
          - https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED
          - https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CHANGE
          - https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CLOSED
paths: {}

This generates this Example.java (beside others):

package de.zalando.einkaufshelden.example.model;


import io.swagger.annotations.*;
import com.fasterxml.jackson.annotation.JsonProperty;


@ApiModel(description = "")
@javax.annotation.Generated(value = "class io.swagger.codegen.languages.JaxRSServerCodegen", date = "2015-09-14T14:33:19.379+02:00")
public class Example  {

  public enum FooEnum {
     https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CHANGE,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CLOSED, 
  };
  private FooEnum foo = null;


  /**
   **/
  @ApiModelProperty(value = "")
  @JsonProperty("foo")
  public FooEnum getFoo() {
    return foo;
  }
  public void setFoo(FooEnum foo) {
    this.foo = foo;
  }



  @Override
  public String toString()  {
    StringBuilder sb = new StringBuilder();
    sb.append("class Example {\n");

    sb.append("  foo: ").append(foo).append("\n");
    sb.append("}\n");
    return sb.toString();
  }
}

The enum constants here have not the valid names:

  public enum FooEnum {
     https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CHANGE,  https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CLOSED, 
  };

_Expected behavior_
The generated code should always be compilable (i.e. the names need to be sanitized), in a way that JSON conversion between the protocol value and the enum object should work without problems (both ways).

[cbornet]

The best would probably to generate string enums

public enum FooEnum {
    FOO1("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL"),
    FOO2("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED")
    ;

    private final String text;

    private Strings(final String text) {
        this.text = text;
    }

    @Override
    public String toString() {
        return text;
    }
}

The issue is that we lose the significance of the enum names.
Other solution if possible with the generator : capitalize and filter/replace unwanted characters

public enum FooEnum {
    FOO_HTTPS_PURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DO_PURCHASE_ORDER_STATUS_INITIAL("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL"),
    FOO_HTTPS_PURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DO_PURCHASE_ORDER_STATUS_ORDERED("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED")
    ;

[cbornet]

Also exists on retrofit clients (and probably android-java)

[cbornet]

Edit : this is already fixed on the master branch for java.
This is what is generated

public enum StatusEnum {
  HTTPSPURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DOPURCHASE_ORDER_STATUSINITIAL("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL"), APPROVED("approved"), DELIVERED("delivered");

  private String value;

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

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

So the fix should be ported to other clients (retrofit, android, jaxrs, ...)

[cbornet]

Fixed here for java https://github.com/swagger-api/swagger-codegen/pull/1126

[cbornet]

Opened #1247 for retrofit

[ePaul]

Yes, it seems I did run a not up-to-date version of codegen. The example file now actually generates this Example.java:

package de.zalando.einkaufshelden.example.model;

import io.swagger.client.StringUtil;



import io.swagger.annotations.*;
import com.fasterxml.jackson.annotation.JsonProperty;


@ApiModel(description = "")
@javax.annotation.Generated(value = "class io.swagger.codegen.languages.JavaClientCodegen", date = "2015-09-17T17:12:00.513+02:00")
public class Example   {


public enum FooEnum {
  HTTPSPURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DOPURCHASE_ORDER_STATUSINITIAL("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL"), HTTPSPURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DOPURCHASE_ORDER_STATUSORDERED("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED"), HTTPSPURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DOPURCHASE_ORDER_STATUSCHANGE("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CHANGE"), HTTPSPURCHASE_ORDER_EINKAUFSHELDEN_ZALAN_DOPURCHASE_ORDER_STATUSCLOSED("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CLOSED");

  private String value;

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

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

  private FooEnum foo = null;


  /**
   **/
  @ApiModelProperty(value = "")
  @JsonProperty("foo")
  public FooEnum getFoo() {
    return foo;
  }
  public void setFoo(FooEnum foo) {
    this.foo = foo;
  }



  @Override
  public String toString()  {
    StringBuilder sb = new StringBuilder();
    sb.append("class Example {\n");

    sb.append("    foo: ").append(StringUtil.toIndentedString(foo)).append("\n");
    sb.append("}");
    return sb.toString();
  }
}

Those enum names could still become a bit nicer (the : and / seem to be replaced by nothing?).

[cbornet]

And maybe put a carriage return after each enum value

[wing328]

Agree with @cbornet on adding a carriage return for each enum value.

[xhh]

I'm working on a PR with the improvements on enum names according to the above suggestions. What's more, it would be better to truncate enum names to remove common prefix, for example:

public enum StatusEnum {
  INITIAL("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/INITIAL"),
  ORDERED("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/ORDERED"),
  CHANGE("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CHANGE"),
  CLOSED("https://purchase-order.einkaufshelden.zalan.do/purchase-order-status/CLOSED");

  private String value;

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

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

[wing328]

@ePaul PR merged into master.

[rosaAdl]

This commit breaks simple scenarios... I have the following enum:

subtype:
            type: string
            enum: 
                 - terms and conditions
                 - terms of use

and the incorrectly generated enum is the following:

public enum SubtypeEnum {
    AND_CONDITIONS("terms and conditions"),
    OF_USE("terms of use");

    private String value;

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

Not a fan of this commit :(

[wing328]

@rosaAdl I think we need an option so that developers can choose how the enum values are named when common prefix is found.

[DwayneJengSage]

@rosaAdl @wing328 I dunno if they look at closed issues, so I filed a new one with this issue: https://github.com/swagger-api/swagger-codegen/issues/4261