You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/07/28 12:11:14 UTC

[brooklyn-server] 02/02: better type inference in jackson deserialization

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 5a75cfbd3da9d073eac370766b1d328ce2538e18
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Jul 28 13:10:03 2022 +0100

    better type inference in jackson deserialization
    
    always using @type if specified, in case where expected type is vague but actual type has a type field.
    also suppress nulls when WrappedValue is not collapsed.
---
 .../resolve/jackson/AsPropertyIfAmbiguous.java     | 108 ++++++++++++++++++---
 .../core/resolve/jackson/WrappedValue.java         |   2 +
 2 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
index 84d84a393b..fc9b041a80 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java
@@ -37,6 +37,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.Reflections;
 import org.apache.brooklyn.util.text.Strings;
+import org.apache.commons.lang3.tuple.Pair;
 
 import java.io.IOException;
 import java.lang.reflect.AccessibleObject;
@@ -136,27 +137,106 @@ public class AsPropertyIfAmbiguous {
 
         @Override
         public Object deserializeTypedFromObject(JsonParser p, DeserializationContext ctxt) throws IOException {
+            AsPropertyButNotIfFieldConflictTypeDeserializer target = this;
+            boolean mustUseConflictingTypePrefix = false;
+
             if (_idResolver instanceof HasBaseType) {
-                if (// object has field with same name as the type property - don't treat the type property supplied here as the type
-                        presentAndNotJsonIgnored(Reflections.findFieldMaybe(((HasBaseType) _idResolver).getBaseType().getRawClass(), _typePropertyName))
-                                || // or object has getter with same name as the type property
-                                presentAndNotJsonIgnored(Reflections.findMethodMaybe(((HasBaseType) _idResolver).getBaseType().getRawClass(), "get" + Strings.toInitialCapOnly(_typePropertyName)))
-                ) {
-                    // look for an '@' type
-                    return cloneWithNewTypePropertyName(CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName).deserializeTypedFromObject(p, ctxt);
-
-                    // previous behaviour:
+                JavaType baseType = ((HasBaseType) _idResolver).getBaseType();
+                if (baseType != null ) {
+                    if (// object has field with same name as the type property - don't treat the type property supplied here as the type
+                            presentAndNotJsonIgnored(Reflections.findFieldMaybe(baseType.getRawClass(), _typePropertyName))
+                                    || // or object has getter with same name as the type property
+                                    presentAndNotJsonIgnored(Reflections.findMethodMaybe(baseType.getRawClass(), "get" + Strings.toInitialCapOnly(_typePropertyName)))
+                    ) {
+                        // look for an '@' type
+//                    return cloneWithNewTypePropertyName(CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName).deserializeTypedFromObject(p, ctxt);
+                        // now we always look for @ first, in case the type is not known but that field is present; but if we know 'type' is a bean field, don't allow it to be used
+                        mustUseConflictingTypePrefix = true;
+
+                        // previous behaviour:
 //                    // don't read type id, just deserialize
 //                    JsonDeserializer<Object> deser = ctxt.findContextualValueDeserializer(((HasBaseType)_idResolver).getBaseType(), _property);
 //                    return deser.deserialize(p, ctxt);
+                    }
+
+                    // ? - MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL should do this
+                    if (!Objects.equals(_defaultImpl, baseType)) {
+                        // note: needed even if baseType is object
+                        target = new AsPropertyButNotIfFieldConflictTypeDeserializer(_baseType, _idResolver, _typePropertyName, _typeIdVisible, ((HasBaseType) _idResolver).getBaseType(), _inclusion);
+                    }
+                }
+            }
+            return target.deserializeTypedFromObjectSuper(p, ctxt, mustUseConflictingTypePrefix);
+        }
+
+        // copied from super class
+        private Object deserializeTypedFromObjectSuper(JsonParser p, DeserializationContext ctxt, boolean mustUseConflictingTypePrefix) throws IOException {
+//            return super.deserializeTypedFromObject(p, ctxt);
+
+            // 02-Aug-2013, tatu: May need to use native type ids
+            Object typeId;
+            if (p.canReadTypeId()) {
+                typeId = p.getTypeId();
+                if (typeId != null) {
+                    return _deserializeWithNativeTypeId(p, ctxt, typeId);
+                }
+            }
+
+            // but first, sanity check to ensure we have START_OBJECT or FIELD_NAME
+            JsonToken t = p.currentToken();
+            if (t == JsonToken.START_OBJECT) {
+                t = p.nextToken();
+            } else if (/*t == JsonToken.START_ARRAY ||*/ t != JsonToken.FIELD_NAME) {
+                /* This is most likely due to the fact that not all Java types are
+                 * serialized as JSON Objects; so if "as-property" inclusion is requested,
+                 * serialization of things like Lists must be instead handled as if
+                 * "as-wrapper-array" was requested.
+                 * But this can also be due to some custom handling: so, if "defaultImpl"
+                 * is defined, it will be asked to handle this case.
+                 */
+                return _deserializeTypedUsingDefaultImpl(p, ctxt, null, _msgForMissingId);
+            }
+            // Ok, let's try to find the property. But first, need token buffer...
+            TokenBuffer tb = null;
+            boolean ignoreCase = ctxt.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
+
+            // changed to look for conflicting property first
+            Pair<String, TokenBuffer> typeIdFindResult = findTypeIdOrUnambiguous(p, ctxt, t, tb, ignoreCase, mustUseConflictingTypePrefix);
+            tb = typeIdFindResult.getRight();
+//            if (typeIdFindResult.getLeft()==null && !mustUseConflictingTypePrefix) {
+//                p = tb.asParserOnFirstToken();
+//                tb = null;
+//                typeIdFindResult = findTypeId(p, ctxt, t, tb, ignoreCase, _typePropertyName);
+//            }
+//            tb = typeIdFindResult.getRight();
+
+            if (typeIdFindResult.getLeft()!=null) return _deserializeTypedForId(p, ctxt, tb, typeIdFindResult.getLeft());
+            else return _deserializeTypedUsingDefaultImpl(p, ctxt, tb, _msgForMissingId);
+        }
+
+        private Pair<String,TokenBuffer> findTypeIdOrUnambiguous(JsonParser p, DeserializationContext ctxt, JsonToken t, TokenBuffer tb, boolean ignoreCase, boolean mustUseConflictingTypePrefix) throws IOException {
+            String typeUnambiguous = CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName;
+
+            for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) {
+                final String name = p.currentName();
+                p.nextToken(); // to point to the value
+
+                // require unambiguous one is first if present; otherwise maintaining the parser and token buffer in the desired states is too hard
+                if (name.equals(typeUnambiguous) || (!mustUseConflictingTypePrefix && (name.equals(_typePropertyName)
+                        || (ignoreCase && name.equalsIgnoreCase(_typePropertyName))))) { // gotcha!
+                    // 09-Sep-2021, tatu: [databind#3271]: Avoid converting null to "null"
+                    String typeId = p.getValueAsString();
+                    if (typeId != null) {
+                        return Pair.of(typeId, tb);
+                    }
                 }
-                // ? - MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL should do this
-                if (!Objects.equals(_defaultImpl, ((HasBaseType) _idResolver).getBaseType())) {
-                    AsPropertyButNotIfFieldConflictTypeDeserializer delegate = new AsPropertyButNotIfFieldConflictTypeDeserializer(_baseType, _idResolver, _typePropertyName, _typeIdVisible, ((HasBaseType) _idResolver).getBaseType(), _inclusion);
-                    return delegate.deserializeTypedFromObject(p, ctxt);
+                if (tb == null) {
+                    tb = ctxt.bufferForInputBuffering(p);
                 }
+                tb.writeFieldName(name);
+                tb.copyCurrentStructure(p);
             }
-            return super.deserializeTypedFromObject(p, ctxt);
+            return Pair.of(null, tb);
         }
 
         private boolean presentAndNotJsonIgnored(Maybe<? extends AccessibleObject> fm) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValue.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValue.java
index 2da334be65..79b986615c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValue.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValue.java
@@ -18,6 +18,7 @@
  */
 package org.apache.brooklyn.core.resolve.jackson;
 
+import com.fasterxml.jackson.annotation.JsonInclude;
 import com.google.common.base.Preconditions;
 import java.util.Objects;
 import java.util.function.Supplier;
@@ -37,6 +38,7 @@ import org.apache.brooklyn.util.core.task.DeferredSupplier;
  *
  * When deserialized, this will parse Brooklyn DSL expressions.
  */
+@JsonInclude(JsonInclude.Include.NON_NULL)
 public class WrappedValue<T> implements Supplier<T>, com.google.common.base.Supplier<T>, DeferredSupplier<T> {
     final static WrappedValue<?> NULL_WRAPPED_VALUE = new WrappedValue<>(null, false);
     final T value;