Potential regression when using ConstructorDetector.USE_PROPERTIES_BASED

[SamBarker]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

We in the Kroxylicious project have observed a potential regression in the 2.18 release. I think I have managed to reproduce it 😬 The following test passes on with 2.17 but fails on the 2.18/2.19 branch.

On 2.18 it fails with:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid type definition for type `com.fasterxml.jackson.databind.tofix.CreatorResolutionTest$HostPort`: Argument #0 of Creator [method com.fasterxml.jackson.databind.tofix.CreatorResolutionTest$HostPort#parse(java.lang.String)] has no property name (and is not Injectable): can not use as property-based Creator
 at [Source: (String)""localhost:9090""; line: 1, column: 1]

	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadTypeDefinition(DeserializationContext.java:1866)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._addSelectedPropertiesBasedCreator(BasicDeserializerFactory.java:542)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._constructDefaultValueInstantiator(BasicDeserializerFactory.java:264)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.findValueInstantiator(BasicDeserializerFactory.java:219)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:262)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:151)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:471)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:415)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:317)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:669)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:5048)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4918)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at com.fasterxml.jackson.databind.tofix.CreatorResolutionTest.shouldUseCreator(CreatorResolutionTest.java:20)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

This issue replaces FasterXML/jackson-modules-java8#323 as I wrongly thought ParameterNamesModule was involved.

Version Information

Working 2.17.3
Failing on 2.18.1

Reproduction

package com.fasterxml.jackson.databind.tofix;

import javax.annotation.Nonnull;

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.databind.cfg.ConstructorDetector;
import com.fasterxml.jackson.databind.testutil.DatabindTestUtil;

import static org.assertj.core.api.Assertions.assertThat;

class CreatorResolutionTest extends DatabindTestUtil {

    @Test
    void shouldUseCreator() throws Exception {

        CreatorResolutionTest.HostPort value = newJsonMapper()
                .setConstructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
                .readValue(
                        a2q("'localhost:9090'"),
                        CreatorResolutionTest.HostPort.class);
        assertThat(value).isNotNull()
                         .satisfies(actual ->
                                    {
                                        assertThat(actual.getHostname()).isEqualTo("localhost");
                                        assertThat(actual.getPort()).isEqualTo(9090);
                                    });
    }

    public static class HostPort {
        private final String hostname;
        private final int port;

        public HostPort(String hostname, int port) {
            this.hostname = hostname;
            this.port = port;
        }

        @Override
        public String toString() {
            return "HostPort{" +
                   "hostname='" + hostname + '\'' +
                   ", port=" + port +
                   '}';
        }

        @JsonCreator
        public static HostPort parse(@Nonnull String hostAndPort) {
            final String[] parts = hostAndPort.split(":");
            return new HostPort(parts[0], Integer.parseInt(parts[1]));
        }

        public String getHostname() {
            return hostname;
        }

        public int getPort() {
            return port;
        }
    }
}

or #4786

Expected behavior

The static method annotated @JsonCreator is used to parse the string.

Additional context

No response

[cowtowncoder]

The test seems invalid: the way it is defined seems that 2.18.0 actually does the right thing, no? (and 2.17 having incorrect handling but one that happened to do what you actually wanted)

Because:

• .setConstructorDetector(ConstructorDetector.USE_PROPERTIES_BASED) defines Mode.PROPERTIES as the default, if no mode present in @JsonCreator
• @JsonCreator on public static HostPort parse(@Nonnull String hostAndPort) has no mode

hence it is attempting to use Properties style, expecting JSON Object as input -- but input is a JSON String (i.e. what you really want is Mode.DELEGATING).

Exception message looks sub-optimal, sure, but behavior seems correct.

What you want to use is either change default or modify @JsonCreator to be

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

[SamBarker]

Thanks for the quick response.

Sounds reasonable to me, I'll try updating the original failure and see if @JsonCreator(mode = JsonCreator.Mode.DELEGATING) helps there or if I over simplified in creating the issue.

[cowtowncoder]

@SamBarker ah yes, could be something that is an actual problem even if reproduction not.

[SamBarker]

Looks like that fixes the original issue as well.

Apparently we have another test that fails because SerializationFeature.FAIL_ON_EMPTY_BEANS doesn't trigger, but I'm trying to understand whats going on there.

Sorry for the noise.

[cowtowncoder]

@SamBarker no problem at all: when behavior changes it is reasonable to consider breakage. Glad it turned out to be something else (similar things have happened in other cases too, but there are actual bugs too)