joelittlejohn / jsonschema2pojo

Generate Java types from JSON or JSON Schema and annotate those types for data-binding with Jackson, Gson, etc
http://www.jsonschema2pojo.org
Apache License 2.0
6.24k stars 1.66k forks source link

JSONB1 style output missing @JsonbCreator for enum #1533

Closed joerg-d-schneider-db closed 1 year ago

joerg-d-schneider-db commented 1 year ago

Hi,

we currently face a problem in our project where we try to switch from Jackson2 annotation style to JSONB1 if it comes to handle enum types.

Given the below json schema, the Jackson2 output creates a @JsonCreator annotation, whereas the JSONB1 output does not have that even though the equivalent @JsonbCreator annotation exists since JSON Binding 1.0.

JSON-Schema (can be tested online)

{
        "type" : "object",
        "properties" : {
            "myEnum" : {
                "type" : "string",
                "enum" : ["one", "two", "three"]
            }
        }
}

Produces Jackson2 output like this

@JsonCreator
public static Example.MyEnum fromValue(String value) {
    Example.MyEnum constant = CONSTANTS.get(value);
    if (constant == null) {
        throw new IllegalArgumentException(value);
    } else {
        return constant;
    }
}

But produces JSONB1 output like this (note the missing @JsonbCreator annotation

public static Example.MyEnum fromValue(String value) {
    Example.MyEnum constant = CONSTANTS.get(value);
    if (constant == null) {
        throw new IllegalArgumentException(value);
    } else {
        return constant;
    }
}

When trying to map the json input string into a Java POJO, we get the following runtime exception:

Caused by: javax.json.bind.JsonbException: Internal error: No enum constant com.example.ClassName.EnumName.Constant
        at org.eclipse.yasson.internal.Unmarshaller.deserializeItem(Unmarshaller.java:68)
        at org.eclipse.yasson.internal.Unmarshaller.deserialize(Unmarshaller.java:51)
        at org.eclipse.yasson.internal.JsonBinding.deserialize(JsonBinding.java:59)
        at org.eclipse.yasson.internal.JsonBinding.fromJson(JsonBinding.java:66)
        ... 34 more
Caused by: java.lang.IllegalArgumentException: No enum constant com.example.ClassName.EnumName.Constant
        at java.lang.Enum.valueOf(Enum.java:238)
        at org.eclipse.yasson.internal.serializer.EnumTypeDeserializer.deserialize(EnumTypeDeserializer.java:37)
        at org.eclipse.yasson.internal.serializer.EnumTypeDeserializer.deserialize(EnumTypeDeserializer.java:23)

Any idea why the @JsonbCreator annotation is missing in the generated JSONB1 class ?

This is currently a blocker for our JSONB1 migration of our code.

Any help appreciated.

Thx

unkish commented 1 year ago

Hi

According to JSR 367: Java™ API for JSON Binding (JSON-B) specification:

3.9 Enum Serialization of an Enum instance to a JSON String value MUST follow the conversion process defined in javadoc specification for their name(). Deserialization of a JSON value into an enum instance MUST be done by calling the enum’s valueOf(String) method.

Thus having given annotation on public static Example.MyEnum fromValue(String value) { would be a NOOP unless chosen JSR367 implementation library does not respect spec.

If you're planning on using a library that does not respect specification or use customizations that need having given annotation, it should be possible to provide own annotator via customAnnotator configuration option.

joerg-d-schneider-db commented 1 year ago

Hi,

may be it helps, if I actually describe what we are trying to achieve and then see if that is possible or not using your plugin. So far we've been using Jackson 2 to convert from json to java and back - the Java classes have been generated using your plugin v 1.2.1 with the Jackson 2 annotation style. That works fine also for enums, as Jackson 2 uses the @JsonCreate and @JsonValue

We are now trying to use JSON-B API and still have Jackson 2 underneath - only thing we'd like to achieve is to use a Java standard API instead of a proprietary Jackson 2 API. Underlying implementation is still Jackson 2 though (as this is supported/provided by our JBoss runtime environment.) So we generated the Java classes from the same json schema using annotation style "jsonb1" (btw, your doc needs to be updated as it mentions "jsonb" as a valid annotation style, but actually "jsonb1" is the right one to use).

Our expectation is that the same conversion happens as was with directly using Jackson 2. In fact no conversion happens as Runtime error is thrown when deserializing enums - is that what the specs mandate ? Thx

unkish commented 1 year ago

I'm unaware if there is an official "JSON-B" support from FasterXML (see issue N19 (JSON-B implementation) under https://github.com/FasterXML/jackson-future-ideas/issues) which theoretically might allow handling of json-b annotations by Jackson. \ @JsonbCreator annotation in enum doesn't work neither with https://github.com/eclipse-ee4j/yasson (JSON-B reference implementation) nor with https://johnzon.apache.org/.


We are now trying to use JSON-B API and still have Jackson 2 underneath

A sample of the stack trace you've provided hints at this not being the case, as we can see that org.eclipse.yasson is being used for deserialization.


Our expectation is that the same conversion happens as was with directly using Jackson 2. In fact no conversion happens as Runtime error is thrown when deserializing enums - is that what the specs mandate ?

To my knowledge Jackson is not JSON-B compliant (can't handle JSON-B annotations), hence spec has nothing to do with that. org.eclipse.yasson would throw exception regardless of @JsonbCreator annotation being present as it adheres to JSON-B spec.

joerg-d-schneider-db commented 1 year ago

Hi,

you are right - the stacktrace is from yasson as we use this lib for our unit test cases (and that is where the stacktrace is from). Nonetheless, we've seen the same exception in our JBoss runtime env which is using Jackson 2. Redhat/Jboss ships with an intermediate layer called "jboss-jackson2-provider" - as per my understanding, that layer handles all the JSON-B stuff(annotations and other objects) and translates that into the resp. Jackson 2 methods, so Jackson itself does not need to support JSON-B.

The basic problem we see with yasson as well as Jboss/Jackson2 is the following as per my understanding -

We have an enum that defines a constant as "CONSTANTNAME("constantname")"

The Json message we consume has a string as "constantname" and during desrialization, that string should be mapped to that enum. Problem is, that the mapping method gets the string with all lower case letters (=constantname) and tries to find a resp. constant in the enum - that doesn't succeed because the constant in the enum is "CONSTANTNAME" => "No enum constant constantname" error.

I double checked by manually modifying the string from "constantname" into "CONSTANTNAME" and then the error is gone and mapping works. Unfortunately we receive those messages from upstream system and they can't/won't change the string contant to be all upper case letters so it matches the enum constant.

Any idea how to solve that problem ?

joerg-d-schneider-db commented 1 year ago

or even simpler - why does the plugin generate enums using all uppercase letters, even though the json schema is using lower case ? The sample given above

"myEnum" : {
                "type" : "string",
                "enum" : ["one", "two", "three"]

result in the generated enum of

@Generated("jsonschema2pojo")
public enum MyEnum {

ONE("one"),
TWO("two"),
THREE("three");

If the generated code would look lie this, it would work

@Generated("jsonschema2pojo")
public enum MyEnum {

one,
two,
three;

Any idea how that can be achieved ?

Thx

unkish commented 1 year ago

Any idea how that can be achieved ?

Please see #2. below.


Unfortunately we receive those messages from upstream system and they can't/won't change the string contant to be all upper case letters so it matches the enum constant.

I see 4 possibilities:

  1. consider dropping the idea of migrating to json-b (current pitfall might be not the last one you'd be encountering)
  2. adjust by either
  3. use yasson/some other json-b library and explore possibility to customize serialization/deserialization behavior eg. https://stackoverflow.com/questions/54290020/ignore-enum-case-with-json-b-yasson
  4. check if there are other plugins available that would work out of the box for you
joelittlejohn commented 1 year ago

Hi @joerg-d-schneider-db. Could you check what actually happens when you add the JsonbCreator annotation on the fromValue method? My guess is that it does not help.

It looks like the right solution here is to declare a new class that implements javax.json.bind.serializer.JsonbDeserializer and then add the JsonbTypeDeserializer on the enum type.

joerg-d-schneider-db commented 1 year ago

Hi,

thanks for your comments and help so far - very much appreciated :-) I'll do the manual changes as suggested and will let you know the outcome.

I spent some more time on analyzing what the root cause of the problem is and imo, at least if it comes to enums, different frameworks simply use different approaches to delegate serialization/deserilaization to custom methods.

Jackson uses @JsonValue and @JsonCreator annotations for this and this can also be found in the generated code. Other frameworks like JSONB (or Gson or Moshi ...) seem to support the concept of a Json Adapter interface that can be used to achieve the same custom methods (see https://rmannibucau.metawerx.net/jsonb-enum-serialization.html )

Afaik, JSONB support has been added based on the Jackson 2 implementation - that again does not know about a Json Adapter interface. Handling enums in JSONB could possibly be similar as it is done for Gson or Moshi impl, but definitely doesn't work in the current implementation as no customized methods exist and thus java.lang.Enum methods are being used which throws runtime exception.

I'll also do a further test and remove @JsonValue and @JsonCreator annotation using Jackson 2 and would expect it throws the same runtime exception as currently thrown by JSONB.

Will keep ypu updated - thx

joelittlejohn commented 1 year ago

If we need to create a static inner class on every enum, that implements javax.json.bind.serializer.JsonbDeserializer, and annotate the enum to reference that class, we can easily do that. This can be active only when using jsonb.

It's a real shame the spec creators didn't think to just use the JsonbCreator annotation on a deserialization method :)

joerg-d-schneider-db commented 1 year ago

If we need to create a static inner class on every enum, that implements javax.json.bind.serializer.JsonbDeserializer, and annotate the enum to reference that class, we can easily do that. This can be active only when using jsonb.

It's a real shame the spec creators didn't think to just use the JsonbCreator annotation on a deserialization method :)

Serialization would also need to use custom methods imo, not just deserialization - same way as it is needed for the Jackson 2 impl

joelittlejohn commented 1 year ago

Yes true, so I guess a javax.json.bind.serializer.JsonbSerializer too :)

joerg-d-schneider-db commented 1 year ago

I tried out the @JsonbCreator annotation on the static fromValue method like this

        @JsonbCreator()
        public static CoreTradeData.EventType fromValue(String value) {
            CoreTradeData.EventType constant = CONSTANTS.get(value);
            if (constant == null) {
                throw new IllegalArgumentException(value);
            } else {
                return constant;
            }
        }

That doesn't help as still the same exception "No enum constant ..." is thrown. I guess the meaning of that annotation is different than the one in use by Jackson. In JSONB, the @JsonbCreator annotation apparently is required for constructors with parameters, but not just as a replacement for the default constructor.

I also tried out to use inner classes to implement either a JsonbAdapter or JsonbDeserializer, but no success as the JSONB implementation don't find the default constructor of that class as it needs to be referenced using the outer class name + inner class name. Using a regular class as adapter/deserializer seems to work - at least the methods are being called ;-)

I don't know if my expertise is good enough to implement either an adapter or de/serializer, but will give my best ;-)

Will keep you posted.

Thx

joerg-d-schneider-db commented 1 year ago

Managed to get the problem fixed at least for one enum deserialization

import java.lang.reflect.Type;

import javax.json.bind.serializer.DeserializationContext;
import javax.json.bind.serializer.JsonbDeserializer;
import javax.json.stream.JsonParser;

import com.db.cse.trade.domain.sdp.CoreTradeData.EventType;

public class EventTypeMapper implements JsonbDeserializer<EventType> {

    @Override
//    public CoreTradeData.EventType adaptFromJson(JsonObject adapted) throws Exception {
  public EventType deserialize(JsonParser parser, DeserializationContext ctx, Type rtType) {
        System.out.println("Inside adaptFromJson!");
        String value = parser.getString();
        if(value != null) {
            System.out.println("found value for event type <" + value + ">");
            return EventType.fromValue(value);
        } else {
            System.out.println("value is null");                        
        }
        return null;
    }           

}
joerg-d-schneider-db commented 1 year ago

Serialization might work like this

    @Override
    public void serialize(EventType eventType, JsonGenerator generator, SerializationContext ctx) {
        generator.write("eventType", eventType.value());
    }

I have to admit I can't fully test it as we have 20+ different enums in our json. Unfortunately I was not bale to implement a generic class that could be used for all of them, but yet have to implement one by one.

May be there is a way to get away with a single class only using reflection ?

Thx

joerg-d-schneider-db commented 1 year ago

Hi,

found that link https://github.com/FasterXML/jackson-databind/issues/677 - apparently Jackson v 2.6 supports @JsonProperty to define a vlue to be used for de/serialization. That would make that custom mapping obsolete imo.

joelittlejohn commented 1 year ago

But we don't need a solution for Jackson, right? The current approach using JsonCreator and JsonValue annotations works correctly.

joerg-d-schneider-db commented 1 year ago

I finally made it somehow - at least works for me

I've put the following inner class into the generated enum with name "EventType" :

    public static class EventTypeMapper implements JsonbAdapter<EventType, String> {
        @Override
        public String adaptToJson(final EventType obj) {
            return obj.value();
        }
        @Override
        public EventType adaptFromJson(final String obj) {
            return EventType.fromValue(obj);
        }
    }

In the surrounding class I add the following annotation

@JsonbTypeAdapter(CoreTradeData.EventType.EventTypeMapper.class)
@JsonbProperty("eventType")
private CoreTradeData.EventType eventType;

works for me and should be easy to implement as only an additional annotation is required for each enum and a static inner class

Any comments ?