diff --git a/.github/workflows/cifuzz.yml b/.github/workflows/cifuzz.yml index a3cb96288d..9ac7362770 100644 --- a/.github/workflows/cifuzz.yml +++ b/.github/workflows/cifuzz.yml @@ -30,7 +30,7 @@ jobs: dry-run: false language: jvm - name: Upload Crash - uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0 + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 if: failure() && steps.build.outcome == 'success' with: name: artifacts diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index a9ba918728..f5304e1f10 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -23,16 +23,16 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/init@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 with: languages: ${{ matrix.language }} - name: Autobuild - uses: github/codeql-action/autobuild@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/autobuild@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@e2b3eafc8d227b0241d48be5f425d47c2d750a13 # v3.26.10 + uses: github/codeql-action/analyze@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 diff --git a/.github/workflows/dep_build_v2.yml b/.github/workflows/dep_build_v2.yml index 681cfa5046..41ce0c1458 100644 --- a/.github/workflows/dep_build_v2.yml +++ b/.github/workflows/dep_build_v2.yml @@ -20,9 +20,9 @@ jobs: env: JAVA_OPTS: "-XX:+TieredCompilation -XX:TieredStopAtLevel=1" steps: - - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up JDK - uses: actions/setup-java@b36c23c0d998641eff861008f374ee103c25ac73 # v4.4.0 + uses: actions/setup-java@8df1039502a15bceb9433410b1a100fbe190c53b # v4.5.0 with: distribution: 'temurin' java-version: ${{ matrix.java_version }} diff --git a/.github/workflows/dep_build_v3.yml b/.github/workflows/dep_build_v3.yml index 5791dd8c19..28064e366b 100644 --- a/.github/workflows/dep_build_v3.yml +++ b/.github/workflows/dep_build_v3.yml @@ -20,11 +20,11 @@ jobs: env: JAVA_OPTS: "-XX:+TieredCompilation -XX:TieredStopAtLevel=1" steps: - - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: master - name: Set up JDK - uses: actions/setup-java@b36c23c0d998641eff861008f374ee103c25ac73 # v4.4.0 + uses: actions/setup-java@8df1039502a15bceb9433410b1a100fbe190c53b # v4.5.0 with: distribution: 'temurin' java-version: ${{ matrix.java_version }} diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cbb079b965..f74ff42f36 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -37,9 +37,9 @@ jobs: env: JAVA_OPTS: "-XX:+TieredCompilation -XX:TieredStopAtLevel=1" steps: - - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up JDK - uses: actions/setup-java@b36c23c0d998641eff861008f374ee103c25ac73 # v4.4.0 + uses: actions/setup-java@8df1039502a15bceb9433410b1a100fbe190c53b # v4.5.0 with: distribution: 'temurin' java-version: ${{ matrix.java_version }} @@ -71,7 +71,7 @@ jobs: run: ./mvnw -B -q -ff -ntp test - name: Publish code coverage if: ${{ github.event_name != 'pull_request' && matrix.release_build }} - uses: codecov/codecov-action@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673 # v4.5.0 + uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4.6.0 with: token: ${{ secrets.CODECOV_TOKEN }} file: ./target/site/jacoco/jacoco.xml diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 0bcade277c..fe2ced2543 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -20,6 +20,19 @@ Project: jackson-databind #4676: Support other enum naming strategies than camelCase (requested by @hajdamak) (contributed by Lars B) +#4680: Custom key deserialiser registered for `Object.class` in nested + Map object is ignored when Map key type not defined + (reported by @devdanylo) + (fix by Joo-Hyuk K) +#4773: `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS` should not apply to Maps + with uncomparable keys + (requested by @nathanukey) + (fix by Joo-Hyuk K) + +2.18.2 (not yet released) + +#4733: Wrong serialization of Type Ids for certain types of Enum values + (reported by @nlisker) 2.18.1 (28-Oct-2024) @@ -120,7 +133,7 @@ Project: jackson-databind #4709: Add `JacksonCollectors` with `toArrayNode()` implementation (contributed by @rikkarth) -2.17.3 (not yet released) +2.17.3 (01-Nov-2024) #4718: Should not fail on trying to serialize `java.time.DateTimeException` diff --git a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java index 1076ce6f3c..7661707aca 100644 --- a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java @@ -1399,14 +1399,14 @@ public JsonCreator.Mode findCreatorAnnotation(MapperConfig config, Annotated /** * Method called to check if introspector can find a Creator it considers - * the "Default Creator": Creator to use as the primary, when no Creator has + * the "Primary Creator": Creator to use as the primary one, when no Creator has * explicit annotation ({@link #findCreatorAnnotation} returns {@code null}). * Examples of default creators include the canonical constructor defined by * Java Records; "Data" classes by frameworks * like Lombok and JVM languages like Kotlin and Scala (case classes) also have * similar concepts. * If introspector can determine that one of given {@link PotentialCreator}s should - * be considered the default, it should return it; if not, should return {@code null}. + * be considered the primary, it should return it; if not, should return {@code null}. * Note that core databind functionality may call this method even in the presence of * explicitly annotated creators; and may or may not use Creator returned depending * on other criteria. @@ -1416,6 +1416,11 @@ public JsonCreator.Mode findCreatorAnnotation(MapperConfig config, Annotated *

* NOTE: method is NOT called for Java Record types; selection of the canonical constructor * as the Primary creator is handled directly by {@link POJOPropertiesCollector} + *

+ * NOTE: naming of this method is unfortunately inconsistent in that "default Creator" + * has other meanings than "primary Creator" -- in other places Jackson code + * refers to no-arguments Constructors as "default Creators". So this method + * ought to have been named {@code findPrimaryCreator()}. * * @param config Configuration settings in effect (for deserialization) * @param valueClass Class being instantiated; defines Creators passed diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java b/src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java index 526cde4bdf..a16e5cbeff 100644 --- a/src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java +++ b/src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java @@ -246,7 +246,7 @@ public enum SerializationFeature implements ConfigFeature * {@link java.util.Calendar} which will always use timezone Calendar value has). * Setting is also ignored by Joda date/time values. *

- * Featured is enabled by default for backwards-compatibility purposes (in + * Feature is enabled by default for backwards-compatibility purposes (in * Jackson 2.12 override was always done if there was explicitly defined timezone). * * @since 2.13 @@ -434,6 +434,23 @@ public enum SerializationFeature implements ConfigFeature */ ORDER_MAP_ENTRIES_BY_KEYS(false), + /** + * Feature that determines whether to intentionally fail when the mapper attempts to + * order map entries with incomparable keys by accessing the first key of the map. + * So depending on the Map implementation, this may not be the same key every time. + *

+ * If enabled, will simply fail by throwing an exception. + * If disabled, will not throw an exception and instead simply return the original map. + *

+ * Note that this feature will apply only when configured to order map entries by keys, either + * through annotation or enabling {@link #ORDER_MAP_ENTRIES_BY_KEYS}. + *

+ * Feature is enabled by default and will default false in Jackson 3 and later. + * + * @since 2.19 + */ + FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY(true), + /* /****************************************************** /* Other diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java index 2a0948ded9..1edfab6c54 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java @@ -122,9 +122,7 @@ public ValueInstantiator constructValueInstantiator(DeserializationContext ctxt) * it with data. Default creator is only used if no other creators are * indicated. * - * @param creator - * Creator method; no-arguments constructor or static factory - * method. + * @param creator Creator method; no-arguments constructor or factory method. */ public void setDefaultCreator(AnnotatedWithParams creator) { _creators[C_DEFAULT] = _fixAccess(creator); diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/UntypedObjectDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/UntypedObjectDeserializer.java index 3cf164fa79..7710adbc63 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/std/UntypedObjectDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/UntypedObjectDeserializer.java @@ -47,6 +47,13 @@ public class UntypedObjectDeserializer protected JsonDeserializer _numberDeserializer; + /** + * Object.class may also have custom key deserializer + * + * @since 2.19 + */ + private KeyDeserializer _customKeyDeserializer; + /** * If {@link java.util.List} has been mapped to non-default implementation, * we'll store type here @@ -73,7 +80,7 @@ public class UntypedObjectDeserializer */ @Deprecated public UntypedObjectDeserializer() { - this(null, null); + this(null, (JavaType) null); } public UntypedObjectDeserializer(JavaType listType, JavaType mapType) { @@ -95,6 +102,7 @@ public UntypedObjectDeserializer(UntypedObjectDeserializer base, _numberDeserializer = (JsonDeserializer) numberDeser; _listType = base._listType; _mapType = base._mapType; + _customKeyDeserializer = base._customKeyDeserializer; _nonMerging = base._nonMerging; } @@ -111,9 +119,27 @@ protected UntypedObjectDeserializer(UntypedObjectDeserializer base, _numberDeserializer = base._numberDeserializer; _listType = base._listType; _mapType = base._mapType; + _customKeyDeserializer = base._customKeyDeserializer; _nonMerging = nonMerging; } + /** + * @since 2.19 + */ + protected UntypedObjectDeserializer(UntypedObjectDeserializer base, + KeyDeserializer keyDeser) + { + super(Object.class); + _mapDeserializer = base._mapDeserializer; + _listDeserializer = base._listDeserializer; + _stringDeserializer = base._stringDeserializer; + _numberDeserializer = base._numberDeserializer; + _listType = base._listType; + _mapType = base._mapType; + _nonMerging = base._nonMerging; + _customKeyDeserializer = keyDeser; + } + /* /********************************************************** /* Initialization @@ -190,19 +216,32 @@ public JsonDeserializer createContextual(DeserializationContext ctxt, // 14-Jun-2017, tatu: [databind#1625]: may want to block merging, for root value boolean preventMerge = (property == null) && Boolean.FALSE.equals(ctxt.getConfig().getDefaultMergeable(Object.class)); + // Since 2.19, 31-Aug-2024: [databind#4680] Allow custom key deserializer for Object.class + KeyDeserializer customKeyDeser = ctxt.findKeyDeserializer(ctxt.constructType(Object.class), property); + // but make sure to ignore standard/default key deserializer (perf optimization) + if (customKeyDeser != null) { + if (ClassUtil.isJacksonStdImpl(customKeyDeser)) { + customKeyDeser = null; + } + } // 20-Apr-2014, tatu: If nothing custom, let's use "vanilla" instance, // simpler and can avoid some of delegation if ((_stringDeserializer == null) && (_numberDeserializer == null) && (_mapDeserializer == null) && (_listDeserializer == null) + && (customKeyDeser == null) // [databind#4680] Since 2.19 : Allow custom key deserializer for Object.class && getClass() == UntypedObjectDeserializer.class) { return UntypedObjectDeserializerNR.instance(preventMerge); } + UntypedObjectDeserializer deser = this; if (preventMerge != _nonMerging) { - return new UntypedObjectDeserializer(this, preventMerge); + deser = new UntypedObjectDeserializer(deser, preventMerge); } - - return this; + // [databind#4680] Since 2.19 : Allow custom key deserializer for Object.class + if (customKeyDeser != null) { + deser = new UntypedObjectDeserializer(deser, customKeyDeser); + } + return deser; } /* @@ -496,6 +535,7 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE // empty map might work; but caller may want to modify... so better just give small modifiable return new LinkedHashMap<>(2); } + key1 = _customDeserializeKey(key1, ctxt); // minor optimization; let's handle 1 and 2 entry cases separately // 24-Mar-2015, tatu: Ideally, could use one of 'nextXxx()' methods, but for // that we'd need new method(s) in JsonDeserializer. So not quite yet. @@ -508,6 +548,8 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE result.put(key1, value1); return result; } + key2 = _customDeserializeKey(key2, ctxt); + p.nextToken(); Object value2 = deserialize(p, ctxt); @@ -521,6 +563,8 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE } return result; } + key = _customDeserializeKey(key, ctxt); + // And then the general case; default map size is 16 LinkedHashMap result = new LinkedHashMap<>(); result.put(key1, value1); @@ -535,9 +579,9 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE final Object oldValue = result.put(key, newValue); if (oldValue != null) { return _mapObjectWithDups(p, ctxt, result, key, oldValue, newValue, - p.nextFieldName()); + _customDeserializeNullableKey(p.nextFieldName(), ctxt)); } - } while ((key = p.nextFieldName()) != null); + } while ((key = _customDeserializeNullableKey(p.nextFieldName(), ctxt)) != null); return result; } @@ -559,12 +603,44 @@ protected Object _mapObjectWithDups(JsonParser p, DeserializationContext ctxt, if ((oldValue != null) && squashDups) { _squashDups(result, key, oldValue, newValue); } - nextKey = p.nextFieldName(); + nextKey = _customDeserializeNullableKey(p.nextFieldName(), ctxt); } return result; } + /** + * Helper function to allow custom key deserialization without null handling. + * Similar to {@link #_customDeserializeNullableKey(String, DeserializationContext)}, but + * null handling is done by the caller. + * + * @returns Custom-deserialized key if both custom key deserializer is set. + * Otherwise the original key. + */ + private final String _customDeserializeKey(String key, DeserializationContext ctxt) throws IOException { + if (_customKeyDeserializer != null) { + return (String) _customKeyDeserializer.deserializeKey(key, ctxt); + } + return key; + } + + /** + * Helper function to allow custom key deserialization with null handling. + * Similar to {@link #_customDeserializeKey(String, DeserializationContext)}, but instead + * only returns custom-deserialized key if key is not null. + * + * @returns Custom-deserialized key if both custom key deserializer is set and key is not null. + * Otherwise the original key. + */ + private final String _customDeserializeNullableKey(String key, DeserializationContext ctxt) throws IOException { + if (_customKeyDeserializer != null) { + if (key != null) { + return (String) _customKeyDeserializer.deserializeKey(key, ctxt); + } + } + return key; + } + @SuppressWarnings("unchecked") private void _squashDups(final Map result, String key, Object oldValue, Object newValue) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java index 44e9d1faa4..e57e409715 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java @@ -739,13 +739,13 @@ public PotentialCreator findDefaultCreator(MapperConfig config, AnnotatedClass valueClass, List declaredConstructors, List declaredFactories) { - PotentialCreator defCtor = _primary.findDefaultCreator(config, + PotentialCreator primaryCtor = _primary.findDefaultCreator(config, valueClass, declaredConstructors, declaredFactories); - if (defCtor == null) { - defCtor = _secondary.findDefaultCreator(config, + if (primaryCtor == null) { + primaryCtor = _secondary.findDefaultCreator(config, valueClass, declaredConstructors, declaredFactories); } - return defCtor; + return primaryCtor; } /* 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 97f56bb6c1..4954ec7b31 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -657,14 +657,17 @@ protected void _addCreators(Map props) List constructors = _collectCreators(_classDef.getConstructors()); List factories = _collectCreators(_classDef.getFactoryMethods()); - // Then find what is the Default Constructor (if one exists for type): + // Then find what is the Primary Constructor (if one exists for type): // for Java Records and potentially other types too ("data classes"): // Needs to be done early to get implicit names populated - final PotentialCreator defaultCreator; + final PotentialCreator primaryCreator; if (_isRecordType) { - defaultCreator = JDK14Util.findCanonicalRecordConstructor(_config, _classDef, constructors); + primaryCreator = JDK14Util.findCanonicalRecordConstructor(_config, _classDef, constructors); } else { - defaultCreator = _annotationIntrospector.findDefaultCreator(_config, _classDef, + // 02-Nov-2024, tatu: Alas, naming here is confusing: method properly + // should have been "findPrimaryCreator()" so as not to confused with + // 0-args default Creators... + primaryCreator = _annotationIntrospector.findDefaultCreator(_config, _classDef, constructors, factories); } // Next: remove creators marked as explicitly disabled @@ -672,7 +675,7 @@ protected void _addCreators(Map props) _removeDisabledCreators(factories); // And then remove non-annotated static methods that do not look like factories - _removeNonFactoryStaticMethods(factories, defaultCreator); + _removeNonFactoryStaticMethods(factories, primaryCreator); // and use annotations to find explicitly chosen Creators if (_useAnnotations) { // can't have explicit ones without Annotation introspection @@ -683,30 +686,30 @@ protected void _addCreators(Map props) creators.hasPropertiesBased()); } - // If no Explicitly annotated creators (or Default one) found, look + // If no Explicitly annotated creators (or Primary one) found, look // for ones with explicitly-named ({@code @JsonProperty}) parameters if (!creators.hasPropertiesBased()) { // only discover constructor Creators? - _addCreatorsWithAnnotatedNames(creators, constructors, defaultCreator); + _addCreatorsWithAnnotatedNames(creators, constructors, primaryCreator); } - // But if no annotation-based Creators found, find/use Default Creator + // But if no annotation-based Creators found, find/use Primary Creator // detected earlier, if any - if (defaultCreator != null) { + if (primaryCreator != null) { // ... but only process if still included as a candidate - if (constructors.remove(defaultCreator) - || factories.remove(defaultCreator)) { + if (constructors.remove(primaryCreator) + || factories.remove(primaryCreator)) { // and then consider delegating- vs properties-based - if (_isDelegatingConstructor(defaultCreator)) { + if (_isDelegatingConstructor(primaryCreator)) { // 08-Oct-2024, tatu: [databind#4724] Only add if no explicit // candidates added if (!creators.hasDelegating()) { // ... not technically explicit but simpler this way - creators.addExplicitDelegating(defaultCreator); + creators.addExplicitDelegating(primaryCreator); } - } else { // default creator is properties-based + } else { // primary creator is properties-based if (!creators.hasPropertiesBased()) { - creators.setPropertiesBased(_config, defaultCreator, "Primary"); + creators.setPropertiesBased(_config, primaryCreator, "Primary"); } } } @@ -718,7 +721,7 @@ protected void _addCreators(Map props) final ConstructorDetector ctorDetector = _config.getConstructorDetector(); if (!creators.hasPropertiesBasedOrDelegating() && !ctorDetector.requireCtorAnnotation()) { - // But only if no default constructor available OR if we are configured + // But only if no Default (0-args) constructor available OR if we are configured // to prefer properties-based Creators if ((_classDef.getDefaultConstructor() == null) || ctorDetector.singleArgCreatorDefaultsToProperties()) { @@ -754,7 +757,7 @@ private boolean _isDelegatingConstructor(PotentialCreator ctor) case DISABLED: case PROPERTIES: return false; - default: // case DEFAULT: + default: } // Only consider single-arg case, for now @@ -805,7 +808,7 @@ private void _removeNonVisibleCreators(List ctors) } private void _removeNonFactoryStaticMethods(List ctors, - PotentialCreator defaultCreator) + PotentialCreator primaryCreator) { final Class rawType = _type.getRawClass(); Iterator it = ctors.iterator(); @@ -815,8 +818,8 @@ private void _removeNonFactoryStaticMethods(List ctors, if (ctor.isAnnotated()) { continue; } - // Do not trim canonical either - if (defaultCreator == ctor) { + // Do not trim Primary creator either + if (primaryCreator == ctor) { continue; } // Copied from `BasicBeanDescription.isFactoryMethod()` @@ -933,14 +936,14 @@ private boolean _isExplicitlyAnnotatedCreatorPropsBased(PotentialCreator ctor, } private void _addCreatorsWithAnnotatedNames(PotentialCreators collector, - List ctors, PotentialCreator defaultCtor) + List ctors, PotentialCreator primaryCtor) { final List found = _findCreatorsWithAnnotatedNames(ctors); - // 16-Jul-2024, tatu: [databind#4620] If Default Creator found, it + // 16-Jul-2024, tatu: [databind#4620] If Primary Creator found, it // will be used to resolve candidate to use, if any - if (defaultCtor != null) { - if (found.contains(defaultCtor)) { - collector.setPropertiesBased(_config, defaultCtor, "implicit"); + if (primaryCtor != null) { + if (found.contains(primaryCtor)) { + collector.setPropertiesBased(_config, primaryCtor, "implicit"); return; } } diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java index 7900008308..9952a00640 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/ClassNameIdResolver.java @@ -92,17 +92,7 @@ protected JavaType _typeFromId(String id, DatabindContext ctxt) throws IOExcepti protected String _idFrom(Object value, Class cls, TypeFactory typeFactory) { - // Need to ensure that "enum subtypes" work too - if (ClassUtil.isEnumType(cls)) { - // 29-Sep-2019, tatu: `Class.isEnum()` only returns true for main declaration, - // but NOT from sub-class thereof (extending individual values). This - // is why additional resolution is needed: we want class that contains - // enumeration instances. - if (!cls.isEnum()) { - // and this parent would then have `Enum.class` as its parent: - cls = cls.getSuperclass(); - } - } + cls = _resolveToParentAsNecessary(cls); String str = cls.getName(); if (str.startsWith(JAVA_UTIL_PKG)) { // 25-Jan-2009, tatu: There are some internal classes that we cannot access as is. diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java index 0f5ba38786..c4f08b07b0 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/MinimalClassNameIdResolver.java @@ -58,14 +58,23 @@ public static MinimalClassNameIdResolver construct(JavaType baseType, MapperConf @Override public String idFromValue(Object value) { - String n = value.getClass().getName(); + return idFromValueAndType(value, value.getClass()); + } + + @Override + public String idFromValueAndType(Object value, Class rawType) { + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + rawType = _resolveToParentAsNecessary(rawType); + String n = rawType.getName(); if (n.startsWith(_basePackagePrefix)) { // note: we will leave the leading dot in there return n.substring(_basePackagePrefix.length()-1); } return n; + } - + @Override protected JavaType _typeFromId(String id, DatabindContext ctxt) throws IOException { diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java index 8af7473344..34afe3c36b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/SimpleNameIdResolver.java @@ -120,6 +120,10 @@ protected String idFromClass(Class clazz) if (clazz == null) { return null; } + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + clazz = _resolveToParentAsNecessary(clazz); + // NOTE: although we may need to let `TypeModifier` change actual type to use // for id, we can use original type as key for more efficient lookup: final String key = clazz.getName(); diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java index 6f4710a46b..9b4d7bb884 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeIdResolverBase.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.jsontype.TypeIdResolver; import com.fasterxml.jackson.databind.type.TypeFactory; +import com.fasterxml.jackson.databind.util.ClassUtil; /** * Partial base implementation of {@link TypeIdResolver}: all custom implementations @@ -69,4 +70,32 @@ public JavaType typeFromId(DatabindContext context, String id) throws IOExcepti public String getDescForKnownTypeIds() { return null; } + + /** + * Helper method for ensuring we properly resolve cases where we don't + * want to use given instance class due to it being a specific inner class + * but rather enclosing (or parent) class. Specific case we know of + * currently are "enum subtypes", cases + * where simple Enum constant has overrides and uses generated sub-class + * if parent Enum type. In this case we need to ensure that we use + * the main/parent Enum type, not sub-class. + * + * @param cls Class to check and possibly resolve + * @return Resolved class to use + * @since 2.18.2 + */ + protected Class _resolveToParentAsNecessary(Class cls) { + // Need to ensure that "enum subtypes" work too + if (ClassUtil.isEnumType(cls)) { + // 29-Sep-2019, tatu: `Class.isEnum()` only returns true for main declaration, + // but NOT from sub-class thereof (extending individual values). This + // is why additional resolution is needed: we want class that contains + // enumeration instances. + if (!cls.isEnum()) { + // and this parent would then have `Enum.class` as its parent: + cls = cls.getSuperclass(); + } + } + return cls; + } } diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java index 2d459514a5..88ee907a16 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java @@ -121,6 +121,10 @@ protected String idFromClass(Class clazz) if (clazz == null) { return null; } + // 04-Nov-2024, tatu: [databind#4733] Need to resolve enum sub-classes + // same way "ClassNameIdResolver" does + clazz = _resolveToParentAsNecessary(clazz); + // NOTE: although we may need to let `TypeModifier` change actual type to use // for id, we can use original type as key for more efficient lookup: final String key = clazz.getName(); diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java b/src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java index e5147cb829..c5c5019924 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java @@ -1166,6 +1166,26 @@ protected Map _orderEntries(Map input, JsonGenerator gen, if (input instanceof SortedMap) { return input; } + // or is it empty? then no need to sort either + if (input.isEmpty()) { + return input; + } + // [databind#4773] Since 2.19: We should not try sorting Maps with uncomparable keys + // And first key is a good enough sample for now. + Object firstKey = input.keySet().iterator().next(); + if (!Comparable.class.isInstance(firstKey)) { + // We cannot sort incomparable keys, should we fail or just skip sorting? + if (!provider.isEnabled(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY)) { + return input; + } else { + Class clazz = firstKey == null ? Object.class : firstKey.getClass(); + provider.reportBadDefinition(clazz, + String.format("Cannot order Map entries by key of incomparable type %s, consider disabling " + + "`SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY` to simply skip sorting", + ClassUtil.classNameOf(firstKey))); + } + } + // [databind#1411]: TreeMap does not like null key... (although note that // check above should prevent this code from being called in that case) // [databind#153]: but, apparently, some custom Maps do manage hit this diff --git a/src/test/java/com/fasterxml/jackson/databind/tofix/CustomObjectKeyDeserializer4680Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/CustomMapKeyDeserializer4680Test.java similarity index 90% rename from src/test/java/com/fasterxml/jackson/databind/tofix/CustomObjectKeyDeserializer4680Test.java rename to src/test/java/com/fasterxml/jackson/databind/deser/jdk/CustomMapKeyDeserializer4680Test.java index 48045108d5..26cb8cb624 100644 --- a/src/test/java/com/fasterxml/jackson/databind/tofix/CustomObjectKeyDeserializer4680Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/CustomMapKeyDeserializer4680Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.tofix; +package com.fasterxml.jackson.databind.deser.jdk; import java.util.Map; @@ -10,15 +10,13 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.module.SimpleModule; -import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected; import static org.junit.jupiter.api.Assertions.assertEquals; -// [databind#4680] Custom key deserialiser registered for `Object.class` is ignored on nested JSON -public class CustomObjectKeyDeserializer4680Test +// [databind#4680] Custom key deserializer registered for `Object.class` is ignored on nested JSON +public class CustomMapKeyDeserializer4680Test { - - @JacksonTestFailureExpected + @SuppressWarnings("unchecked") @Test void testCustomKeyDeserializer() throws Exception diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java new file mode 100644 index 0000000000..2ae8503457 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/jdk/EnumTyping4733Test.java @@ -0,0 +1,127 @@ +package com.fasterxml.jackson.databind.jsontype.jdk; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; + +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +public class EnumTyping4733Test extends DatabindTestUtil +{ + // Baseline case that already worked + @JsonTypeInfo(use = Id.CLASS) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_CLASS.class), + }) + interface InterClass { + default void yes() {} + } + + enum A_CLASS implements InterClass { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.MINIMAL_CLASS) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_MIN_CLASS.class), + }) + interface InterMinimalClass { + default void yes() {} + } + + enum A_MIN_CLASS implements InterMinimalClass { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.NAME) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_NAME.class), + }) + interface InterName { + default void yes() {} + } + + enum A_NAME implements InterName { + A1, + A2 { + @Override + public void yes() { } + }; + } + + // Failed before fix for [databind#4733] + @JsonTypeInfo(use = Id.SIMPLE_NAME) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A_SIMPLE_NAME.class), + }) + interface InterSimpleName { + default void yes() {} + } + + enum A_SIMPLE_NAME implements InterSimpleName { + A1, + A2 { + @Override + public void yes() { } + }; + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + @Test + public void testIssue4733Class() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_CLASS.A1); + String json2 = MAPPER.writeValueAsString(A_CLASS.A2); + + assertEquals(A_CLASS.A1, MAPPER.readValue(json1, A_CLASS.class)); + assertEquals(A_CLASS.A2, MAPPER.readValue(json2, A_CLASS.class)); + } + + @Test + public void testIssue4733MinimalClass() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_MIN_CLASS.A1); + String json2 = MAPPER.writeValueAsString(A_MIN_CLASS.A2); + assertEquals(A_MIN_CLASS.A1, MAPPER.readValue(json1, A_MIN_CLASS.class), + "JSON: "+json1); + assertEquals(A_MIN_CLASS.A2, MAPPER.readValue(json2, A_MIN_CLASS.class), + "JSON: "+json2); + } + + @Test + public void testIssue4733Name() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_NAME.A1); + String json2 = MAPPER.writeValueAsString(A_NAME.A2); + assertEquals(A_NAME.A1, MAPPER.readValue(json1, A_NAME.class), + "JSON: "+json1); + assertEquals(A_NAME.A2, MAPPER.readValue(json2, A_NAME.class), + "JSON: "+json2); + } + + @Test + public void testIssue4733SimpleName() throws Exception + { + String json1 = MAPPER.writeValueAsString(A_SIMPLE_NAME.A1); + String json2 = MAPPER.writeValueAsString(A_SIMPLE_NAME.A2); + assertEquals(A_SIMPLE_NAME.A1, MAPPER.readValue(json1, A_SIMPLE_NAME.class), + "JSON: "+json1); + assertEquals(A_SIMPLE_NAME.A2, MAPPER.readValue(json2, A_SIMPLE_NAME.class), + "JSON: "+json2); + } +} diff --git a/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationSorted4773Test.java b/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationSorted4773Test.java new file mode 100644 index 0000000000..7f62d830d4 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationSorted4773Test.java @@ -0,0 +1,99 @@ +package com.fasterxml.jackson.databind.ser.jdk; + +import java.util.Currency; +import java.util.HashMap; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; + +import static org.junit.jupiter.api.Assertions.*; + +// [databind#4773] Test to verify `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS` behavior +// when serializing `Map` instances with un-comparable keys. +public class MapSerializationSorted4773Test + extends DatabindTestUtil +{ + public static class IncomparableContainer4773 { + public Map exampleMap = new HashMap<>(); + } + + public static class ObjectContainer4773 { + public Map exampleMap = new HashMap<>(); + } + + private final ObjectMapper objectMapper = jsonMapperBuilder() + .configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true) + .build(); + + @Test + void testSerializationFailureWhenEnabledWithIncomparableKeys() + throws Exception + { + // Given + IncomparableContainer4773 entity = new IncomparableContainer4773(); + entity.exampleMap.put(Currency.getInstance("GBP"), "GBP_TEXT"); + entity.exampleMap.put(Currency.getInstance("AUD"), "AUD_TEXT"); + + // When : Throws exception + // com.fasterxml.jackson.databind.JsonMappingException: class java.util.Currency cannot be cast to class java.lang.Comparable + try { + objectMapper.writer() + .with(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY) + .writeValueAsString(entity); + fail("Should not pass"); + } catch (InvalidDefinitionException e) { + // Then + verifyException(e, "Cannot order Map entries by key of incomparable type"); + } + } + + @Test + void testSerializationWithGenericObjectKeys() + throws Exception + { + // Given + ObjectContainer4773 entity = new ObjectContainer4773(); + entity.exampleMap.put(5, "N_TEXT"); + entity.exampleMap.put(1, "GBP_TEXT"); + entity.exampleMap.put(3, "T_TEXT"); + entity.exampleMap.put(4, "AUD_TEXT"); + entity.exampleMap.put(2, "KRW_TEXT"); + + // When + String jsonResult = objectMapper.writeValueAsString(entity); + + // Then + assertEquals(a2q("{'exampleMap':{" + + "'1':'GBP_TEXT'," + + "'2':'KRW_TEXT'," + + "'3':'T_TEXT'," + + "'4':'AUD_TEXT'," + + "'5':'N_TEXT'}}"), jsonResult); + } + + @Test + void testSerWithNullType() + throws Exception + { + // Given : Mixed keys with incomparable `Currency` and comparable `Integer` + ObjectContainer4773 entity = new ObjectContainer4773(); + entity.exampleMap.put(null, "AUD_TEXT"); + + // When : Throws exception + try { + objectMapper.writer() + .with(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY) + .writeValueAsString(entity); + fail("Should not pass"); + } catch (InvalidDefinitionException e) { + // Then + verifyException(e, "Cannot order Map entries by key of incomparable type [null]"); + } + } + +} diff --git a/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationTest.java b/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationTest.java index d52198eb9f..6ceef81128 100644 --- a/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationTest.java @@ -164,7 +164,8 @@ public void testOrderByKey() throws IOException @Test public void testOrderByWithNulls() throws IOException { - ObjectWriter sortingW = MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS); + ObjectWriter sortingW = MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS) + .without(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY); // 16-Oct-2016, tatu: but mind the null key, if any Map mapWithNullKey = new LinkedHashMap(); mapWithNullKey.put(null, 1); diff --git a/src/test/java/com/fasterxml/jackson/databind/tofix/JsonCreatorNoArgs4777Test.java b/src/test/java/com/fasterxml/jackson/databind/tofix/JsonCreatorNoArgs4777Test.java new file mode 100644 index 0000000000..3115d8fb89 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/tofix/JsonCreatorNoArgs4777Test.java @@ -0,0 +1,64 @@ +package com.fasterxml.jackson.databind.tofix; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.deser.*; +import com.fasterxml.jackson.databind.introspect.AnnotatedMethod; +import com.fasterxml.jackson.databind.introspect.AnnotatedWithParams; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; +import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class JsonCreatorNoArgs4777Test extends DatabindTestUtil +{ + static class Foo { + private Foo() { } + + @JsonCreator + static Foo create() { + return new Foo(); + } + } + + static class Instantiators implements ValueInstantiators { + @Override + public ValueInstantiator findValueInstantiator( + DeserializationConfig config, + BeanDescription beanDesc, + ValueInstantiator defaultInstantiator + ) { + if (beanDesc.getBeanClass() == Foo.class) { + AnnotatedWithParams dc = defaultInstantiator.getDefaultCreator(); + if (!(dc instanceof AnnotatedMethod) + || !dc.getName().equals("create")) { + throw new IllegalArgumentException("Wrong DefaultCreator: should be static-method 'create()', is: " + +dc); + } + } + return defaultInstantiator; + } + } + + // For [databind#4777] + @SuppressWarnings("serial") + @Test + @JacksonTestFailureExpected + public void testCreatorDetection4777() throws Exception { + SimpleModule sm = new SimpleModule() { + @Override + public void setupModule(SetupContext context) { + super.setupModule(context); + context.addValueInstantiators(new Instantiators()); + } + }; + ObjectMapper mapper = JsonMapper.builder().addModule(sm).build(); + + Foo result = mapper.readValue("{}", Foo.class); + assertNotNull(result); + } +}