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

@JsonUnwrapped not supported for Map-valued properties #171

Closed
stevenschlansker opened this issue Feb 20, 2013 · 34 comments
Closed

@JsonUnwrapped not supported for Map-valued properties #171

stevenschlansker opened this issue Feb 20, 2013 · 34 comments
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action will-not-fix Closed as either non-issue or something not planned to be worked on

Comments

@stevenschlansker
Copy link

According to the documentation,

Annotation used to indicate that a property should be serialized "unwrapped"; that is, if it would be serialized as JSON Object, its properties are instead included as properties of its containing Object.

Unfortunately, this seems to work only with bean types, and does not work with Map<String, Object>. Given that bean types are generally completely interchangeable with Maps, it would be very nice if this worked correctly.

@cowtowncoder
Copy link
Member

Hmmh. Unfortunately Maps and POJOs are internally handled rather differently. But I am all for unification if it is technically possible, so limitation is not philosophical, just practical. I suspect that serialization part should be easy enough to make work; deserialization might get trickier?

@stevenschlansker
Copy link
Author

It would be very cool if this worked 😄

@cowtowncoder
Copy link
Member

:-)

One quick question: I assume most users would expect it to work both ways. But I suspect serialization is much easier to make work. Would you find any value in serialization-only impl? (especially initial one)

One more thing: as per this:

http://www.cowtowncoder.com/blog/archives/2011/07/entry_458.html

one can use @JsonAnyGetter/setter to do something possibly similar. One missing pieces is that currently one must have getter (can't use it on Map filed), but that should be easy enough to address.

@stevenschlansker
Copy link
Author

Yes, a serialization-only implementation would solve my immediate use case, although I can see the deserialization as being extremely useful as well.

It does seem that this could potentially be done with serialization-only JsonUnwrapped usage and then JsonAnySetter to handle the deserialization case, although that feels more than a little bit janky to me.

@cowtowncoder
Copy link
Member

Yes my concern is really with unexpected one-way street: unwrapping on way out, but not "wrapping it back" as Map on way back in.

@cowtowncoder
Copy link
Member

Ok, I think supporting this would be very useful for CSV, f.ex see FasterXML/jackson-dataformat-csv#25
so maybe I should go back, try to tackle this.

Although, with CSV there are other open questions due to name/index mapping. But still, solving this on databind side could help.

@mmadson
Copy link

mmadson commented May 27, 2015

+1

1 similar comment
@igorcadelima
Copy link

+1

@cowtowncoder
Copy link
Member

A related note for anyone who happens upon this issue: one alternative is use of @JsonAnyGetter, which does allow functionality for a single Map.

@igorcadelima
Copy link

Great! @JsonAnyGetter is exactly what I was looking for. Thanks!

christophercurrie pushed a commit to christophercurrie/jackson-databind that referenced this issue Jul 19, 2015
@cowtowncoder cowtowncoder changed the title JsonUnwrapped does not seem to work correctly with Map typed values @JsonUnwrapped not supported for Map-valued properties Dec 11, 2015
@ghost
Copy link

ghost commented Jan 24, 2017

I find this behavior surprising at a conceptual level, since it seems to be contrary to how Jackson behaves in general.

Would I be correct in assuming that this is one of the features that will likely not be in the next Jackson 2.9 release, per the Changes likely to be postponed section?

@cowtowncoder
Copy link
Member

@matthew-pwnieexpress You are correct in that no work is planned for this feature.
I can see how this is unexpected from users perspective: difference is due to technical evolution of backend implementation where Maps and POJOs have very different handling. But conceptually this should not surface quite this strongly, or, if it does, would need to be explained and documented much better.

@henryptung
Copy link

@cowtowncoder Given that @JsonAnyGetter and @JsonAnySetter exist, how correct/incorrect would an implementation be to have @JsonUnwrapped imply the other two annotations when used with MapSerializer?

@cowtowncoder
Copy link
Member

@henryptung Interesting thought... hmmh. There is also then the possible question of what if more than one such "any property" is declared.
I guess this is not problematic with POJOs as their properties are somewhat defined and multiples unwrapped POJOs may co-exist; whereas any-x is fallback.

@marceloverdijk
Copy link

marceloverdijk commented Apr 15, 2018

Unfortunately the @JsonAnyGetter does not work (easily) with Kotlin:

class Links(
        @JsonAnyGetter
        val links: Map<String, Link>
) : Serializable

as the annotation needs to put on a method.

!! UPDATE !!

It does work (easily):

class Links(
        @get:JsonAnyGetter
        val links: Map<String, Link>
) : Serializable

@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 16, 2018

True; as of now (2.9), @JsonAnyGetter must be on method. In theory it could be extended to fields, to allow construction of Map instance. One challenge would be the fact that semantics would be slightly different -- in one case method gets fed key/value pair: that could lead to confusion (unless logic further added to perhaps also allow single Map-argument... but that can lead to another can of worms).

EDIT (2023-05-15): @JsonAnyGetter is available on Fields too (see #1458) since 2.12.

@odrotbohm
Copy link

We face the challenge that we provide a wrapper type for some content object that could either be a custom object or a Map:

class SomeWrapper<T> {

  T content;

  @JsonUnwrapped
  public getContent() {
    return content;
  }
}

Is there something we could do using a custom serializer to not have to add the extra method or extra class?

odrotbohm added a commit to spring-projects/spring-data-rest that referenced this issue Nov 29, 2019
…ces.

The usage of text/uri-list as media type was entirely broken and not even advertised in the reference docs anymore. It's now again supported for both to-one and to-many associations via Collections. Maps are rejected as they cannot be rendered as list of URIs correctly. Updated reference documentation accordingly.

Added a custom MapModel implementation of RepresentationModel as apparently using Maps with EntityModel does not unwrap the content properly due to [0].

[0] FasterXML/jackson-databind#171
odrotbohm added a commit to spring-projects/spring-data-rest that referenced this issue Nov 29, 2019
…ces.

The usage of text/uri-list as media type was entirely broken and not even advertised in the reference docs anymore. It's now again supported for both to-one and to-many associations via Collections. Maps are rejected as they cannot be rendered as list of URIs correctly. Updated reference documentation accordingly.

Added a custom MapModel implementation of RepresentationModel as apparently using Maps with EntityModel does not unwrap the content properly due to [0].

[0] FasterXML/jackson-databind#171
@cowtowncoder
Copy link
Member

@odrotbohm I can't think of anything: @JsonUnwrapped is quite tied to way BeanSerializer and -Deserializer works and although one can override handlers I am not sure custom (de)serializer route would lead to anything but fragile solution.

@odrotbohm
Copy link

I have a POC with the following steps:

  • register a custom StdSerializer<SomeWrapper>
  • in that, inspect the content. If it's a Map take the content and wrap it into a type using @JsonAnyGetter, default serialize that. If no, wrap into an almost copy of SomeWrapper using a plain @JsonUnwrapped

That way, SomeWrapper stays the only user facing API but does the right thing™ during serialization.

@mjustin
Copy link

mjustin commented Nov 16, 2020

Incidentally, the Stack Overflow question for this issue has 46 upvotes, and its top answer has 101 upvotes, so this is definitely an issue users have been encountering.

@cowtowncoder cowtowncoder added 2.13 most-wanted Tag to indicate that there is heavy user +1'ing action labels Nov 16, 2020
@cowtowncoder
Copy link
Member

Ok, marking this as "most-wanted" as there are a few thumbs-ups here too.

About the only question I have is whether there are some specific differences between just using @JsonAnyGetter/@JsonAnySetter combo, and potential @JsonUnwrapped.
Using one less annotation seems like nice-but-not-essential; consistency a minor plus too.
Or put another way: if @JsonUnwrapped was essentially implemented as sort of alias for "any-getter", would that work?
(another potential concern: can only have one "any getter/setter" per class -- but multiple @JsonUnwrappeds)

@Vroo
Copy link

Vroo commented Jan 5, 2021

It's also very surprising it doesn't work for ObjectNodes, e.g., the trivial case JsonNodeFactory.instance.objectNode().put("sample", 1). I would guess that's because under the covers, ObjectNode is a map, but to someone writing that code, it's just a json object. And the @JsonAnyGetter trick doesn't work for this.

@cowtowncoder
Copy link
Member

@Vroo Conceptually, JsonNode types are not POJOs, so most annotations do not have any effect by design. It would be good to document this better as I can see why it might be surprising... there isn't much documentation on concept, intended differences. In fact, POJOs, "untyped" (Object / List / Map), Trees (JsonNode) are all somewhat different models within Jackson, and while they interoperate fine, they are not treated the same way.

@cowtowncoder
Copy link
Member

@Vroo that said, I can see why specific cases of @JsonAnyGetter/@JsonAnySetter would make sense for JsonNode/ObjectNode values -- if so, feel free to file an issue to add support for that usage.

@JsonUnwrapped is a bit trickier just because the whole machinery for it to work is... rather complicated and fragile: you could file an issue for that, too (since adding support for Maps would be technically different from JsonNode; there isn't much synergy in getting both implemented)

@cowtowncoder cowtowncoder removed the 2.13 label Apr 15, 2021
@Druckles
Copy link

Druckles commented Jun 16, 2021

About the only question I have is whether there are some specific differences between just using @JsonAnyGetter/@JsonAnySetter combo, and potential @JsonUnwrapped.
...
Or put another way: if @JsonUnwrapped was essentially implemented as sort of alias for "any-getter", would that work?

I have the same problem as @odrotbohm: @JsonUnwrapped sits on top of a generic. To get it to work, I've needed to create subclass that overrides the getter so I can add @JsonAnyGetter. The any-getter doesn't work because it's of type T, not a Map and it throws an error during runtime otherwise.

This creates other problems because I've subclassed it. Being able to use @JsonUnwrapped would be much easier, cleaner and safer.

@Akaame
Copy link

Akaame commented Mar 7, 2022

I had a go about this and could get Unwrapping aspect working. The JsonSerializer<T> has a unwrappingSerializer method that is called within UnwrappingBeanPropertyWriter that sort of lifts the serializer into a unwrapping serializer. When we try to serialize a Map instance a MapSerializer is responsible for the serialization step. Overriding unwrappingSerializer for MapSerializer to create a UnwrappingMapSerializer should give us what we want.

@Override
public JsonSerializer<Map<?, ?>> unwrappingSerializer(NameTransformer transformer) {
    return new UnwrappingMapSerializer(this, transformer);
}

The sole responsibility of an UnwrappingMapSerializer is making MapSerializer's keySerializer an unwrapping serializer.

this._keySerializer = this._keySerializer.unwrappingSerializer(transformer);

This changes the result of TestUnwrappedMap171.testMapUnwrapSerialize

junit.framework.ComparisonFailure: 
Expected :{"map.test": 6}
Actual   :{"test":6}

As you can see unwrapping worked but the name transformer did not have any effect as StdKeySerializers do not have a rename concept as BeanPropertyWriters do.

@fprochazka
Copy link

fprochazka commented Aug 3, 2022

Hi, I'd like to add here my use-case:

ugly Data
@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
public static final class Data
{

    @JsonProperty("Status")
    private String status;

    @JsonAnyGetter
    private Map<String, Object> otherFields = new LinkedHashMap<>();

    public Data(
        final String status,
        final Map<String, Object> otherFields
    )
    {
        this.status = status;
        this.otherFields = otherFields;
    }

    public Data()
    {
    }

    @JsonAnySetter
    public void anySetter(final String key, final Object value)
    {
        otherFields.put(key, value);
    }

    @Override
    public boolean equals(final Object o)
    {
        if (this == o) {
            return true;
        }
        if (!(o instanceof Data data)) {
            return false;
        }
        return status.equals(data.status) && otherFields.equals(data.otherFields);
    }

    @Override
    public int hashCode()
    {
        return Objects.hash(status, otherFields);
    }

    @Override
    public String toString()
    {
        return "{status='" + status + '\'' + ", otherFields=" + otherFields + '}';
    }

}
pretty Data
public record Data(
    @JsonProperty("Status") String status,
    @JsonUnwrapped Map<String, Object> otherFields
)
{

}
OtherFieldsJacksonTest
class OtherFieldsJacksonTest
{

    private final ObjectMapper jsonObjectMapper = Jackson2ObjectMapperBuilder.json()
        .featuresToEnable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
        .build();

    @Test
    public void serialize() throws Exception
    {
        var other = new LinkedHashMap<String, Object>();
        other.put("list", List.of(1, 2, 3));
        other.put("Id", "1234");
        other.put("object", Map.of("foo", "1234"));
        Data input = new Data("Ok", other);

        String result = jsonObjectMapper.writeValueAsString(input);

        String expected = "{\"Status\":\"Ok\",\"list\":[1,2,3],\"Id\":\"1234\",\"object\":{\"foo\":\"1234\"}}";

        Assertions.assertEquals(expected, result);
    }

    @Test
    public void deserialize() throws Exception
    {
        String json = "{\"Status\":\"Ok\",\"object\":{\"foo\":\"1234\"},\"Id\":\"1234\",\"list\":[1,2,3]}";

        Data result = jsonObjectMapper.readValue(json, Data.class);

        var other = new LinkedHashMap<String, Object>();
        other.put("list", List.of(1, 2, 3));
        other.put("Id", "1234");
        other.put("object", Map.of("foo", "1234"));
        Data expected = new Data("Ok", other);

        Assertions.assertEquals(expected, result);
    }

}

The test passes with "ugly Data", but obviously not with "pretty Data".

I'd like to collect the extra fields into a separate Map, then do some work with the data object, then serialize it back to json string. I don't need them for anything, but I want to preserve them if I later decide I do need them.


When I define the record like this, the serialization works as expected and deserialization doesn't throw errors, but it puts null into the otherFields field.

pretty Data
public record Data(
    @JsonProperty("Status") String status,
    @JsonUnwrapped @JsonAnyGetter @JsonAnySetter Map<String, Object> otherFields
)
{

}

I've done some more googling and this is basically the same #3439 (comment), sorry for a duplicate post

@cowtowncoder
Copy link
Member

@fprochazka Please don't add things that are not relevant for the specific issues. For usage questions, mailing list:

https://groups.google.com/g/jackson-user

works wonders. For issues to report, file a new one UNLESS you have something specific to add into existing one.

@fprochazka
Copy link

@cowtowncoder sorry, I thought it is relevant to this issue, since I didn't find the sample anywhere previously

@cowtowncoder
Copy link
Member

Hmmh. Well, looking at it, it's a more complicated case where Record is also problematic, as well as any setter -- I guess my point was that it does not help solve the basic problem wrt Map valued properties.
But I understand you wanted to help with additional information so I guess that is ok.

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 23, 2022

Thinking about this more I think that as things are, it really SHOULD be possible to just use @JsonAnyGetter and @JsonAnySetter combo to achieve the same result as non-prefixed @JsonUnwrapped, wrt Maps.
@JsonAnySetter also works for JsonNode (although `@JsonAnyGetter not yet, see #3604).
Both annotations can be used on fields and methods and you only need to annotate one of the accessors (setter OR getter, not both).

Because of this, I am leaning towards closing this as "will-not-fix". But will not do that quite as yet.

Note, too, that it is perfectly possible to add features (like use of "prefix") to @JsonAnyGetter/@JsonAnySetter.

@agorbachenko
Copy link

agorbachenko commented May 6, 2023

Hi, I'd like to add here my use-case:

ugly Data
pretty Data
OtherFieldsJacksonTest
The test passes with "ugly Data", but obviously not with "pretty Data".

I'd like to collect the extra fields into a separate Map, then do some work with the data object, then serialize it back to json string. I don't need them for anything, but I want to preserve them if I later decide I do need them.

When I define the record like this, the serialization works as expected and deserialization doesn't throw errors, but it puts null into the otherFields field.

pretty Data
I've done some more googling and this is basically the same #3439 (comment), sorry for a duplicate post

Hi! @fprochazka, you can try to solve this for your "pretty Data" with a static method annotated with @JsonCreator:

public record Data(
    @JsonProperty("Status") String status,
    @JsonAnyGetter Map<String, Object> otherFields
) {
        @JsonCreator
        public static Data create(Map<String, Object> attributes) {
            Object status = attributes.remove("Status");
            assert status instanceof String;
            return new Data((String) status, attributes);
        }
}

@fprochazka
Copy link

@agorbachenko thanks for the alternative, it's much nicer than my first example, but I don't like it either; I've already solved it, see: #3439 (comment), until jackson has native support for this, I'm content using my hack.

@cowtowncoder
Copy link
Member

Although there may be some aspects where @JsonAnyGetter won't work -- f.ex, inability to use more than 1 per POJO -- I think it has big enough overlap that I will close this issue as "wont-fix" (allowing multiple @JsonAnyGetters is a possibility if someone has time to work on doing that).

@cowtowncoder cowtowncoder closed this as not planned Won't fix, can't repro, duplicate, stale Nov 26, 2023
@cowtowncoder cowtowncoder added the will-not-fix Closed as either non-issue or something not planned to be worked on label Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action will-not-fix Closed as either non-issue or something not planned to be worked on
Projects
None yet
Development

No branches or pull requests