Fix #1382

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -5,6 +5,8 @@ Project: jackson-databind
 
 2.9.4 (not yet released)
 
+#1382: `@JsonProperty(access=READ_ONLY)` unxepected behaviour with `Collections`
+ (reported by hexfaker@github)
 #1673: Serialising generic value classes via Reference Types (like Optional) fails
   to include type information
  (reported by Pier-Luc W)

src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java

@@ -327,6 +327,13 @@ public JsonPOJOBuilder.Value getBuilderConfig() {
         return _builderConfig;
     }
 
+    /**
+     * @since 2.9.4
+     */
+    public boolean hasIgnorable(String name) {
+        return (_ignorableProps != null) && _ignorableProps.contains(name);
+    }
+
     /*
     /**********************************************************
     /* Build method(s)

src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java

@@ -482,6 +482,8 @@ protected void addBeanProps(DeserializationContext ctxt,
         if (anySetter != null) {
             builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetter));
         } else {
+            // 23-Jan-2018, tatu: although [databind#1805] would suggest we should block
+            //   properties regardless, for now only consider unless there's any setter...
             Collection<String> ignored2 = beanDesc.getIgnoredPropertyNames();
             if (ignored2 != null) {
                 for (String propName : ignored2) {
@@ -509,10 +511,9 @@ protected void addBeanProps(DeserializationContext ctxt,
         for (BeanPropertyDefinition propDef : propDefs) {
             SettableBeanProperty prop = null;
 
-            /* 18-Oct-2013, tatu: Although constructor parameters have highest precedence,
-             *   we need to do linkage (as per [databind#318]), and so need to start with
-             *   other types, and only then create constructor parameter, if any.
-             */
+            // 18-Oct-2013, tatu: Although constructor parameters have highest precedence,
+            //   we need to do linkage (as per [databind#318]), and so need to start with
+            //   other types, and only then create constructor parameter, if any.
             if (propDef.hasSetter()) {
                 AnnotatedMethod setter = propDef.getSetter();
                 JavaType propertyType = setter.getParameterType(0);
@@ -526,7 +527,13 @@ protected void addBeanProps(DeserializationContext ctxt,
                 AnnotatedMethod getter = propDef.getGetter();
                 if (getter != null) {
                     if (useGettersAsSetters && _isSetterlessType(getter.getRawType())) {
-                        prop = constructSetterlessProperty(ctxt, beanDesc, propDef);
+                        // 23-Jan-2018, tatu: As per [databind#1805], need to ensure we don't
+                        //   accidentally sneak in getter-as-setter for `READ_ONLY` properties
+                        if (builder.hasIgnorable(propDef.getName())) {
+                            ;
+                        } else {
+                            prop = constructSetterlessProperty(ctxt, beanDesc, propDef);
+                        }
                     } else if (!propDef.hasConstructorParameter()) {
                         PropertyMetadata md = propDef.getMetadata();
                         // 25-Oct-2016, tatu: If merging enabled, might not need setter.

src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -721,7 +721,7 @@ protected void _removeUnwantedProperties(Map<String, POJOPropertyBuilder> props)
                 }
                 // otherwise just remove ones marked to be ignored
                 prop.removeIgnored();
-                if (!_forSerialization && !prop.couldDeserialize()) {
+                if (!prop.couldDeserialize()) {
                     _collectIgnorals(prop.getName());
                 }
             }
@@ -742,7 +742,7 @@ protected void _removeUnwantedAccessor(Map<String, POJOPropertyBuilder> props)
             POJOPropertyBuilder prop = it.next();
             // 26-Jan-2017, tatu: [databind#935]: need to denote removal of
             JsonProperty.Access acc = prop.removeNonVisible(inferMutators);
-            if (!_forSerialization && (acc == JsonProperty.Access.READ_ONLY)) {
+            if (acc == JsonProperty.Access.READ_ONLY) {
                 _collectIgnorals(prop.getName());
             }
         }

src/test/java/com/fasterxml/jackson/databind/deser/AnySetterTest.java

@@ -367,7 +367,7 @@ public void testGenericAnySetter() throws Exception
         assertEquals(integerGeneric.getDynamicallyMappedProperties(), integerGenericMap);
     }
 
-	/*
+    /*
     /**********************************************************
     /* Private helper methods
     /**********************************************************

src/test/java/com/fasterxml/jackson/failing/ReadOnlyDeser1382Test.java

@@ -2,6 +2,7 @@
 
 import java.util.*;
 
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.*;
 
@@ -21,11 +22,57 @@ public Foo setList(List<Long> list) {
         }
     }
 
-    public void testReadOnly() throws Exception
+    // [databind#1805]
+    static class UserWithReadOnly {
+        public String name;
+
+        @JsonProperty(access = JsonProperty.Access.READ_ONLY)
+        public List<String> getRoles() {
+            return Arrays.asList("admin", "monitor");
+        }
+    }
+
+    // [databind#1805]
+    @JsonIgnoreProperties(value={ "roles" }, allowGetters=true)
+    static class UserAllowGetters {
+        public String name;
+
+        public List<String> getRoles() {
+            return Arrays.asList("admin", "monitor");
+        }
+    }
+
+    /*
+    /**********************************************************
+    /* Test methods
+    /**********************************************************
+     */
+
+    private final ObjectMapper MAPPER = newObjectMapper();
+
+    public void testReadOnly1382() throws Exception
     {
-        final ObjectMapper mapper = new ObjectMapper();
         String payload = "{\"list\":[1,2,3,4]}";
-        Foo foo = mapper.readValue(payload, Foo.class);
+        Foo foo = MAPPER.readValue(payload, Foo.class);
         assertTrue("List should be empty", foo.getList().isEmpty());
     }
+
+    // [databind#1805]
+    public void testViaReadOnly() throws Exception {
+        UserWithReadOnly user = new UserWithReadOnly();
+        user.name = "foo";
+        String json = MAPPER.writeValueAsString(user);
+        UserWithReadOnly result = MAPPER.readValue(json, UserWithReadOnly.class);
+        assertNotNull(result);
+    }
+
+    // [databind#1805]
+    public void testUsingAllowGetters() throws Exception {
+        UserAllowGetters user = new UserAllowGetters();
+        user.name = "foo";
+        String json = MAPPER.writeValueAsString(user);
+        assertTrue(json.contains("roles"));
+        UserAllowGetters result = MAPPER.readValue(json, UserAllowGetters.class);
+        assertNotNull(result);
+    }
 }