Does Jackson behave correctly for final variables initialized during declaration?

user_12939222 0
Viewed 978

Considering the following (failing) JUnit test for Jackson 2.9.5:

package de.azamir.test;

import static org.junit.Assert.assertEquals;

import java.io.IOException;

import org.junit.Test;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;

public class JacksonFinalTest {

    private static final class MyClass {

        @JsonProperty("myProperty")
        public final String myProperty = "myPropertyValueInit";
    }

    @Test
    public void doTest() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException, JsonParseException, JsonMappingException, IOException {

        final MyClass deserializedMyClass1 = new ObjectMapper().readValue("{\"myProperty\":\"myPropertyValueDeserialized\"}", MyClass.class);

        // "myPropertyValueInit"
        final String directValue = deserializedMyClass1.myProperty;

        // "myPropertyValueDeserialized"
        final String reflectValue = (String) MyClass.class.getDeclaredField("myProperty").get(deserializedMyClass1);

        assertEquals(directValue, reflectValue);

    }
}

I suspect the Java VM to optimize accesses to final fields such that the value written by Jackson (via reflection) can only be retrieved again via reflection. This just today lead to a very subtle and hard to find bug in our codebase.

Is this a problem that has been addressed in one way or another by Jackson? My gut feeling would be that there is no way for Jackson code do "see" this but who knows.

I know there are multiple ways of improving this code such as

  • removing the @JsonProperty and disabling final fields as mutators
  • not initializing the field during declaration but in a constructor
  • not making the fields final at all

but is there a recommended way of making sure that no developer in our team (who might not know the caveat) does this?

jackson java
2 Answers

So there is a way: (idea from this answer, also nice to read is this question) Instead of using the constant expression and directly assign the variable at declaration site, init your final variables in your constructor. See the changes when you use MyClass2:

public class JacksonFinalTest {

    private static final class MyClass {

        @JsonProperty("myProperty")
        public final String myProperty = "myPropertyValueInit";
    }

    private static final class MyClass2 {

        @JsonProperty("myProperty")
        public final String myProperty;

        public MyClass2() {
            myProperty = "myPropertyValueInit";
        }
    }

    @Test
    public void doTest() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException, JsonParseException, JsonMappingException, IOException {

        final String json = "{\"myProperty\":\"myPropertyValueDeserialized\"}";
        final MyClass2 deserializedMyClass1 = new ObjectMapper().readValue(json, MyClass2.class);

        // "myPropertyValueInit"
        final String directValue = deserializedMyClass1.myProperty;

        // "myPropertyValueDeserialized"
        final String reflectValue = (String) MyClass2.class.getDeclaredField("myProperty").get(deserializedMyClass1);

        assertEquals(directValue, reflectValue);

    }
}
user_64733980

I just found another solution that causes a RuntimeException. You can check the field during deserialization using a BeanDeserializationModifier (inspired by this answer):

    @Test
    public void doTest() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException, JsonParseException, JsonMappingException, IOException {

        final BeanDeserializerModifier modifier = new BeanDeserializerModifier() {

            @Override
            public List<BeanPropertyDefinition> updateProperties(final DeserializationConfig config, final BeanDescription beanDesc, final List<BeanPropertyDefinition> propDefs) {
                propDefs.stream()
                        .filter(pd -> pd.hasField())
                        .forEach(propDef -> {
                            final AnnotatedField f = propDef.getField();
                            if (Modifier.isFinal(f.getModifiers())) {
                                throw new RuntimeException("Bad final field " + f.getFullName());
                            }
                        });

                return super.updateProperties(config, beanDesc, propDefs);
            }

        };
        final DeserializerFactory dFactory = BeanDeserializerFactory.instance.withDeserializerModifier(modifier);
        final ObjectMapper mapper = new ObjectMapper(null, null, new DefaultDeserializationContext.Impl(dFactory));

        final ObjectMapper objectMapper = mapper;
        final String json = "{\"myProperty\":\"myPropertyValueDeserialized\"}";
        final MyClass deserializedMyClass1 = objectMapper.readValue(json, MyClass.class);

        // "myPropertyValueInit"
        final String directValue = deserializedMyClass1.myProperty;

        // "myPropertyValueDeserialized"
        final String reflectValue = (String) MyClass.class.getDeclaredField("myProperty").get(deserializedMyClass1);

        assertEquals(directValue, reflectValue);

    }
user_64733980
Related