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

Add @JsonKey annotation (similar to @JsonValue) for customizable serialization of Map keys #2871

Closed
CidTori opened this issue Oct 5, 2020 · 8 comments · Fixed by #2905
Closed
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest-accepted Tag for issues, PRs that are explicitly marked as accepted for Hacktoberfest
Milestone

Comments

@CidTori
Copy link

CidTori commented Oct 5, 2020

Describe the bug
When serializing a map key, if the key's type uses @JsonValue on one of its attributes, and if that attribute's type uses @JsonValue on one of its own attributes, the second @JsonValue is ignored, and toString() is used instead.

Version information
2.10.0

To Reproduce

    class Inner {

        @JsonValue
        String string;

        Inner(String string) {
            this.string = string;
        }

        public String toString() {
            return "Inner(String="+this.string+")";
        }

    }

    class Outer {

        @JsonValue
        Inner inner;

        Outer(Inner inner) {
            this.inner = inner;
        }

    }

    public void test() throws Exception {
        Outer outer = new Outer(new Inner("key"));
        ObjectMapper mapper = new ObjectMapper();
        System.out.println(mapper.writeValueAsString(outer)); // outputs "key", as expected
        System.out.println(mapper.writeValueAsString(Collections.singletonMap(outer,"value"))); // outputs {"Inner(String=key)":"value"}, expected {"key":"value"}
    }
@CidTori CidTori added the to-evaluate Issue that has been received but not yet evaluated label Oct 5, 2020
@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 5, 2020

Correct: @JsonValue currently only affects serialization of values, not (Map) keys.
The reason for this is mostly duality of key and value handling: at low level actual (de)serializers need to work bit differently both since keys can only be Strings (at least in JSON; many other formats allow different types), and since streaming API (JsonParser, JsonGenerator) handle key token different from String value token.

Currently one has to register separate key serializer either using Module (like SimpleModule), or using @JsonSerialize(keyUsing=) on property or key class itself.

Having said that, I can see why it'd be really nice if @JsonValue did also work for Map keys.
I have slight concern as to whether this could cause problems for some users (if they wanted to register different key serializer from annotations -- annotations typically have precedence over (de)serializer registrations), so it might be safest to add something like

@JsonKey OR @JsonUseAsKey

annotation. That would often mean having to add both @JsonValue and the new annotation.
Alternatively I guess it would be possible to add a property for @JsonValue that would allow optional exclusion for use for key (for unlikely case that only true "value" serialization would use annotation field/method).

@cowtowncoder cowtowncoder added 2.13 and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 5, 2020
@CidTori
Copy link
Author

CidTori commented Oct 6, 2020

It could be best to use another annotation indeed, like @JsonKey, because you could also want to serialize an object differently if it's a key or a value (since, as you said, in JSON, keys must be strings, unlike values).

I just want to add that, currently, @JsonValue does affect serialization of keys, since, in my example, the @JsonValue on Outer.inner is not ignored ; it's the @JsonValue on Inner.string that is ignored.

@cowtowncoder cowtowncoder changed the title Serialization of map keys does not follow @JsonValue chains Serialization of map keys does not use @JsonValue: add @JsonKey annotation? Oct 6, 2020
@cowtowncoder
Copy link
Member

Good point wrt different annotation for key/value aspect, somehow missed that (I think I had that in mind in the past).

I'll have to see how @JsonValue is used, then; I was thinking that it might be used for Enums but not other types. But perhaps I have forgotten about a feature I added at some point...
Adding a separate annotation would make sense, regardless, I think -- but obviously need to consider existing handling as users may be relying on that.
And I agree that if chaining works for values, it should work for keys as well.

@cowtowncoder cowtowncoder changed the title Serialization of map keys does not use @JsonValue: add @JsonKey annotation? Serialization of map keys does not use @JsonValue similar to values (no chaining?) -- maybe add @JsonKey annotation Oct 6, 2020
@CidTori
Copy link
Author

CidTori commented Oct 7, 2020

Yep, for example : while serializing a map key, if there is a @JsonKey, use it (with chaining), else if there is a @JsonValue, use it (with chaining too), else use toString()?

@cowtowncoder cowtowncoder added good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest Issue related to Hactoberfest activities, eligible for additional rewards labels Oct 9, 2020
@cowtowncoder cowtowncoder removed the hacktoberfest Issue related to Hactoberfest activities, eligible for additional rewards label Oct 16, 2020
@Anusien
Copy link
Contributor

Anusien commented Oct 26, 2020

I've started working on this.

@Anusien
Copy link
Contributor

Anusien commented Oct 26, 2020

    class Inner {

        @JsonKey
        @JsonValue
        String string;

        Inner(String string) {
            this.string = string;
        }

        public String toString() {
            return "Inner(String="+this.string+")";
        }
    }

    class Outer {

        @JsonValue
        Inner inner;

        Outer(Inner inner) {
            this.inner = inner;
        }
    }

If we only put @JsonValue on the Outer class (and not @JsonKey), would you expect it to trickle down and read the @JsonKey value on the inner class? Or would the requirement be to have @JsonKey on Outer and Inner?

@cowtowncoder
Copy link
Member

@Anusien Interesting... this gets quite tricky, wrt overlap of annotations, precedence.

I think that it will be simpler and more reliable to keep @JsonValue and @JsonKey separate, and that would, I think, require duplication of annotation at both levels.
Conversely if handling of the two was coupled it could cover more cases with fewer annotations but would probably have weird edge cases and more complicated handling internally (to try to combine intended logic).

Does this make sense?

@cowtowncoder
Copy link
Member

Just realized there's the original issue, #47, that gives more context.

Anusien added a commit to Anusien/jackson-databind that referenced this issue Oct 26, 2020
When serializing the key of a Map, look for a `@JsonKey` annotation.
When present (taking priority over `@JsonValue`), skip the
StdKey:Serializer and attempt to find a serializer for the inner type.

Fixes FasterXML#2871
Anusien added a commit to Anusien/jackson-databind that referenced this issue Oct 26, 2020
When serializing the key of a Map, look for a `@JsonKey` annotation.
When present (taking priority over `@JsonValue`), skip the
StdKey:Serializer and attempt to find a serializer for the inner type.

Fixes FasterXML#2871
@cowtowncoder cowtowncoder added 2.12 hacktoberfest-accepted Tag for issues, PRs that are explicitly marked as accepted for Hacktoberfest and removed 2.13 labels Oct 28, 2020
cowtowncoder pushed a commit that referenced this issue Nov 2, 2020
Adds support for `@JsonKey` annotation

When serializing the key of a Map, look for a `@JsonKey` annotation.
When present (taking priority over `@JsonValue`), skip the
StdKey:Serializer and attempt to find a serializer for the inner type.

Fixes #2871
@cowtowncoder cowtowncoder changed the title Serialization of map keys does not use @JsonValue similar to values (no chaining?) -- maybe add @JsonKey annotation Add @JsonKey annotation (similar to @JsonValue) for customizable serialization of Map keys Nov 2, 2020
@cowtowncoder cowtowncoder added this to the 2.12.0-rc2 milestone Nov 2, 2020
cowtowncoder added a commit that referenced this issue Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest-accepted Tag for issues, PRs that are explicitly marked as accepted for Hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants