From 31a4e688887ff8e2899a58f2a705fa10dd7a84f9 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 22 Apr 2024 14:36:49 +0200 Subject: [PATCH] Fix enum as map keys (#826) --- gradle/libs.versions.toml | 2 +- .../serde/jackson/JsonIgnoreSpec.groovy | 26 ++++ .../jackson/tst/AfterCareStatsEntry.java | 22 ++++ .../serde/jackson/tst/Aggregation.java | 22 ++++ .../jackson/tst/ClassificationAndStats.java | 20 +++ .../serde/jackson/tst/ClassificationVars.java | 9 ++ .../serde/jackson/tst/MainAggregationVm.java | 11 ++ .../serde/jackson/tst/StatsEntry.java | 7 ++ .../annotation/SerdeJsonIgnoreSpec.groovy | 1 + .../serializers/CustomizedMapSerializer.java | 115 +++++++++++++----- .../CustomizedMapSerializerSpec.groovy | 85 ------------- 11 files changed, 203 insertions(+), 117 deletions(-) create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/AfterCareStatsEntry.java create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/Aggregation.java create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationAndStats.java create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationVars.java create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/MainAggregationVm.java create mode 100644 serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/StatsEntry.java delete mode 100644 serde-support/src/test/groovy/io/micronaut/serde/support/serializers/CustomizedMapSerializerSpec.groovy diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b064c4d4c..61a696c7a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -16,7 +16,7 @@ jmh = "1.37" groovy = "4.0.18" micronaut = "4.4.0" -micronaut-platform = "4.3.8" +micronaut-platform = "4.4.0" micronaut-docs = "2.0.0" micronaut-test = "4.2.1" micronaut-discovery = "4.2.0" diff --git a/serde-jackson-tck/src/main/groovy/io/micronaut/serde/jackson/JsonIgnoreSpec.groovy b/serde-jackson-tck/src/main/groovy/io/micronaut/serde/jackson/JsonIgnoreSpec.groovy index d5ec1d873..36f2885ce 100644 --- a/serde-jackson-tck/src/main/groovy/io/micronaut/serde/jackson/JsonIgnoreSpec.groovy +++ b/serde-jackson-tck/src/main/groovy/io/micronaut/serde/jackson/JsonIgnoreSpec.groovy @@ -1,11 +1,37 @@ package io.micronaut.serde.jackson +import io.micronaut.context.ApplicationContext import io.micronaut.core.type.Argument +import io.micronaut.json.JsonMapper +import io.micronaut.serde.jackson.tst.AfterCareStatsEntry +import io.micronaut.serde.jackson.tst.ClassificationAndStats +import io.micronaut.serde.jackson.tst.ClassificationVars +import io.micronaut.serde.jackson.tst.MainAggregationVm abstract class JsonIgnoreSpec extends JsonCompileSpec { abstract protected String unknownPropertyMessage(String propertyName, String className) + def 'JsonIgnore and enum as map keys'() { + given: + def ctx = ApplicationContext.run() + def jsonMapper = ctx.getBean(JsonMapper) + def obj = new MainAggregationVm( + List.of( + new ClassificationAndStats( + new ClassificationVars("01"), + new AfterCareStatsEntry() + ) + ) + ) + def json = '{"afterCare":[{"klassifisering":{"regionKode":"01"},"stats":{"SomeField1":0,"SomeField2":0}}]}' + expect: + serializeToString(jsonMapper, obj) == json + + cleanup: + ctx.close() + } + void 'JsonIgnoreType'() { given: def compiled = buildContext(''' diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/AfterCareStatsEntry.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/AfterCareStatsEntry.java new file mode 100644 index 000000000..8abf17193 --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/AfterCareStatsEntry.java @@ -0,0 +1,22 @@ +package io.micronaut.serde.jackson.tst; + +import io.micronaut.core.annotation.Introspected; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +@Introspected +public record AfterCareStatsEntry() implements StatsEntry { + + static List AFTERCARE_AGGREGATIONS = List.of( + Aggregation.FIELD_1, + Aggregation.FIELD_2 + ); + + @Override + public Map getShouldNotAppearInJson() { + return AFTERCARE_AGGREGATIONS.stream().collect(Collectors.toMap(it -> it, it -> 0, (x, y) -> y, LinkedHashMap::new)); + } +} diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/Aggregation.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/Aggregation.java new file mode 100644 index 000000000..6b000d563 --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/Aggregation.java @@ -0,0 +1,22 @@ +package io.micronaut.serde.jackson.tst; + +import com.fasterxml.jackson.annotation.JsonValue; +import io.micronaut.core.annotation.Introspected; + +@Introspected +public enum Aggregation { + + FIELD_1("SomeField1"), + FIELD_2("SomeField2"); + + private String fieldName; + + Aggregation(String fieldName) { + this.fieldName = fieldName; + } + + @JsonValue + public String getFieldName() { + return fieldName; + } +} diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationAndStats.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationAndStats.java new file mode 100644 index 000000000..bcac074ce --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationAndStats.java @@ -0,0 +1,20 @@ +package io.micronaut.serde.jackson.tst; + +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonIgnore; +import io.micronaut.core.annotation.Introspected; + +import java.util.Map; + +@Introspected +public record ClassificationAndStats( + ClassificationVars klassifisering, + /** Ignore field to avoid double wrapping of values in resulting JSON */ + @JsonIgnore + T stats +) { + @JsonGetter("stats") + Map getValues() { + return stats.getShouldNotAppearInJson(); + } +} diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationVars.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationVars.java new file mode 100644 index 000000000..115939120 --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/ClassificationVars.java @@ -0,0 +1,9 @@ +package io.micronaut.serde.jackson.tst; + +import io.micronaut.core.annotation.Introspected; + +@Introspected +public record ClassificationVars( + String regionKode +) { +} diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/MainAggregationVm.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/MainAggregationVm.java new file mode 100644 index 000000000..0aed59f9e --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/MainAggregationVm.java @@ -0,0 +1,11 @@ +package io.micronaut.serde.jackson.tst; + +import io.micronaut.core.annotation.Introspected; + +import java.util.List; + +@Introspected +public record MainAggregationVm( + List> afterCare +) { +} diff --git a/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/StatsEntry.java b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/StatsEntry.java new file mode 100644 index 000000000..a0040aec3 --- /dev/null +++ b/serde-jackson-tck/src/main/java/io/micronaut/serde/jackson/tst/StatsEntry.java @@ -0,0 +1,7 @@ +package io.micronaut.serde.jackson.tst; + +import java.util.Map; + +public interface StatsEntry { + Map getShouldNotAppearInJson(); +} diff --git a/serde-jackson/src/test/groovy/io/micronaut/serde/jackson/annotation/SerdeJsonIgnoreSpec.groovy b/serde-jackson/src/test/groovy/io/micronaut/serde/jackson/annotation/SerdeJsonIgnoreSpec.groovy index 5b252e3ac..acc74472e 100644 --- a/serde-jackson/src/test/groovy/io/micronaut/serde/jackson/annotation/SerdeJsonIgnoreSpec.groovy +++ b/serde-jackson/src/test/groovy/io/micronaut/serde/jackson/annotation/SerdeJsonIgnoreSpec.groovy @@ -1,5 +1,6 @@ package io.micronaut.serde.jackson.annotation + import io.micronaut.core.naming.NameUtils import io.micronaut.serde.jackson.JsonIgnoreSpec diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/CustomizedMapSerializer.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/CustomizedMapSerializer.java index 66d18eb8d..593c0a881 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/CustomizedMapSerializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/CustomizedMapSerializer.java @@ -16,15 +16,18 @@ package io.micronaut.serde.support.serializers; import io.micronaut.core.annotation.Internal; +import io.micronaut.core.beans.exceptions.IntrospectionException; import io.micronaut.core.convert.exceptions.ConversionErrorException; import io.micronaut.core.type.Argument; import io.micronaut.core.util.ArrayUtils; import io.micronaut.core.util.CollectionUtils; +import io.micronaut.json.tree.JsonNode; import io.micronaut.serde.Encoder; import io.micronaut.serde.ObjectSerializer; import io.micronaut.serde.Serializer; import io.micronaut.serde.exceptions.SerdeException; import io.micronaut.serde.support.SerializerRegistrar; +import io.micronaut.serde.support.util.JsonNodeEncoder; import io.micronaut.serde.util.CustomizableSerializer; import java.io.IOException; @@ -43,8 +46,11 @@ final class CustomizedMapSerializer implements CustomizableSerializer> createSpecific(EncoderContext context, Argument> type) throws SerdeException { final Argument[] generics = type.getTypeParameters(); - final boolean hasGenerics = ArrayUtils.isNotEmpty(generics) && generics.length != 2; + final boolean hasGenerics = ArrayUtils.isNotEmpty(generics) && generics.length == 2; if (hasGenerics) { + final Argument keyGeneric = (Argument) generics[0]; + final Serializer keySerializer = findKeySerializer(context, keyGeneric); + final boolean isStringKey = keyGeneric.getType().equals(String.class) || CharSequence.class.isAssignableFrom(keyGeneric.getType()); final Argument valueGeneric = (Argument) generics[1]; final Serializer valSerializer = (Serializer) context.findSerializer(valueGeneric).createSpecific(context, valueGeneric); return new ObjectSerializer<>() { @@ -58,17 +64,20 @@ public void serialize(Encoder encoder, EncoderContext context, Argument> type, Map value) throws IOException { - for (K k : value.keySet()) { - encodeMapKey(context, encoder, k); - final V v = value.get(k); + for (Map.Entry entry : value.entrySet()) { + K k = entry.getKey(); + if (k == null) { + encoder.encodeNull(); + } else if (isStringKey) { + encoder.encodeKey(k.toString()); + } else { + encodeMapKey(context, encoder, keyGeneric, keySerializer, k); + } + V v = entry.getValue(); if (v == null) { encoder.encodeNull(); } else { - valSerializer.serialize( - encoder, - context, - valueGeneric, v - ); + valSerializer.serialize(encoder, context, valueGeneric, v); } } } @@ -91,20 +100,30 @@ public void serialize(Encoder encoder, EncoderContext context, Argument> type, Map value) throws IOException { + Argument keyGeneric = null; + Serializer keySerializer = null; + Argument valueGeneric = null; + Serializer valSerializer = null; for (Map.Entry entry : value.entrySet()) { - encodeMapKey(context, encoder, entry.getKey()); + K k = entry.getKey(); + if (k instanceof CharSequence) { + encoder.encodeKey(k.toString()); + } else { + if (keyGeneric == null || !keyGeneric.getType().equals(k.getClass())) { + keyGeneric = (Argument) Argument.of(k.getClass()); + keySerializer = findKeySerializer(context, keyGeneric); + } + encodeMapKey(context, encoder, keyGeneric, keySerializer, k); + } final V v = entry.getValue(); if (v == null) { encoder.encodeNull(); } else { - @SuppressWarnings("unchecked") final Argument valueGeneric = (Argument) Argument.of(v.getClass()); - final Serializer valSerializer = context.findSerializer(valueGeneric) - .createSpecific(context, valueGeneric); - valSerializer.serialize( - encoder, - context, - valueGeneric, v - ); + if (valueGeneric == null || !valueGeneric.getType().equals(v.getClass())) { + valueGeneric = (Argument) Argument.of(v.getClass()); + valSerializer = context.findSerializer(valueGeneric).createSpecific(context, valueGeneric); + } + valSerializer.serialize(encoder, context, valueGeneric, v); } } } @@ -117,21 +136,55 @@ public boolean isEmpty(EncoderContext context, Map value) { } } - private void encodeMapKey(EncoderContext context, Encoder childEncoder, K k) throws IOException { - // relies on the key type implementing toString() correctly - // perhaps we should supply conversion service - if (k instanceof CharSequence) { - childEncoder.encodeKey(k.toString()); + private Serializer findKeySerializer(EncoderContext context, Argument keyGeneric) throws SerdeException { + try { + return (Serializer) context.findSerializer(keyGeneric).createSpecific(context, keyGeneric); + } catch (SerdeException e) { + if (e.getCause() instanceof IntrospectionException) { + // The key is not introspected + return (encoder, ctx, type, value) -> convertMapKeyToStringAndEncode(ctx, encoder, value); + } + throw e; + } + } + + private void encodeMapKey(EncoderContext context, + Encoder encoder, + Argument keyGeneric, + Serializer keySerializer, + K k) throws IOException { + JsonNodeEncoder keyEncoder = JsonNodeEncoder.create(); + try { + keySerializer.serialize(keyEncoder, context, keyGeneric, k); + } catch (SerdeException e) { + if (e.getCause() instanceof IntrospectionException) { + // The key is not introspected + convertMapKeyToStringAndEncode(context, encoder, k); + return; + } + throw e; + } + JsonNode keyNode = keyEncoder.getCompletedValue(); + if (keyNode.isString()) { + encoder.encodeKey(keyNode.getStringValue()); + } else if (keyNode.isNull()) { + throw new SerdeException("Null key for a Map not allowed in JSON"); + } else if (keyNode.isBoolean() || keyNode.isNumber()) { + encoder.encodeString(keyNode.coerceStringValue()); } else { - try { - final String result = context.getConversionService().convertRequired( - k, - Argument.STRING - ); - childEncoder.encodeKey(result != null ? result : k.toString()); - } catch (ConversionErrorException e) { - throw new SerdeException("Error converting Map key [" + k + "] to String: " + e.getMessage(), e); + convertMapKeyToStringAndEncode(context, encoder, keyNode.getValue()); + } + } + + private void convertMapKeyToStringAndEncode(EncoderContext context, Encoder encoder, Object keyValue) throws IOException { + try { + final String result = context.getConversionService().convertRequired(keyValue, Argument.STRING); + if (result == null) { + throw new SerdeException("Null key for a Map not allowed in JSON"); } + encoder.encodeKey(result); + } catch (ConversionErrorException ce) { + throw new SerdeException("Error converting Map key [" + keyValue + "] to String: " + ce.getMessage(), ce); } } diff --git a/serde-support/src/test/groovy/io/micronaut/serde/support/serializers/CustomizedMapSerializerSpec.groovy b/serde-support/src/test/groovy/io/micronaut/serde/support/serializers/CustomizedMapSerializerSpec.groovy deleted file mode 100644 index b8dd6b136..000000000 --- a/serde-support/src/test/groovy/io/micronaut/serde/support/serializers/CustomizedMapSerializerSpec.groovy +++ /dev/null @@ -1,85 +0,0 @@ -package io.micronaut.serde.support.serializers - -import io.micronaut.core.type.Argument -import io.micronaut.serde.Encoder -import io.micronaut.serde.Serializer -import spock.lang.Specification - -class CustomizedMapSerializerSpec extends Specification { - def 'create specific with generics'() { - given: - def encoder = Mock(Encoder) - encoder.encodeArray(_ as Argument) >> encoder - encoder.encodeObject(_ as Argument) >> encoder - - def defaultSerializer = Mock(Serializer) - defaultSerializer.createSpecific(_ as Serializer.EncoderContext, _ as Argument) >> defaultSerializer - - def context = Mock(Serializer.EncoderContext) - context.findSerializer(Argument.of(String)) >> defaultSerializer - context.findSerializer({Argument a -> List.isAssignableFrom(a.type)}) >> new IterableSerializer<>() - context.findSerializer({Argument a -> Map.isAssignableFrom(a.type)}) >> new CustomizedMapSerializer<>() - - def serializer = new CustomizedMapSerializer() - - def type = Argument.of(Map, String, String) - def specific = serializer.createSpecific(context, type) - when: - specific.serialize(encoder, context, type, [ - val1: 'abc', - val2: '123', - val3: 'true' - ]) - then: - encoder.encodeObject(_ as Argument) - encoder.encodeKey("val1") - encoder.encodeString("abc") - encoder.encodeKey("val2") - encoder.encodeString("123") - encoder.encodeKey("val1") - encoder.encodeString("true") - encoder.finishStructure() - } - - def 'create specific without generics'() { - given: - def encoder = Mock(Encoder) - encoder.encodeArray(_ as Argument) >> encoder - encoder.encodeObject(_ as Argument) >> encoder - - def defaultSerializer = Mock(Serializer) - defaultSerializer.createSpecific(_ as Serializer.EncoderContext, _ as Argument) >> defaultSerializer - - def context = Mock(Serializer.EncoderContext) - context.findSerializer(Argument.of(String)) >> defaultSerializer - context.findSerializer({Argument a -> List.isAssignableFrom(a.type)}) >> new IterableSerializer<>() - context.findSerializer({Argument a -> Map.isAssignableFrom(a.type)}) >> new CustomizedMapSerializer<>() - - def serializer = new CustomizedMapSerializer() - - def type = Argument.of(Map) - def specific = serializer.createSpecific(context, type) - when: - specific.serialize(encoder, context, type, [ - keys: [ - [ - val1: 'abc', - val2: '123', - val3: 'true' - ] - ] - ]) - then: - encoder.encodeObject(_ as Argument) - encoder.encodeKey("keys") - encoder.encodeArray(_ as Argument) - encoder.encodeObject(_ as Argument) - encoder.encodeKey("val1") - encoder.encodeString("abc") - encoder.encodeKey("val2") - encoder.encodeString("123") - encoder.encodeKey("val1") - encoder.encodeString("true") - 3 * encoder.finishStructure() - } -}