Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Serializing a Map<K, V> field uses declared type for key serialization rather than key instance type #4736

Open
1 task done
emilyy-dev opened this issue Oct 7, 2024 · 4 comments
Labels

Comments

@emilyy-dev
Copy link

emilyy-dev commented Oct 7, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

When serializing a field of type Map<K, V>, Jackson will inspect the declared K type for serialization annotations, rather than the individual entries key's instance type. This creates issues where K is a supertype and the map holds subtypes of K (K') that do specify serialization annotations.

Values are serialized as expected and the instance type is used for annotation introspection.

Version Information

2.18.0

Reproduction

public final class Main {
  public static void main(final String[] args) throws IOException {
    final var list = new IntOrStringList(List.of(new IntOrString.StringValue("foo"), new IntOrString.IntValue(123)));
    final var map = new IntOrStringMap(Map.of(new IntOrString.StringValue("bar"), new IntOrString.IntValue(456)));

    final var mapper = new JsonMapper();
    System.out.println(mapper.writeValueAsString(list));
    System.out.println(mapper.writeValueAsString(map));
    System.out.println(mapper.writeValueAsString(Map.of(new IntOrString.StringValue("baz"), new IntOrString.IntValue(789))));
  }

  public record IntOrStringList(List<IntOrString> values) {
  }

  public record IntOrStringMap(Map<IntOrString, IntOrString> values) {
  }

  public sealed interface IntOrString {

    @JsonKey(false) String forMapKeySerialization();

    record IntValue(@JsonValue int value) implements IntOrString {

      @Override
      public String forMapKeySerialization() {
        return String.valueOf(this.value);
      }
    }

    record StringValue(@JsonValue String value) implements IntOrString {

      @Override
      public String forMapKeySerialization() {
        return this.value;
      }
    }
  }
}

Expected behavior

In the reproducer above, the first println prints {"values":["foo",123]}, the second println prints {"values":{"StringValue[value=bar]":456}}, using the default toString() for the key. The third println is included to display that the issue is when serializing a map field, not a map value directly as that third prints {"baz":789} which is the kind of serialization behavior I'd expect for the second case as well.

Enabling the @JsonKey in the interface's method makes it behave how I'd expect, and the second println now prints {"values":{"bar":456}}. The annotation's presence when disabled doesn't play a role, as it doesn't behave any differently when it isn't there (as expected).

Additional context

The most relevant issue I could find is #3119 given the inheritance setting, but explicitly specifying @JsonSerialize(keyUsing = ...) behaves correctly so I ruled that out, but they might be related in a way I can't see.

@emilyy-dev emilyy-dev added the to-evaluate Issue that has been received but not yet evaluated label Oct 7, 2024
@pjfanning
Copy link
Member

pjfanning commented Oct 9, 2024

Even if this is changed, it will require a configuration that only enables the change if it explicitly set by the user. We might be able to change the behaviour in Jackson 3 but in Jackson 2, we need to assume that there are users who depend on the existing behaviour.

The suggested new behaviour is probably also going to run slower as you need to go around checking more classes.

Jackson supports the registration of custom key serializers so you can workaround this by using this approach. https://www.baeldung.com/jackson-map includes some documenation about this.

@JooHyukKim
Copy link
Member

I agree suggested behavior would add runtime, overhead type-checking for every key instance. I expect a lot of people will come and complain about performance.

Even if this is changed, it will require a configuration that only enables the change if it explicitly set by the user.

Only if we agree that suggested behavior should be supported? +1 on configuration.

@emilyy-dev
Copy link
Author

I figured it'd be a bug given that values are serialized based on the instance's type, it was quite unexpected to see keys weren't, but if this is intentional I'm more than happy to close this report in favour of a feature request instead

@cowtowncoder
Copy link
Member

This works as intended: annotation introspection for Map type is indeed done once, and will not be done on per-entry basis for keys (things are slightly more dynamic for values).
Performance is one consideration, as well as significantly added complexity for key serializer lookup.

I have no interest in changing this myself, but I am not against feature requests being filed (or this being changed).

@cowtowncoder cowtowncoder added 2.18 and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants