Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON properties missing from REST unmarshalling #1952

Closed
nvafiades opened this issue Mar 1, 2018 · 10 comments
Closed

JSON properties missing from REST unmarshalling #1952

nvafiades opened this issue Mar 1, 2018 · 10 comments

Comments

@nvafiades
Copy link

nvafiades commented Mar 1, 2018

We have a large web application that uses Jackson annotated pojos to marshal and unmarshal data on REST routes. After upgrading from 2.8.11 to 2.9.4, several of our REST routes are no longer working. There is either data missing from GETs or not getting processed from POSTs/PUTs. I have reproduced this with a test class highlighted below, demonstrating a simple scenario with an IS getter method vs a GET method, but there are other cases as well. This is preventing us from upgrading to 2.9.

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.container.AsyncResponse;
import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

import org.springframework.stereotype.Controller;

import com.fasterxml.jackson.annotation.JsonAutoDetect;

@Controller
@Path("/diagnostics")
public class JacksonTestController
{
    public JacksonTestController()
    {
    }

    @JsonAutoDetect(setterVisibility = JsonAutoDetect.Visibility.NONE, getterVisibility = JsonAutoDetect.Visibility.ANY)
    public class JacksonTestPOJO
    {
        public JacksonTestPOJO()
        {

        }

        private boolean booleanProperty1;
        private boolean booleanProperty2;

        public void setBooleanProperty1(boolean value)
        {
            this.booleanProperty1 = value;
        }

        public boolean isBooleanProperty1()
        {
            return booleanProperty1;
        }

        public void setBooleanProperty2(boolean value)
        {
            this.booleanProperty2 = value;
        }

        public boolean getBooleanProperty2()
        {
            return booleanProperty2;
        }
    }

    @GET
    @Path("/jacksonTest")
    @Produces(MediaType.APPLICATION_JSON)
    public void get(@Suspended final AsyncResponse asyncResponse)
    {
        JacksonTestPOJO view = new JacksonTestPOJO();

        view.setBooleanProperty1(true);
        view.setBooleanProperty2(true);

        asyncResponse.resume(Response.ok(view).build());
    }

    // Jackson 2.8.11 returns:
    // {
    // "booleanProperty1" : true
    // "booleanProperty2" : true
    // }

    // Jackson 2.9.4 returns:
    // {
    // "booleanProperty2" : true
    // }
}

@cowtowncoder
Copy link
Member

Test classes help, but what is needed is stand-alone test without using framework (JAX-RS via Spring MVC or Boot?).

@nvafiades
Copy link
Author

@cowtowncoder I am trying to reproduce outside the framework, but the challenge is we don't understand the root cause, so how to configure the test to reproduce it is a challenge. In posting this, I'd hoped a change would stand-out as the potential cause and give us some direction in configuring or adjust our implementation at a global level so we don't have to fix every JSON annotation.

If you have any suggestions or documentation for implementing a standalone test case, that would be helpful.

@cowtowncoder
Copy link
Member

Ok. So, if I understand you correctly, you are saying that constructing mapper, giving instance, does NOT exhibit the issue unless it goes through framework?

There is one upcoming fix for 2.9.5:

#1947

that could be related, although probably not (since your settings are different).
But other than that code looks good, and missing property wrong... I don't have any real guesses as to what might be going. 2.9.4 is new enough version that there are no known concurrency issues, and there is no real hierarchy to consider either (for rare problems earlier versions like 2.7 had).

So unfortunately I do not have many suggestions. If you can build 2.9.5-SNAPSHOT of databind (2.9.4 fine for other components), you can perhaps rule out issue I mention above.

@nadiramra
Copy link

Hi, I do not know if this is related but I we are seeing something as well. I have (maybe?) narrowed it down in how Jackson seems to generating identifiers from Java getter/setter methods. We are running jackson 2.9.1 (prior to that we were on 2.6).

In the past, if we have a property that is all capitalized, such as:

String SKU;

String getSKU() { ... }

void setSKU() { .. }

without any JASON annotations whatsoever, the JSON identifier was "SKU". But now, with the version 2.9.1, it seems it somehow expects the identifier to be "sku". In fact, even if the property was SKUData, jackson seems to want skudata. In either case, it fails when doing a REST request.

So wondering if this is a defect and whether I am in the right area?

@cowtowncoder
Copy link
Member

@nadiramra There is one relevant setting:

MapperFeature.USE_STD_BEAN_NAMING

which sounds relevant; however, default setting for that is false and that has not change between 2.6 and 2.9.

However. As I have suggested earlier, it is possible that when Jackson is used through framework (such as Spring) that default settings could vary. If so, baseline settings changing for this feature (for example) would change behavior.

Also: many users by default disable

DeserializationFeature. FAIL_ON_UNKNOWN_PROPERTIES

which is REALLY nasty thing to do wrt testing because it will now effectively hide any JSON properties that fail to match POJO properties. This is why this feature defaults to false: although it may make sense for some use cases, it can hide many types of otherwise obvious problems.

So if possible it is good to enable that feature when troubleshooting.

@nadiramra
Copy link

nadiramra commented Jul 24, 2018

Thanks @cowtowncoder , this helps me understand what is going on. I do have one question, I was mistaken about 2.6, it seems it was 2.4. It seems that MapperFeature.USE_STD_BEAN_NAMING did not exist in the version, am I correct? If it did not, did 2.4 do standard bean naming processing?

Also, curious to know if the answer to above is yes, why MapperFeature.USE_STD_BEAN_NAMING was ever introduced. And if the answer is no, how did one make it do standard bean naming in 2.4?

Thanks in advance.

@nvafiades
Copy link
Author

nvafiades commented Sep 25, 2018

I have managed to write an isolated test class where this is reproducible, demonstrating the change from 2.8 to 2.9. It would be helpful to understand why the change in behaviour has occurred and what the appropriate configuration change might be to correct it.

In the following test class, the final assert is valid for 2.8, but fails in 2.9.

package com.rim.platform.mdm;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.text.SimpleDateFormat;

import javax.ws.rs.core.Response;

import org.testng.Assert;
import org.testng.annotations.Test;

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonEncoding;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;
import com.fasterxml.jackson.databind.SerializationConfig;
import com.fasterxml.jackson.databind.SerializationFeature;

public class JacksonUpgradeTest
{
    @JsonAutoDetect(setterVisibility = JsonAutoDetect.Visibility.NONE, getterVisibility = JsonAutoDetect.Visibility.ANY)
    private class JacksonUpdatePOJO
    {
        private boolean booleanProperty;

        public void setBooleanProperty(boolean booleanProperty)
        {
            this.booleanProperty = booleanProperty;
        }

        public boolean isBooleanProperty()
        {
            return booleanProperty;
        }
    }

    private static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSSZ";

    @Test
    public void test() throws IOException
    {
        JacksonUpdatePOJO view = new JacksonUpdatePOJO();

        view.setBooleanProperty(true);

        ObjectMapper mapper = new ObjectMapper();

        String jsonInString = mapper.writeValueAsString(view);

        Assert.assertEquals(jsonInString, "{\"booleanProperty\":true}");

        Response response = Response.ok(view).build();

        ObjectMapper objectMapper = new ObjectMapper();

        SerializationConfig cfg = objectMapper.getSerializationConfig();
        cfg =
                cfg.with(new SimpleDateFormat(DATE_FORMAT))
                        .with(SerializationFeature.INDENT_OUTPUT)
                        .without(MapperFeature.AUTO_DETECT_FIELDS, MapperFeature.AUTO_DETECT_GETTERS,
                                MapperFeature.AUTO_DETECT_IS_GETTERS)
                        .without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
                        .withPropertyInclusion(
                                JsonInclude.Value.construct(JsonInclude.Include.NON_NULL, JsonInclude.Include.NON_NULL));

        objectMapper.setConfig(cfg);

        ObjectWriter writer = objectMapper.writer();

        ByteArrayOutputStream entityStream = new ByteArrayOutputStream();

        JsonGenerator jg = writer.getFactory().createGenerator(entityStream, JsonEncoding.UTF8);
        writer.writeValue(jg, response.getEntity());

        String responseBody = new String(entityStream.toByteArray());

        Assert.assertTrue(responseBody.contains("\"booleanProperty\" : true"),
                "Response body missing booleanProperty, body contents: ** " + responseBody + " ** ");
    }
}

@nvafiades
Copy link
Author

I've attached an updated test class that does not use ResponseBuilder and added another scenario. With just "@JsonAutoDetect" annotation and nothing else, the behaviour is different between 2.8 and 2.9.

JacksonUpgradeTest.zip

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 25, 2018

From above example, I think your problem is that you are essentially disabling "is-getter" visibility, as part of:

    .without(MapperFeature.AUTO_DETECT_FIELDS,
         MapperFeature.AUTO_DETECT_GETTERS,
         MapperFeature.AUTO_DETECT_IS_GETTERS)

but only enabling regular getters with:

@JsonAutoDetect(setterVisibility = JsonAutoDetect.Visibility.NONE,
    getterVisibility = JsonAutoDetect.Visibility.ANY)

so you may want to include isGetterVisibility = JsonAutoDetect.Visibility.ANY to enable discovery of isBooleanProperty.

As to why this was not the case with 2.8 I assume it was a flaw in handling. It may have been addressed as part of #1347.

@cowtowncoder
Copy link
Member

Not sure if there is anything remaining here: if there are issues, reproducible with 2.12, please file a new issue (with a ref to this issue); closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants