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

Annotation support should allow @JsonValue/JsonCreator on enum #91

Closed
ashleyfrieze opened this issue Jan 4, 2022 · 7 comments
Closed
Milestone

Comments

@ashleyfrieze
Copy link
Contributor

As suggested in https://cowtowncoder.medium.com/using-jackson-annotations-with-jackson-jr-51087850b95e this would be an important addition to jackson jr.

Our context is that we're using jackson jr in an AWS Lambda in order to reduce code size. We have an open api module which has code-generated Java classes/enums, and we have the following generated code:

public enum VehicleType {
    CAR("Car"),
    MOTORCYCLE("Motorcycle"),
    ...


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

    @JsonCreator
    public static VehicleType fromValue(String value) {

As things stand at the moment, there's no way we can deserialize Car into VehicleType.CAR

@ashleyfrieze
Copy link
Contributor Author

I've hacked together a solution which will bridge the gap for us until a proper fix is available:

public class EnumCompatibleAnnotationExtension extends JacksonJrExtension {

    @Override
    protected void register(ExtensionContext ctxt) {
        ctxt.insertModifier(new EnumCompatibleAnnotationModifier(JsonAutoDetect.Value.defaultVisibility()));
    }

    private static class EnumCompatibleAnnotationModifier extends AnnotationBasedValueRWModifier {

        public EnumCompatibleAnnotationModifier(JsonAutoDetect.Value visibility) {
            super(visibility);
        }

        @Override
        public ValueWriter overrideStandardValueWriter(JSONWriter writeContext, Class<?> type, int stdTypeId) {
            if (stdTypeId == SER_ENUM_ID) {
                return new EnumReaderWriter(type);
            }
            return null;
        }

        @Override
        public ValueReader modifyValueReader(JSONReader readContext, Class<?> type, ValueReader defaultReader) {
            if (type.isEnum()) {
                return new EnumReaderWriter(type);
            }
            return super.modifyValueReader(readContext, type, defaultReader);
        }
    }

    @FunctionalInterface
    private interface ConversionFunction<T, R> {
        R apply(T value) throws IOException;
    }

    private static class EnumReaderWriter extends ValueReader implements ValueWriter {
        private final Map<String, String> enumMap;
        private final ConversionFunction<Object, String> writer;
        private final ConversionFunction<String, Object> reader;

        public EnumReaderWriter(Class<?> type) {
            super(type);
            enumMap = new HashMap<>();
            Field[] fields = type.getDeclaredFields();
            for (Field field : fields) {
                if (field.isAnnotationPresent(JsonProperty.class)) {
                    enumMap.put(field.getName(), field.getAnnotation(JsonProperty.class).value());
                } else {
                    enumMap.put(field.getName(), field.getName());
                }
            }

            Optional<ConversionFunction<Object, String>> jsonValueWriter = getJsonValueWriter(type);

            writer = jsonValueWriter.orElse(value -> enumMap.get(((Enum<?>) value).name()));

            Optional<ConversionFunction<String, Object>> jsonCreator = getJsonCreator(type);
            reader = jsonCreator.orElse(value -> Enum.valueOf((Class) type, value));
        }

        @Override
        public Object read(JSONReader jsonReader, JsonParser parser) throws IOException {
            return reader.apply(parser.getText());
        }

        private Optional<ConversionFunction<Object, String>> getJsonValueWriter(Class<?> type) {
            return Arrays.stream(type.getDeclaredMethods())
                    .filter(method -> method.getDeclaredAnnotation(JsonValue.class) != null)
                    .filter(method -> method.getParameterCount() == 0)
                    .filter(method -> method.getReturnType().equals(String.class))
                    .findFirst()
                    .map(method -> value -> {
                       try {
                           return (String) method.invoke(value);
                       } catch (Exception e) {
                           throw new IOException("Cannot call JsonValue method", e);
                       }
                    });
        }

        private Optional<ConversionFunction<String, Object>> getJsonCreator(Class<?> type) {
            return Arrays.stream(type.getDeclaredMethods())
                    .filter(method -> method.getDeclaredAnnotation(JsonCreator.class) != null)
                    .filter(method -> method.getParameterCount() == 1)
                    .filter(method -> method.getReturnType().equals(type))
                    .findFirst()
                    .map(method -> value -> {
                        try {
                            return (Object) method.invoke(type, value);
                        } catch (Exception e) {
                            throw new IOException("Cannot call JsonCreator method", e);
                        }
                    });
        }

        @Override
        public void writeValue(JSONWriter context, JsonGenerator g, Object value) throws IOException {
            context.writeValue(writer.apply(value));
        }

        @Override
        public Class<?> valueType() {
            return _valueType;
        }
    }
}

This would need more work to make it fully comply with the annotations, and it's targeted only at enum. It's also worth noting that the @JsonProperty support for enum in the current code seems to be one-way only. While the output values are mapped out of the @JsonProperty field, there's no reading based on that value. This solution could be coerced into doing that by producing a second map to reverse the mapping and then using that as the default deserializer method.

@cowtowncoder
Copy link
Member

Adding more support would be great, of course, but @JsonCreator is probably tricky one in general (although in theory perhaps simpler for Enums).
It might make sense to try to add support piece by piece since support for @JsonValue seems like something that would be easier to add? Both generally and for Enums.

On changes: one thing to note is that as late as 2.13, jackson-jr only requires Java 6 (or perhaps Java 7 in practice to run): for build Java 8 is needed.
But I think it might be time to raise the baseline to Java 8 for Jackson 2.14: this would need to be proposed on the dev mailing list but is probably reasonable.

@ashleyfrieze
Copy link
Contributor Author

@cowtowncoder - use of Java 8 in the above solution is just a convenience for our codebase. It could be downgraded to anonymous inner classes, and the use of Optional could be removed.

If you want to give some direction about the scope of this, I could look at coming up with a PR, or you can work from the code I've left here. I'd say that enum support for @JsonValue and @JsonCreator is important, as would be making the current @JsonProperty solution symmetrical.

This whole issue came about because upgrading to the latest jackson-jr broke our dependency on it. It used to perform some sort of caseless matching of enum values (apparently) but doesn't anymore. Hence the need to get @JsonValue and @JsonCreator working for enum in respect of the open api generated code.

Let me know what you need me to do. In general, I prefer to incorporate changes back into the OS library rather than have hacked point solutions on our side. Give me a bit of design guidance, and I can offer a PR if you wish.

@cowtowncoder
Copy link
Member

Right, Java 8 is not a blocker either way. Just thought I'd mention.

As to case-insensitivity; that does not sound right -- I don't think that would have been the case. Nor do I think enum handling has really changed much at all.

Now: I would really like to help with code directly but there is a high chance I just will not have time.
But I will add @JsonValue part on my todo list, we'll see.

@ashleyfrieze
Copy link
Contributor Author

The EnumWriter I copied in the above example is new to the code. The previous version of jackson-jr didn't have it, and was able to marshall Car into CAR - I don't know how, but it was.

@ashleyfrieze
Copy link
Contributor Author

@cowtowncoder I humbly offer a solution in #92. Please advise.

@cowtowncoder cowtowncoder changed the title Annotation support should allow @JsonValue/JsonCreator on 'enum' Annotation support should allow @JsonValue/JsonCreator on enum Jan 10, 2022
cowtowncoder added a commit that referenced this issue Jan 10, 2022
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Jan 10, 2022
@cowtowncoder
Copy link
Member

Merged for inclusion in 2.14.0.

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

2 participants