Skip to content
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

Enum types should generate a schema that includes the possible enum values #57

Closed
StormeHawke opened this issue Jan 9, 2015 · 19 comments
Milestone

Comments

@StormeHawke
Copy link

Cross posting from my StackOverflow question. After doing some digging it seems like the schema is defaulting to using the bare serialized type rather than properly using the possible enum values.

Link to the question on StackOverflow:
http://stackoverflow.com/questions/27863689/generate-json-schema-from-pojo-with-a-twist

I'm generating a JSON schema from a pojo. My code to generate the schema looks like so:

ObjectMapper mapper = new ObjectMapper();
TitleSchemaFactoryWrapper visitor = new TitleSchemaFactoryWrapper();
mapper.acceptJsonFormatVisitor(clazz, visitor);
JsonSchema schema = visitor.finalSchema();
schemas.put(clazz, mapper.writerWithDefaultPrettyPrinter().writeValueAsString(schema));

I'm generating several schemas via the above code. One of the pojos has an internal embedded enum to limit the possible values, like so:

public class MyClass {

    @JsonProperty("name")
    private String name;
    @JsonProperty("startDayOfWeek")
    private MyClass.StartDayOfWeek startDayOfWeek;
    /**
     * The ID of a timezone returned by the timezones route.
     * 
     */
    @JsonProperty("timezone")
    private String timezone;
    @JsonIgnore
    private Map<String, Object> additionalProperties = new HashMap<String, Object>();

    /**
     * 
     * @return
     *     The startDayOfWeek
     */
    @JsonProperty("startDayOfWeek")
    public MyClass.StartDayOfWeek getStartDayOfWeek() {
        return startDayOfWeek;
    }

    /**
     * 
     * @param startDayOfWeek
     *     The startDayOfWeek
     */
    @JsonProperty("startDayOfWeek")
    public void setStartDayOfWeek(MyClass.StartDayOfWeek startDayOfWeek) {
        this.startDayOfWeek = startDayOfWeek;
    }

    public static enum StartDayOfWeek {

        MONDAY("Monday"),
        TUESDAY("Tuesday"),
        WEDNESDAY("Wednesday"),
        THURSDAY("Thursday"),
        FRIDAY("Friday"),
        SATURDAY("Saturday"),
        SUNDAY("Sunday");
        private final String value;
        private static Map<String, MyClass.StartDayOfWeek> constants = new HashMap<String, MyClass.StartDayOfWeek>();

        static {
            for (MyClass.StartDayOfWeek c: values()) {
                constants.put(c.value, c);
            }
        }

        private StartDayOfWeek(String value) {
            this.value = value;
        }

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

        @JsonCreator
        public static MyClass.StartDayOfWeek fromValue(String value) {
            MyClass.StartDayOfWeek constant = constants.get(value);
            if (constant == null) {
                throw new IllegalArgumentException(value);
            } else {
                return constant;
            }
        }

    }

}

The above code should limit the possible String values in the JSON data that's passed around to "Monday", "Tuesday", "Wednesday", etc.

When I run the schema generator on the code in question, I expect to get something like the following schema:

{
  "type" : "object",
  "javaType" : "my.package.MyClass",
  "properties": {
    "startDayOfWeek" : {
      "type" : "string",
      "enum" : [ "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday" ]
    }
  }
}

but instead I'm getting this:

{
  "type" : "object",
  "id" : "urn:jsonschema:my:package:MyClass",
  "title" : "Lmy/package/MyClass;",
  "properties" : {
    "startDayOfWeek" : {
      "type" : "string"
    }
  }
}

I've done some digging in the Jackson Schema Module source code and figured out that what's happening is Jackson's using ".toString()" as the default serialization method for enum types, but what I need it to do instead is create the line that looks like this based on StartDayOfWeek.values():

"enum" : [ "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday" ]
@cowtowncoder
Copy link
Member

Yes, I can reproduce this with 2.5.1-SNAPSHOT. Not sure what gives; EnumSerializer should be passing proper name. Will investigate more.

@cowtowncoder
Copy link
Member

Actually, hmmh. Not sure I can reproduc this. Are you using the latest versions of databind and JSON Schema module? 2.4.4 actually had one fix that may be relevant here; so either 2.4.4 or 2.5.0 would be versions to test. I modified an existing test to ensure that Enum.value() should be used (use of @JsonValue may still be problematic however) is used, not Enum.toString() (except if configured to use that).

@cowtowncoder cowtowncoder added this to the 2.4.4 milestone Jan 13, 2015
@StormeHawke
Copy link
Author

Specifically, what needs to happen is enums should be encoded (I think starting in java 1.6) using an array of all the enum values via ENUMVALUE.toString(). I believe prior to 1.6 it would need to be via ENUMVALUE.name(), though I would have to do some research to confirm. I'm using java 8 so an array of all the ENUMVALUE.toString()s would be perfect

@StormeHawke
Copy link
Author

@cowtowncoder Can you reproduce using the MyClass class that I included in the bug report?

I'm still seeing it with jackson 2.5.0

    <dependency>
        <groupId>com.fasterxml.jackson.core</groupId>
        <artifactId>jackson-databind</artifactId>
        <version>2.5.0</version>
    </dependency>
    <dependency>
        <groupId>com.fasterxml.jackson.module</groupId>
        <artifactId>jackson-module-jsonSchema</artifactId>
        <version>2.5.0</version>
    </dependency>

@cowtowncoder
Copy link
Member

Jackson default method for serializing Enums IS to use Enum.name().
And as such, result in JSON Schema should do exactly the same. It is possible to configure ObjectMapper to use toString() instead, with SerializeFeature.WRITE_ENUMS_USING_TO_STRING (or similar, I forget exact name).

So: what test needs to demonstrate is that JSON Schema includes values different from serialization.

Put another way: Jackson should produce Schema that is compatible with its own serialization method.

@StormeHawke
Copy link
Author

Ok so I just cloned jackson-databind and attempted to duplicate by modifying TestGenerateJsonSchema's SimpleBean class to have a simple enum with values and tested the schema validation and that, oddly enough, generated exactly the expected output. However, if I plop my own little class down in that package and write a new test based on that class, the proper enum schema is NOT generated. Still digging to see if I can figure out what is special about my enum that is breaking it

@StormeHawke
Copy link
Author

Found it.
If I comment out @JsonValue like so:

    //@JsonValue
    @Override
    public String toString() {
        return this.value;
    }

Then the schema generates as expected.

@StormeHawke
Copy link
Author

Ok, I have a test you can use to reproduce. Modify the following file:
/jackson-databind/src/test/java/com/fasterxml/jackson/databind/jsonschema/TestGenerateJsonSchema.java

Add the following nested class:

public static class BeanWithEmbeddedEnum {
    private TestEnum enumProperty;

    public TestEnum getEnumProperty() {
        return enumProperty;
    }

    public void setEnumProperty(TestEnum enumProperty) {
        this.enumProperty = enumProperty;
    }

    public static enum TestEnum {
        A("A_Value"), B("B_Value"), C("C_Value");

        public final String value;

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

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

Then add the following test:

public void testGeneratingJsonSchemaWithEmbeddedEnum()
        throws Exception
{
    JsonSchema jsonSchema = MAPPER.generateJsonSchema(BeanWithEmbeddedEnum.class);
    ObjectNode root = jsonSchema.getSchemaNode();
    JsonNode propertiesSchema = root.get("properties");

    JsonNode enumPropertySchema = propertiesSchema.get("enumProperty");
    assertNotNull(enumPropertySchema);
    assertEquals("string", enumPropertySchema.get("type").asText());
    JsonNode enumNode = enumPropertySchema.get("enum");
    assertNotNull(enumNode);
    assertTrue(enumNode.isArray());
    assertEquals("[\"A\",\"B\",\"C\"]", enumNode.toString());
}

With the @JsonValue annotation present the test will fail, with it commented out it will pass

@cowtowncoder
Copy link
Member

Ok, just to make sure: it looks like @JsonValue has effect when serializing, but NOT with JSON Schema generation? This sounds plausible, and a bug at that.

@StormeHawke
Copy link
Author

Not sure if I read you right, so I'll rehash it just to make sure we're saying the same thing:

@JsonValue _should_ have an effect when serializing a class instance. It _should not_ have an effect during schema generation, because the schema's purpose in the case of an enum is to define which values are valid. Hopefully this helps

@cowtowncoder
Copy link
Member

No, I don't think this is how I see it. To me, JSON Schema generation should produce schema that validates actual output serializer would do. Why would definition of valid for schema be different from actual produced output? Keep in mind that JSON Schema knows nothing about datatypes or objects, and only about actual literal JSON serialization -- enumerated Strings that may be found in instance documents.

@jcogilvie
Copy link

I think I see a communication disconnect here. (Hi there, I work with @StormeHawke.)

If my enum has:

enum MyEnum {
   FOO(1),
   BAR(2),
   BAZ(3); 

public MyEnum(int ordinal) {...}

@JsonValue
public String toString() {...}

public int ordinal() {...}

}

I expect the json schema to be:
enum: [ FOO, BAR, BAZ ]

If instead I have:

enum MyEnum {
   FOO(1),
   BAR(2),
   BAZ(3); 

public MyEnum(int ordinal) {...}

public String toString() {...}

@JsonValue
public int ordinal() {...}

}

I expect my json schema to be:
enum: [1, 2, 3]

In this way, I expect @JsonValue to have an effect on the output, and the output remains valid json schema. What I don't expect is for @JsonValue to suggest that the schema should be anything other than an enum (right now, it gets schemafied as a string instead of an enum if @JsonValue is present on toString()).

So in sum: @JsonValue should have an effect on the contents of the enumeration in the schema. It should not have an effect on the type that is output to the schema. Have I got that right?

@StormeHawke
Copy link
Author

That explains it perfectly.

@cowtowncoder cowtowncoder removed this from the 2.4.4 milestone Jan 16, 2015
@cowtowncoder
Copy link
Member

Ah! Ok, I did miss some parts. I agree here, I think. Thank you for clarifying this!

One thing that would help here is a simple unit test: I will eventually get to it and should be able to write one. But a contribution could speed things up.

@cowtowncoder
Copy link
Member

Ah-ha. I can reproduce this, and the root problem is that @JsonValue translates into different kind of serializer.... i.e. "forgets" it was an enum.

@cowtowncoder
Copy link
Member

Ok. This will get rather tricky to fix, but is probably doable. Requires special handling of Enum values on databind side, most likely within JsonValueSerializer. I can add a failing unit test for JSON Schema for now.

cowtowncoder added a commit that referenced this issue Jan 23, 2015
@Fillius
Copy link

Fillius commented Apr 7, 2015

+1 I also have this issue using Jackson version 2.5.1

@cowtowncoder
Copy link
Member

Ok. Needed a brutal hack in jackson-databind, but this will work in 2.6.0.

@cowtowncoder
Copy link
Member

Hmmh. Looking back at this, I am actually not sure this is not something to support, because meaning of @JsonValue gets mixed up here. In fact there probably should have been different way to express this than use of @JsonValue -- for all other types, schema will and should reflect the returned type and NOT the type of class that has it.

But given that this is the current behavior I guess it'll have to do. Just noticed the problem when rewriting schema traversal for @JsonValue.

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

No branches or pull requests

4 participants