From e9a11ff72c674c4a26003d097ac4508449cc8f00 Mon Sep 17 00:00:00 2001 From: Kevin Binswanger Date: Mon, 26 Oct 2020 15:34:53 -0500 Subject: [PATCH 1/3] 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 --- .../databind/AnnotationIntrospector.java | 17 ++++ .../jackson/databind/BeanDescription.java | 11 +++ .../introspect/BasicBeanDescription.java | 6 ++ .../JacksonAnnotationIntrospector.java | 9 +++ .../introspect/POJOPropertiesCollector.java | 37 +++++++++ .../databind/ser/BasicSerializerFactory.java | 30 ++++--- .../databind/jsontype/MapSerializingTest.java | 79 +++++++++++++++++++ 7 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java diff --git a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java index 0ea35648e5..5242643c5f 100644 --- a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java @@ -967,6 +967,23 @@ public PropertyName findNameForSerialization(Annotated a) { return null; } + /** + * Method for checking whether given method has an annotation + * that suggests the return value of annotated method + * should be used as "the key" of the object instance; usually + * serialized as a primitive value such as String or number. + * + * @return {@link Boolean#TRUE} if such annotation is found and is not disabled; + * {@link Boolean#FALSE} if disabled annotation (block) is found (to indicate + * accessor is definitely NOT to be used "as value"); or `null` if no + * information found. + * + * @since TODO + */ + public Boolean hasAsKey(Annotated a) { + return null; + } + /** * Method for checking whether given method has an annotation * that suggests that the return value of annotated method diff --git a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java index 24ddfe9e3a..0d10687ae2 100644 --- a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java @@ -173,6 +173,17 @@ public boolean isNonStaticInnerClass() { /********************************************************** */ + /** + * Method for locating accessor (readable field, or "getter" method) + * that has + * {@link com.fasterxml.jackson.annotation.JsonKey} annotation, + * if any. If multiple ones are found, + * an error is reported by throwing {@link IllegalArgumentException} + * + * @since TODO + */ + public abstract AnnotatedMember findJsonKeyAccessor(); + /** * Method for locating accessor (readable field, or "getter" method) * that has diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java index aaaa584e2c..e01b7ccd18 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java @@ -239,6 +239,12 @@ public List findProperties() { return _properties(); } + @Override + public AnnotatedMember findJsonKeyAccessor() { + return (_propCollector == null) ? null + : _propCollector.getJsonKeyAccessor(); + } + @Override @Deprecated // since 2.9 public AnnotatedMethod findJsonValueMethod() { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java index 178e10521a..b426ceeb34 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java @@ -1071,6 +1071,15 @@ public PropertyName findNameForSerialization(Annotated a) return null; } + @Override + public Boolean hasAsKey(Annotated a) { + JsonKey ann = _findAnnotation(a, JsonKey.class); + if (ann == null) { + return null; + } + return ann.value(); + } + @Override // since 2.9 public Boolean hasAsValue(Annotated a) { JsonValue ann = _findAnnotation(a, JsonValue.class); diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index cf0b842883..f66b069dea 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -106,6 +106,11 @@ public class POJOPropertiesCollector protected LinkedList _anySetterField; + /** + * Method(s) annotated with 'JsonKey' annotation + */ + protected LinkedList _jsonKeyAccessors; + /** * Method(s) marked with 'JsonValue' annotation *

@@ -187,6 +192,23 @@ public Map getInjectables() { return _injectables; } + public AnnotatedMember getJsonKeyAccessor() { + if (!_collected) { + collectAll(); + } + // If @JsonKey defined, must have a single one + if (_jsonKeyAccessors != null) { + if (_jsonKeyAccessors.size() > 1) { + reportProblem("Multiple 'as-value' properties defined (%s vs %s)", + _jsonKeyAccessors.get(0), + _jsonKeyAccessors.get(1)); + } + // otherwise we won't greatly care + return _jsonKeyAccessors.get(0); + } + return null; + } + /** * @since 2.9 */ @@ -384,6 +406,13 @@ protected void _addFields(Map props) final boolean transientAsIgnoral = _config.isEnabled(MapperFeature.PROPAGATE_TRANSIENT_MARKER); for (AnnotatedField f : _classDef.fields()) { + // @JsonKey? + if (Boolean.TRUE.equals(ai.hasAsKey(f))) { + if (_jsonKeyAccessors == null) { + _jsonKeyAccessors = new LinkedList<>(); + } + _jsonKeyAccessors.add(f); + } // @JsonValue? if (Boolean.TRUE.equals(ai.hasAsValue(f))) { if (_jsonValueAccessors == null) { @@ -596,6 +625,14 @@ protected void _addGetterMethod(Map props, _anyGetters.add(m); return; } + // @JsonKey? + if (Boolean.TRUE.equals(ai.hasAsKey(m))) { + if (_jsonKeyAccessors == null) { + _jsonKeyAccessors = new LinkedList<>(); + } + _jsonKeyAccessors.add(m); + return; + } // @JsonValue? if (Boolean.TRUE.equals(ai.hasAsValue(m))) { if (_jsonValueAccessors == null) { diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java index e8f36bed71..56faee56d6 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java @@ -228,18 +228,30 @@ public JsonSerializer createKeySerializer(SerializerProvider ctxt, ser = StdKeySerializers.getStdKeySerializer(config, keyType.getRawClass(), false); // As per [databind#47], also need to support @JsonValue if (ser == null) { - AnnotatedMember am = beanDesc.findJsonValueAccessor(); - if (am != null) { - final Class rawType = am.getRawType(); - JsonSerializer delegate = StdKeySerializers.getStdKeySerializer(config, - rawType, true); + AnnotatedMember keyAm = beanDesc.findJsonKeyAccessor(); + if (keyAm != null) { + final Class rawType = keyAm.getRawType(); + JsonSerializer delegate = createKeySerializer(ctxt, config.constructType(rawType), null); if (config.canOverrideAccessModifiers()) { - ClassUtil.checkAndFixAccess(am.getMember(), + ClassUtil.checkAndFixAccess(keyAm.getMember(), config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS)); } - ser = new JsonValueSerializer(am, delegate); - } else { - ser = StdKeySerializers.getFallbackKeySerializer(config, keyType.getRawClass()); + ser = new JsonValueSerializer(keyAm, delegate); + } + if (ser == null) { + AnnotatedMember am = beanDesc.findJsonValueAccessor(); + if (am != null) { + final Class rawType = am.getRawType(); + JsonSerializer delegate = StdKeySerializers.getStdKeySerializer(config, + rawType, true); + if (config.canOverrideAccessModifiers()) { + ClassUtil.checkAndFixAccess(am.getMember(), + config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS)); + } + ser = new JsonValueSerializer(am, delegate); + } else { + ser = StdKeySerializers.getFallbackKeySerializer(config, keyType.getRawClass()); + } } } } diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java new file mode 100644 index 0000000000..49bf58587e --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java @@ -0,0 +1,79 @@ +package com.fasterxml.jackson.databind.jsontype; + +import java.util.Collections; +import java.util.Map; + +import com.fasterxml.jackson.annotation.JsonKey; +import com.fasterxml.jackson.annotation.JsonValue; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Ignore; +import org.junit.Test; + +public class MapSerializingTest { + class Inner { + @JsonKey + String key; + + @JsonValue + String value; + + Inner(String key, String value) { + this.key = key; + this.value = value; + } + + public String toString() { + return "Inner(" + this.key + "," + this.value + ")"; + } + + } + + class Outer { + @JsonKey + @JsonValue + Inner inner; + + Outer(Inner inner) { + this.inner = inner; + } + + } + + class NoKeyOuter { + @JsonValue + Inner inner; + + NoKeyOuter(Inner inner) { + this.inner = inner; + } + } + + @Ignore + @Test + public void testClassAsKey() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + Outer outer = new Outer(new Inner("innerKey", "innerValue")); + Map map = Collections.singletonMap(outer, "value"); + String actual = mapper.writeValueAsString(map); + Assert.assertEquals("{\"innerKey\":\"value\"}", actual); + } + + @Ignore + @Test + public void testClassAsValue() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + Map mapA = Collections.singletonMap("key", new Outer(new Inner("innerKey", "innerValue"))); + String actual = mapper.writeValueAsString(mapA); + Assert.assertEquals("{\"key\":\"innerValue\"}", actual); + } + + @Ignore + @Test + public void testNoKeyOuter() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + Map mapA = Collections.singletonMap("key", new NoKeyOuter(new Inner("innerKey", "innerValue"))); + String actual = mapper.writeValueAsString(mapA); + Assert.assertEquals("{\"key\":\"innerValue\"}", actual); + } +} From c3490bf07954b03ad7c17e2eb87afea9b1a8429e Mon Sep 17 00:00:00 2001 From: Kevin Binswanger Date: Mon, 26 Oct 2020 15:51:38 -0500 Subject: [PATCH 2/3] Removes `@Ignore` from the MapSerializingTest These were added accidentally and need to be removed. --- .../jackson/databind/jsontype/MapSerializingTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java index 49bf58587e..48e020f1c7 100644 --- a/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/MapSerializingTest.java @@ -48,8 +48,7 @@ class NoKeyOuter { this.inner = inner; } } - - @Ignore + @Test public void testClassAsKey() throws Exception { ObjectMapper mapper = new ObjectMapper(); @@ -59,7 +58,6 @@ public void testClassAsKey() throws Exception { Assert.assertEquals("{\"innerKey\":\"value\"}", actual); } - @Ignore @Test public void testClassAsValue() throws Exception { ObjectMapper mapper = new ObjectMapper(); @@ -68,7 +66,6 @@ public void testClassAsValue() throws Exception { Assert.assertEquals("{\"key\":\"innerValue\"}", actual); } - @Ignore @Test public void testNoKeyOuter() throws Exception { ObjectMapper mapper = new ObjectMapper(); From 9a528beea6a9e4820f18ab43225b72eb9af889e4 Mon Sep 17 00:00:00 2001 From: Kevin Binswanger Date: Wed, 28 Oct 2020 15:36:12 -0500 Subject: [PATCH 3/3] Fixes some code review comments Miscellaneous cleanups, mostly tweaks on things that were copied from how `@JsonValue` was handled. --- .../jackson/databind/AnnotationIntrospector.java | 2 +- .../fasterxml/jackson/databind/BeanDescription.java | 4 +++- .../introspect/JacksonAnnotationIntrospector.java | 2 +- .../databind/introspect/POJOPropertiesCollector.java | 12 +++++++----- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java index 5242643c5f..5152e883e6 100644 --- a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java @@ -980,7 +980,7 @@ public PropertyName findNameForSerialization(Annotated a) { * * @since TODO */ - public Boolean hasAsKey(Annotated a) { + public Boolean hasAsKey(MapperConfig config, Annotated a) { return null; } diff --git a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java index 0d10687ae2..6e4dd5c423 100644 --- a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java @@ -182,7 +182,9 @@ public boolean isNonStaticInnerClass() { * * @since TODO */ - public abstract AnnotatedMember findJsonKeyAccessor(); + public AnnotatedMember findJsonKeyAccessor() { + return null; + } /** * Method for locating accessor (readable field, or "getter" method) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java index b426ceeb34..4f96611f6d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java @@ -1072,7 +1072,7 @@ public PropertyName findNameForSerialization(Annotated a) } @Override - public Boolean hasAsKey(Annotated a) { + public Boolean hasAsKey(MapperConfig config, Annotated a) { JsonKey ann = _findAnnotation(a, JsonKey.class); if (ann == null) { return null; diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index f66b069dea..5dcac477c5 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -6,6 +6,8 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonKey; +import com.fasterxml.jackson.annotation.JsonValue; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.cfg.HandlerInstantiator; @@ -107,12 +109,12 @@ public class POJOPropertiesCollector protected LinkedList _anySetterField; /** - * Method(s) annotated with 'JsonKey' annotation + * Accessors (field or "getter" method annotated with {@link JsonKey} */ protected LinkedList _jsonKeyAccessors; /** - * Method(s) marked with 'JsonValue' annotation + *Accessors (field or "getter" method) annotated with {@link JsonValue} *

* NOTE: before 2.9, was `AnnotatedMethod`; with 2.9 allows fields too */ @@ -199,7 +201,7 @@ public AnnotatedMember getJsonKeyAccessor() { // If @JsonKey defined, must have a single one if (_jsonKeyAccessors != null) { if (_jsonKeyAccessors.size() > 1) { - reportProblem("Multiple 'as-value' properties defined (%s vs %s)", + reportProblem("Multiple 'as-key' properties defined (%s vs %s)", _jsonKeyAccessors.get(0), _jsonKeyAccessors.get(1)); } @@ -407,7 +409,7 @@ protected void _addFields(Map props) for (AnnotatedField f : _classDef.fields()) { // @JsonKey? - if (Boolean.TRUE.equals(ai.hasAsKey(f))) { + if (Boolean.TRUE.equals(ai.hasAsKey(_config, f))) { if (_jsonKeyAccessors == null) { _jsonKeyAccessors = new LinkedList<>(); } @@ -626,7 +628,7 @@ protected void _addGetterMethod(Map props, return; } // @JsonKey? - if (Boolean.TRUE.equals(ai.hasAsKey(m))) { + if (Boolean.TRUE.equals(ai.hasAsKey(_config, m))) { if (_jsonKeyAccessors == null) { _jsonKeyAccessors = new LinkedList<>(); }