Fixed #64

cowtowncoder · cowtowncoder · commit eb6b98a84a48 · 2015-06-26T15:49:07.000-07:00
diff --git a/pom.xml b/pom.xml
@@ -29,7 +29,7 @@ Guava (http://code.google.com/p/guava-libraries/) types (currently mostly just c
   </contributors>
 
   <properties>
-    <version.jackson>2.6.0-rc2</version.jackson>
+    <version.jackson>2.6.0-rc3</version.jackson>
 
     <version.guava>15.0</version.guava>
     <version.guava.osgi>${version.guava}.0</version.guava.osgi>
diff --git a/release-notes/CREDITS b/release-notes/CREDITS
@@ -7,13 +7,16 @@ Tatu Saloranta (tatu.saloranta@iki.fi): author
 Steven Schlansker (steven@nesscomputing.com): co-author
 
 Stephan Schroevers: (Stephan202@github)
-
-* Contributed #56: Add support `HashCode`
- (2.5.0)
-* Contributed #65: Add deserialization support for SortedMultiset and ImmutableSortedMultiset
- (2.5.3)
+  * Contributed #56: Add support `HashCode`
+   (2.5.0)
+  * Contributed #65: Add deserialization support for SortedMultiset and ImmutableSortedMultiset
+   (2.5.3)
 
 Michael Hixson:
+  * Contributed fix for #67: Support deserializing ImmutableSetMultimaps
+   (2.6.0)
+
+Alexey Kobyakov (akobiakov@github)
+  * Reported #64: `@JsonUnwrapped` annotation is ignored when a field is an Optional
+   (2.6.0)
 
-* Contributed fix for #67: Support deserializing ImmutableSetMultimaps
- (2.6.0)
diff --git a/release-notes/VERSION b/release-notes/VERSION
@@ -6,6 +6,8 @@ Project: jackson-datatype-guava
 
 2.6.0 (not yet released)
 
+#64: `@JsonUnwrapped` annotation is ignored when a field is an Optional
+ (reported by Alexey K, akobiakov@github)
 #66: Add `GuavaModule.configureAbsentsAsNulls(boolean)` to change whether
  `Optional.absent()` is to be handled same as Java null during serialization
  (default: true) or not.
diff --git a/src/main/java/com/fasterxml/jackson/datatype/guava/GuavaModule.java b/src/main/java/com/fasterxml/jackson/datatype/guava/GuavaModule.java
@@ -10,7 +10,7 @@
  *<p>
  * Current configurability includes:
  *<ul>
- * <li><code>configureAbsentsAsNulls</code> (default: <code>true</code>):
+ * <li><code>configureAbsentsAsNulls</code> (default: <code>false</code>):
  *    Determines whether inclusion strategy of <code>NON_NULL</code> should additionally consider
  *    <code>Optional.absent()</code> values (as POJO properties) to be excluded; if true, they will
  *     be excluded, if false, they will be included.
diff --git a/src/main/java/com/fasterxml/jackson/datatype/guava/ser/GuavaOptionalSerializer.java b/src/main/java/com/fasterxml/jackson/datatype/guava/ser/GuavaOptionalSerializer.java
@@ -7,8 +7,10 @@
 import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonFormatVisitorWrapper;
 import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
 import com.fasterxml.jackson.databind.ser.ContextualSerializer;
+import com.fasterxml.jackson.databind.ser.impl.PropertySerializerMap;
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 import com.fasterxml.jackson.databind.type.TypeFactory;
+import com.fasterxml.jackson.databind.util.NameTransformer;
 import com.google.common.base.Optional;
 
 @SuppressWarnings("serial")
@@ -17,43 +19,61 @@ public final class GuavaOptionalSerializer
     implements ContextualSerializer
 {
     /**
-     * Declared type for the property being serialized with
-     * this serializer instance.
+     * Declared type parameter for Optional.
      */
-    protected final JavaType _optionalType;
+    protected final JavaType _referredType;
 
+    protected final BeanProperty _property;
+    
     protected final JsonSerializer<Object> _valueSerializer;
 
-    public GuavaOptionalSerializer(JavaType type) {
-        this(type, null);
+    /**
+     * To support unwrapped values of dynamic types, will need this:
+     */
+    protected final NameTransformer _unwrapper;
+
+    /**
+     * If element type can not be statically determined, mapping from
+     * runtime type to serializer is handled using this object
+     *
+     * @since 2.6
+     */
+    protected transient PropertySerializerMap _dynamicSerializers;
+    
+    /*
+    /**********************************************************
+    /* Life-cycle
+    /**********************************************************
+     */
+
+    public GuavaOptionalSerializer(JavaType optionalType) {
+        super(optionalType);
+        _referredType = _valueType(optionalType);
+        _property = null;
+        _valueSerializer = null;
+        _unwrapper = null;
+        _dynamicSerializers = PropertySerializerMap.emptyForProperties();
     }
 
     @SuppressWarnings("unchecked")
-    protected GuavaOptionalSerializer(JavaType type, JsonSerializer<?> valueSer)
+    protected GuavaOptionalSerializer(GuavaOptionalSerializer base,
+            BeanProperty property, JsonSerializer<?> valueSer, NameTransformer unwrapper)
     {
-        super(type);
-        _optionalType = type;
+        super(base);
+        _referredType = base._referredType;
+        _dynamicSerializers = base._dynamicSerializers;
+        _property = property;
         _valueSerializer = (JsonSerializer<Object>) valueSer;
+        _unwrapper = unwrapper;
     }
 
-    protected GuavaOptionalSerializer withResolved(BeanProperty property,
-            JsonSerializer<?> ser)
+    protected GuavaOptionalSerializer withResolved(BeanProperty prop,
+            JsonSerializer<?> ser, NameTransformer unwrapper)
     {
-        if (_valueSerializer == ser) {
+        if ((_property == prop) && (_valueSerializer == ser) && (_unwrapper == unwrapper)) {
             return this;
         }
-        return new GuavaOptionalSerializer(_optionalType, ser);
-    }
-
-    @Override
-    @Deprecated
-    public boolean isEmpty(Optional<?> value) {
-        return isEmpty(null, value);
-    }
-
-    @Override
-    public boolean isEmpty(SerializerProvider prov, Optional<?> value) {
-        return (value == null) || !value.isPresent();
+        return new GuavaOptionalSerializer(this, prop, ser, unwrapper);
     }
 
     @Override
@@ -63,78 +83,145 @@ public JsonSerializer<?> createContextual(SerializerProvider provider,
         JsonSerializer<?> ser = _valueSerializer;
         if (ser == null) {
             // we'll have type parameter available due to GuavaTypeModifier making sure it is, so:
-            JavaType valueType = _valueType();
-            boolean realType = !valueType.hasRawClass(Object.class);
+            boolean realType = !_referredType.hasRawClass(Object.class);
             /* Can only assign serializer statically if the declared type is final,
              * or if we are to use static typing (and type is not "untyped")
              */
             if (realType &&
                     (provider.isEnabled(MapperFeature.USE_STATIC_TYPING)
-                    || valueType.isFinal())) {
+                    || _referredType.isFinal())) {
                 return withResolved(property,
-                        provider.findPrimaryPropertySerializer(valueType, property));
+                        provider.findPrimaryPropertySerializer(_referredType, property),
+                        _unwrapper);
             }
         } else {
             // not sure if/when this should occur but proper way to deal would be:
             return withResolved(property,
-                    provider.handlePrimaryContextualization(ser, property));
+                    provider.handlePrimaryContextualization(ser, property),
+                    _unwrapper);
         }
         return this;
     }
 
     @Override
-    public void serialize(Optional<?> value, JsonGenerator jgen, SerializerProvider provider)
+    public JsonSerializer<Optional<?>> unwrappingSerializer(NameTransformer transformer) {
+        JsonSerializer<Object> ser = _valueSerializer;
+        if (ser != null) {
+            ser = ser.unwrappingSerializer(transformer);
+        }
+        NameTransformer unwrapper = (_unwrapper == null) ? transformer
+                : NameTransformer.chainedTransformer(transformer, _unwrapper);
+        return withResolved(_property, ser, unwrapper);
+    }
+
+    /*
+    /**********************************************************
+    /* API overrides
+    /**********************************************************
+     */
+
+    @Override
+    @Deprecated
+    public boolean isEmpty(Optional<?> value) {
+        return isEmpty(null, value);
+    }
+
+    @Override
+    public boolean isEmpty(SerializerProvider prov, Optional<?> value) {
+        return (value == null) || !value.isPresent();
+    }
+
+    public boolean isUnwrappingSerializer() {
+        return (_unwrapper != null);
+    }
+
+    /*
+    /**********************************************************
+    /* Serialization methods
+    /**********************************************************
+     */
+
+    @Override
+    public void serialize(Optional<?> opt, JsonGenerator gen, SerializerProvider provider)
         throws IOException
     {
-        if (value.isPresent()) {
-            if (_valueSerializer != null) {
-                _valueSerializer.serialize(value.get(), jgen, provider);
-            } else {
-                provider.defaultSerializeValue(value.get(), jgen);
+        if (opt.isPresent()) {
+            Object value = opt.get();
+            JsonSerializer<Object> ser = _valueSerializer;
+            if (ser == null) {
+                ser = _findSerializer(provider, value.getClass());
             }
+            ser.serialize(value, gen, provider);
         } else {
-            provider.defaultSerializeNull(jgen);
+            provider.defaultSerializeNull(gen);
         }
     }
 
     @Override
-    public void serializeWithType(Optional<?> value,
-            JsonGenerator jgen, SerializerProvider provider,
+    public void serializeWithType(Optional<?> opt,
+            JsonGenerator gen, SerializerProvider provider,
             TypeSerializer typeSer) throws IOException
     {
-        if (value.isPresent()) {
+        if (opt.isPresent()) {
+            Object value = opt.get();
             JsonSerializer<Object> ser = _valueSerializer;
             if (ser == null) {
-                // note: could improve by retaining property... needed?
-                ser = provider.findValueSerializer(_valueType(), null);
+                ser = _findSerializer(provider, value.getClass());
             }
-            ser.serializeWithType(value.get(), jgen, provider, typeSer);
+            ser.serializeWithType(value, gen, provider, typeSer);
         } else {
-            provider.defaultSerializeNull(jgen);
+            provider.defaultSerializeNull(gen);
         }
     }
 
+    /*
+    /**********************************************************
+    /* Introspection support
+    /**********************************************************
+     */
+    
     @Override
     public void acceptJsonFormatVisitor(JsonFormatVisitorWrapper visitor, JavaType typeHint) throws JsonMappingException
     {
-        JavaType valueType = _valueType();
-        if (valueType != null) {
-            JsonSerializer<?> ser = _valueSerializer;
-            if (ser == null) {
-                ser = visitor.getProvider().findValueSerializer(valueType, null);
-            }
-            ser.acceptJsonFormatVisitor(visitor, valueType);
-        } else {
-            super.acceptJsonFormatVisitor(visitor, typeHint);
+        JsonSerializer<?> ser = _valueSerializer;
+        if (ser == null) {
+            ser = _findSerializer(visitor.getProvider(), _referredType.getRawClass());
         }
+        ser.acceptJsonFormatVisitor(visitor, _referredType);
     }
 
-    protected JavaType _valueType() {
-        JavaType valueType = _optionalType.containedType(0);
+    /*
+    /**********************************************************
+    /* Misc other
+    /**********************************************************
+     */
+    
+    protected static JavaType _valueType(JavaType optionalType) {
+        JavaType valueType = optionalType.containedType(0);
         if (valueType == null) {
             valueType = TypeFactory.unknownType();
         }
         return valueType;
     }
+
+    /**
+     * Helper method that encapsulates logic of retrieving and caching required
+     * serializer.
+     */
+    protected final JsonSerializer<Object> _findSerializer(SerializerProvider provider, Class<?> type)
+        throws JsonMappingException
+    {
+        PropertySerializerMap.SerializerAndMapResult result = _dynamicSerializers
+                .findAndAddPrimarySerializer(type, provider, _property);
+        if (_dynamicSerializers != result.map) {
+            _dynamicSerializers = result.map;
+        }
+        JsonSerializer<Object> ser = result.serializer;
+        // 26-Jun-2015, tatu: Sub-optimal if we do not cache unwrapped instance; but on plus side
+        //   construction is a cheap operation, so won't add huge overhead
+        if (_unwrapper != null) {
+            ser = ser.unwrappingSerializer(_unwrapper);
+        }
+        return ser;
+    }
 }
-    
diff --git a/src/test/java/com/fasterxml/jackson/datatype/guava/OptionalBasicTest.java b/src/test/java/com/fasterxml/jackson/datatype/guava/OptionalBasicTest.java
@@ -9,7 +9,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Optional;
 
-public class TestOptional extends ModuleTestBase
+public class OptionalBasicTest extends ModuleTestBase
 {
     private final ObjectMapper MAPPER = mapperWithModule();
 
@@ -76,39 +76,39 @@ public void testDeserComplexObject() throws Exception {
         assertTrue(data.get().myString.isPresent());
         assertEquals("simpleString", data.get().myString.get());
     }
-    
+
     public void testDeserGeneric() throws Exception {
         TypeReference<Optional<OptionalGenericData<String>>> type = new TypeReference<Optional<OptionalGenericData<String>>>() {};
         Optional<OptionalGenericData<String>> data = MAPPER.readValue("{\"myData\":\"simpleString\"}", type);
         assertTrue(data.isPresent());
         assertTrue(data.get().myData.isPresent());
         assertEquals("simpleString", data.get().myData.get());
     }
-    
+
     public void testSerAbsent() throws Exception {
         String value = MAPPER.writeValueAsString(Optional.absent());
         assertEquals("null", value);
     }
-    
+
     public void testSerSimpleString() throws Exception {
         String value = MAPPER.writeValueAsString(Optional.of("simpleString"));
         assertEquals("\"simpleString\"", value);
     }
-    
+
     public void testSerInsideObject() throws Exception {
         OptionalData data = new OptionalData();
         data.myString = Optional.of("simpleString");
         String value = MAPPER.writeValueAsString(data);
         assertEquals("{\"myString\":\"simpleString\"}", value);
     }
-    
+
     public void testSerComplexObject() throws Exception {
         OptionalData data = new OptionalData();
         data.myString = Optional.of("simpleString");
         String value = MAPPER.writeValueAsString(Optional.of(data));
         assertEquals("{\"myString\":\"simpleString\"}", value);
     }
-    
+
     public void testSerGeneric() throws Exception {
         OptionalGenericData<String> data = new OptionalGenericData<String>();
         data.myData = Optional.of("simpleString");
@@ -198,7 +198,7 @@ public void testObjectId() throws Exception
         final Unit input = new Unit();
         input.link(input);
         String json = MAPPER.writeValueAsString(input);
-        Unit result = MAPPER.readValue(json,  Unit.class);
+        Unit result = MAPPER.readValue(json, Unit.class);
         assertNotNull(result);
         assertNotNull(result.baseUnit);
         assertTrue(result.baseUnit.isPresent());
diff --git a/src/test/java/com/fasterxml/jackson/datatype/guava/OptionalUnwrappedTest.java b/src/test/java/com/fasterxml/jackson/datatype/guava/OptionalUnwrappedTest.java
