jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

read list value by element java types

Open zrlw opened this issue 1 year ago • 6 comments

#3592

zrlw avatar Sep 08 '22 11:09 zrlw

(note: I will be calling this feature "tuples" because that's what they are)

im not a fan of this approach. imo adding new methods to ObjectMapper for this is not very flexible (needs duplication from other read methods, does not give a way to define such a "tuple" using an annotation). also, adding a new method to JsonDeserializer is weird, i don't really see how subclasses should implement it.

i can think of two approaches to get the same feature without these drawbacks:

  • we could introduce a special JavaType for tuples, with a special JsonDeserializer. this seems like an easy solution, though it means that the new JavaType does not have a match in the java language, which is a bit weird.
  • alternatively, we could adjust ObjectDeserializer to be able to deserialize objects from a non-object json "shape". the user would define a tuple class with the fields of the different types they want, and ObjectDeserializer would parse this tuple class from a json array instead of from a json object.

yawkat avatar Sep 08 '22 12:09 yawkat

Ah, I just looked at how the kotlin module does this, and it turns out my second suggestion is already implemented: https://github.com/FasterXML/jackson-module-kotlin/issues/72 – you can annotate a class with an array shape, and it will be deserialized exactly like you want. imo this is already a good solution for the tuple feature, with the added benefit of better type safety.

    public static void main(String[] args) throws JsonProcessingException {
        TestTuple tuple = new TestTuple();
        tuple.integer = 1;
        tuple.bigDecimal = new BigDecimal("2");
        tuple.l = 3L;
        tuple.s = null;

        ObjectMapper objectMapper = new ObjectMapper();
        String json = objectMapper.writeValueAsString(tuple);
        // json == [1,2,3,null]
        objectMapper.readValue(json, TestTuple.class); // returns same tuple
    }

    @JsonFormat(shape = JsonFormat.Shape.ARRAY)
    public static class TestTuple {
        public Integer integer;
        public BigDecimal bigDecimal;
        public long l;
        public String s;
    }

yawkat avatar Sep 08 '22 12:09 yawkat

it is weird to define a annotated class that includes the parameter types of each rpc method for generized service adaptation.

zrlw avatar Sep 08 '22 14:09 zrlw

Hmmh. First of all, thank you for submitting this contribution. I can see why someone might want Tuples.

My initial reaction is that this is too intrusive, adding new method for many deserializers to implement. It also seems like there should be something to denote proper Tuple types.

One suggestion here, related to @yawkat's concern is that ObjectMapper is probably the wrong place for this -- methods should be added (only) in ObjectReader. Type list/array could be added there; and in fact I think it should be possible to keep handling of Tuple fields within ObjectReader and not require new method(s) in JsonDeserializer.

So I think I would be more amenable to such an approach: configuring ObjectReader with List/array of JavaTypes (with possible convenience method of Class[] alternative), and then readValue() taking that into account (if "type List" is defined).

cowtowncoder avatar Sep 09 '22 02:09 cowtowncoder

introduce a special JavaType for tuples: #3597

zrlw avatar Sep 09 '22 08:09 zrlw

One suggestion here, related to @yawkat's concern is that ObjectMapper is probably the wrong place for this -- methods should be added (only) in ObjectReader. Type list/array could be added there; and in fact I think it should be possible to keep handling of Tuple fields within ObjectReader and not require new method(s) in JsonDeserializer.

changed. but it seemed not to be elegant as lots of codes was copyed from CollectionDeserializer to ObjectReader just for avoiding to require new method in JsonDeserializer.

maybe #3597 is better.

zrlw avatar Sep 11 '22 05:09 zrlw