FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.51k stars 1.37k forks source link

A conversion that worked before doesn't anymore:serviceInstanceRequest]]: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Conflicting getter definitions for property "organization_guid" #3255

Open TerryHuang91 opened 3 years ago

TerryHuang91 commented 3 years ago

Describe the bug A conversion that worked before doesn't anymore: serviceInstanceRequest]]: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Conflicting getter definitions for property "organization_guid"

Version information Which Jackson version(s) was this for? upgrade from 2.11.4 to 2.12.4

To Reproduce If you have a way to reproduce this with:

Here is the pojo class: https://github.com/spring-cloud/spring-cloud-open-service-broker/blob/main/spring-cloud-open-service-broker-core/src/main/java/org/springframework/cloud/servicebroker/model/instance/CreateServiceInstanceRequest.java

Convert this class will return this error log: _com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Conflicting getter definitions for property "organizationguid": org.springframework.cloud.servicebroker.model.instance.CreateServiceInstanceRequest#getOrganizationGuid() vs org.springframework.cloud.servicebroker.model.instance.CreateServiceInstanceRequest#getOrganizationGuidToSerialize()

Expected behavior Conversion should succeed.

Additional context spring-cloud-starter-open-service-broker 3.3.0

cowtowncoder commented 3 years ago

I would need a trimmed down version of the issue here, unfortunately, to see what kind of a change we are talking about.

TerryHuang91 commented 3 years ago

I think the core realted lib is:
`

com.fasterxml.jackson.core jackson-databind

`

If I specify the version to 2.11.4, this issue would not happed.

cowtowncoder commented 3 years ago

Yes, relevant code is in jackson-databind. But without more compact test case I don't think I can really help.

TerryHuang91 commented 3 years ago
<dependency>
    <groupId>com.fasterxml.jackson.core</groupId>
    <artifactId>jackson-databind</artifactId>
    <version>2.12.4</version>
</dependency>

<dependency>
       <groupId>com.fasterxml.jackson.dataformat</groupId>
    <artifactId>jackson-dataformat-xml</artifactId>
    <version>2.12.4</version>
</dependency>
<dependency>
    <groupId>org.springframework.cloud</groupId>
    <artifactId>spring-cloud-starter-open-service-broker</artifactId>
    <version>3.3.0</version>
</dependency>
    public void testJackson() {
        String serviceInstanceStr = "{\n" + 
                "  \"context\": {\n" + 
                "    \"subaccount_id\":\"123456\",\n" + 
                "    \"crm_customer_id\": \"159918\",\n" + 
                "    \"platform\": \"cloudfoundry\",\n" + 
                "    \"some_field\": \"some-contextual-data\"\n" + 
                "  },\n" + 
                "  \"service_id\": \"bdb1be2e-360b-495c-8115-d7697f9c6a9f\",\n" + 
                "  \"plan_id\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                "  \"organization_guid\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                " \"space_guid\": \"space-guid-here\","+
                "  \"bind_resource\": {\n" + 
                "    \"app_guid\": \"app-guid-here\"\n" + 
                "  }\n" + 
                "}";
     //it will failed at method this.objectMapper.canDeserialize()  
     CreateServiceInstanceRequest result = this.objectMapper.convertValue(serviceInstanceStr, CreateServiceInstanceRequest.class);

Here is the test code, it will faild at method this.objectMapper.canDeserialize()

cowtowncoder commented 3 years ago

Thank you @doublezhuang: this helps, but the POJO class is 600 lines long. Would it be possible to try to trim that down to something smaller?

TerryHuang91 commented 3 years ago

Hi,

I copy the original class and simplify it to less than 150 rows, may be you can use it now.

    public void testJackson() {
        String serviceInstanceStr = "{\n" + 
                "  \"context\": {\n" + 
                "    \"subaccount_id\":\"123456\",\n" + 
                "    \"crm_customer_id\": \"159918\",\n" + 
                "    \"platform\": \"cloudfoundry\",\n" + 
                "    \"some_field\": \"some-contextual-data\"\n" + 
                "  },\n" + 
                "  \"service_id\": \"bdb1be2e-360b-495c-8115-d7697f9c6a9f\",\n" + 
                "  \"plan_id\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                "  \"organization_guid\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                " \"space_guid\": \"space-guid-here\","+
                "  \"bind_resource\": {\n" + 
                "    \"app_guid\": \"app-guid-here\"\n" + 
                "  }\n" + 
                "}";
     //it will failed at method this.objectMapper.canDeserialize()  
     TestModel result = this.objectMapper.convertValue(serviceInstanceStr, TestModel.class);

Here is the TestModel class

import java.util.Map;

import javax.validation.constraints.NotEmpty;

import org.springframework.cloud.servicebroker.model.CloudFoundryContext;
import org.springframework.cloud.servicebroker.model.Context;
import org.springframework.cloud.servicebroker.model.catalog.Plan;
import org.springframework.cloud.servicebroker.model.catalog.ServiceDefinition;
import org.springframework.cloud.servicebroker.model.instance.AsyncParameterizedServiceInstanceRequest;

import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;

@SuppressWarnings({"deprecation", "DeprecatedIsStillUsed"})
public class TestModel extends AsyncParameterizedServiceInstanceRequest {

    @NotEmpty
    @JsonProperty("service_id")
    private final String serviceDefinitionId;

    @NotEmpty
    @JsonProperty("plan_id")
    private final String planId;

    /**
     * remains in the model for marshalling support but test harnesses should not use
     */
    @Deprecated
    private final String organizationGuid;

    /**
     * remains in the model for marshalling support but test harnesses should not use
     */
    @Deprecated
    private final String spaceGuid;

    @JsonIgnore //mapped as path param
    private transient String serviceInstanceId;

    @JsonIgnore /*internal field*/
    private transient ServiceDefinition serviceDefinition;

    @JsonIgnore /*internal field*/
    private transient Plan plan;

    /**
     * Construct a new {@link TestModel}
     */
    public TestModel() {
        this(null, null, null, null, null, null, null, false, null, null, null, null);
    }

    public TestModel(String serviceDefinitionId, String serviceInstanceId, String planId,
            ServiceDefinition serviceDefinition, Plan plan, Map<String, Object> parameters, Context context,
            boolean asyncAccepted, String platformInstanceId, String apiInfoLocation, Context originatingIdentity,
            String requestIdentity) {
        this(serviceDefinitionId, serviceInstanceId, planId, serviceDefinition, plan, parameters, context,
                asyncAccepted, platformInstanceId, apiInfoLocation, originatingIdentity, requestIdentity, null, null
                );
    }

    private TestModel(String serviceDefinitionId, String serviceInstanceId, String planId,
            ServiceDefinition serviceDefinition, Plan plan, Map<String, Object> parameters, Context context,
            boolean asyncAccepted, String platformInstanceId, String apiInfoLocation, Context originatingIdentity,
            String requestIdentity, String organizationGuid, String spaceGuid ) {
        super(parameters, context, asyncAccepted, platformInstanceId, apiInfoLocation, originatingIdentity,
                requestIdentity);
        this.serviceDefinitionId = serviceDefinitionId;
        this.serviceInstanceId = serviceInstanceId;
        this.planId = planId;
        this.serviceDefinition = serviceDefinition;
        this.plan = plan;
        this.organizationGuid = organizationGuid;
        this.spaceGuid = spaceGuid;
    }

    public String getServiceInstanceId() {
        return this.serviceInstanceId;
    }

    public void setServiceInstanceId(final String serviceInstanceId) {
        this.serviceInstanceId = serviceInstanceId;
    }
    public String getServiceDefinitionId() {
        return this.serviceDefinitionId;
    }

    public String getPlanId() {
        return this.planId;
    }
    @Deprecated
    public String getOrganizationGuid() {
        return this.organizationGuid;
    }

    @Deprecated
    public String getSpaceGuid() {
        return this.spaceGuid;
    }

    @JsonGetter("space_guid")
    protected String getSpaceGuidToSerialize() {
        //prefer explicitly set field if any
        String spaceGuid = this.spaceGuid;
        //then use cloudfoundry context if any
        if (spaceGuid == null && getContext() instanceof CloudFoundryContext) {
            spaceGuid = ((CloudFoundryContext) getContext()).getSpaceGuid();
        }
        //Otherwise, default to an arbitrary string
        if (spaceGuid == null) {
            spaceGuid = "default-undefined-value"; //OSB spec says "MUST be a non-empty string."
        }
        return spaceGuid;
    }

    /**
     * Determine the organization GUID
     *
     * @return the organization GUID
     */
    @JsonGetter("organization_guid")
    protected String getOrganizationGuidToSerialize() {
        //prefer explicitly set field if any
        String organizationGuid = this.organizationGuid;
        //then use cloudfoundry context if any
        if (organizationGuid == null && getContext() instanceof CloudFoundryContext) {
            organizationGuid = ((CloudFoundryContext) getContext()).getOrganizationGuid();
        }
        //Otherwise, default to an arbitrary string
        if (organizationGuid == null) {
            organizationGuid = "default-undefined-value"; //OSB spec says "MUST be a non-empty string."
        }
        return organizationGuid;
    }

    public ServiceDefinition getServiceDefinition() {
        return this.serviceDefinition;
    }

    public void setServiceDefinition(ServiceDefinition serviceDefinition) {
        this.serviceDefinition = serviceDefinition;
    }

}
TerryHuang91 commented 3 years ago

Hi @cowtowncoder ,

is this short pojo can be used?

Best regards, Zhuang

cowtowncoder commented 3 years ago

Thank you @doublezhuang: yes, this is much better.

cowtowncoder commented 3 years ago

Oh. Actually... we'll see. There are references to unknown types; I'll see if I can simply drop them and reproduce the issue. If so, this is fine.

cowtowncoder commented 3 years ago

No, unfortunately I cannot create a failing test after all. I will need something without various context objects (or mocked); something that can be run as-is, with no references to missing classes.

But one thing I would suggest as a workaround would be simply to add @JsonIgnore on this:

    @Deprecated
    public String getOrganizationGuid() {

since Jackson does not make any use of @Deprecated marker.

TerryHuang91 commented 3 years ago

But the pojo class if also defined by 3rd party open source lib: spring-cloud-starter-open-service-broker, I have no chance to change it. You may add this lib to you test project.

org.springframework.cloud spring-cloud-starter-open-service-broker 3.3.0
cowtowncoder commented 3 years ago

I am sorry but unit tests cannot contain additional 3rd party dependencies. I don't think these dependencies cause the issue but I do not have time to spend on adding all kinds of various set ups for a single case. It should be possible to somehow reduce the test but I just cannot reproduce this particular issue with a scaled-down version. And until I can do that, I cannot really see what is going on.

But one other thing that is missing and matters a lot: how exactly is ObjectMapper in use configured? Code absolutely needs this; for example, maybe it specifies a PropertyNamingStrategy to use? This is why I need a test case and not simply model class(es).

I would still suggest adding a @JsonIgnore in the method reported as conflicting to see how to work around the problme.

TerryHuang91 commented 3 years ago

I tried to remove the 3rd party denpenddencies, but this issue won't be reprocduced again. And I don't know why. And you are right, add a @JsonIgnore may helps and I forward this point to service broker teams.

BTW, here is the test case which include the objectMapper init:

class TestSample {

    @Test
    public void testJackson() {
        String serviceInstanceStr = "{\n" + 
                "  \"context\": {\n" + 
                "    \"subaccount_id\":\"123456\",\n" + 
                "    \"crm_customer_id\": \"159918\",\n" + 
                "    \"platform\": \"cloudfoundry\",\n" + 
                "    \"some_field\": \"some-contextual-data\"\n" + 
                "  },\n" + 
                "  \"service_id\": \"bdb1be2e-360b-495c-8115-d7697f9c6a9f\",\n" + 
                "  \"plan_id\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                "  \"organization_guid\": \"b973fb78-82f3-49ef-9b8b-c1876974a6cf\",\n" + 
                " \"space_guid\": \"space-guid-here\","+
                "  \"bind_resource\": {\n" + 
                "    \"app_guid\": \"app-guid-here\"\n" + 
                "  }\n" + 
                "}";
     //it will failed at method this.objectMapper.canDeserialize()  

            ObjectMapper objectMapper = new ObjectMapper();
            //objectMapper.setVisibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE);

            objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
            TestModel result = objectMapper.convertValue(serviceInstanceStr, TestModel.class);
            assertNotNull(result);
    }
}

Could you please just introduce this service broker lib into the test project and run it:

<dependency>
    <groupId>com.fasterxml.jackson.core</groupId>
    <artifactId>jackson-databind</artifactId>
    <version>2.12.4</version>
</dependency>

<dependency>
       <groupId>com.fasterxml.jackson.dataformat</groupId>
    <artifactId>jackson-dataformat-xml</artifactId>
    <version>2.12.4</version>
</dependency>
<dependency>
    <groupId>org.springframework.cloud</groupId>
    <artifactId>spring-cloud-starter-open-service-broker</artifactId>
    <version>3.3.0</version>
</dependency>
cowtowncoder commented 3 years ago

I am sorry but I will not add external dependencies for testing. It sounds like external dependencies might even be triggering the issue.

At this point I don't think I can be of further help as things are unfortunately.