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 8b327a27bd2a0529edb79776a5c18b768ed5d37c Author: Alex Heneveld <[email protected]> AuthorDate: Thu Dec 1 11:14:12 2022 +0000 introduce and prefer `(type)` as the key to use in maps where `type` is ambiguous previously we used `@type` as the key if the object had a field `type`. this is still supported but so is `(type)` now with the latter preferred. the reason for this is that `(type): Class` is allowed in yaml whereas if a key starts `@` it is necessary to quote it, eg `"@type": Class`. we also add support for `(type)` in several places, BeanWithType and CAMP and JavaClassName transformers, so we can use it better in registered types. introduce new test which failed before and passes with the changes. --- .../BrooklynComponentTemplateResolver.java | 3 +- .../creation/BrooklynEntityDecorationResolver.java | 4 +- .../spi/creation/BrooklynYamlTypeInstantiator.java | 8 ++-- .../brooklyn/spi/creation/CampInternalUtils.java | 17 +++++--- .../spi/creation/CampTypePlanTransformer.java | 3 +- .../camp/brooklyn/CustomTypeConfigYamlTest.java | 22 +++++++++++ .../catalog/internal/BasicBrooklynCatalog.java | 6 +-- .../resolve/jackson/AsPropertyIfAmbiguous.java | 21 ++++++---- .../jackson/BeanWithTypePlanTransformer.java | 19 ++++++--- .../typereg/JavaClassNameTypePlanTransformer.java | 8 +++- ...klynRegisteredTypeJacksonSerializationTest.java | 46 +++++++++++++++++++++- .../resolve/jackson/PerverseSerializationTest.java | 3 +- 12 files changed, 128 insertions(+), 32 deletions(-) diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index 932d823d70..bd6d1c9334 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -64,6 +64,7 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils; import org.apache.brooklyn.core.mgmt.ManagementContextInjectable; import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext; import org.apache.brooklyn.core.resolve.entity.EntitySpecResolver; +import org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous; import org.apache.brooklyn.core.typereg.AbstractTypePlanTransformer; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; import org.apache.brooklyn.util.collections.MutableList; @@ -604,7 +605,7 @@ public class BrooklynComponentTemplateResolver { Exception exceptionToInclude; // heuristic - if (resolvedConfig.containsKey("type")) { + if (resolvedConfig.containsKey(CampInternalUtils.TYPE_SIMPLE_KEY) || resolvedConfig.containsKey(CampInternalUtils.TYPE_UNAMBIGUOUS_KEY)) { // if it has a key 'type' then it is likely a CAMP entity, abbreviated syntax (giving a type), so just give e1 exceptionToInclude = e1; } else if (resolvedConfig.containsKey("brooklyn.services")) { diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java index 438e9f40d4..7da9d97840 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java @@ -44,6 +44,7 @@ import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynYamlTypeInstantiat import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.mgmt.BrooklynTags; import org.apache.brooklyn.core.objs.BasicSpecParameter; +import org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; import org.apache.brooklyn.core.typereg.AbstractTypePlanTransformer; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; @@ -208,7 +209,8 @@ public abstract class BrooklynEntityDecorationResolver<DT> { } catch (Exception e) { Exceptions.propagateIfFatal(e); // fall back to the old way, eg if caller specifies initializerType, or for some other reason bean-with-type fails - Object type = decorationJson.get("type"); + Object type = decorationJson.get(CampInternalUtils.TYPE_UNAMBIGUOUS_KEY); + if (type==null) type = decorationJson.get(CampInternalUtils.TYPE_SIMPLE_KEY); try { result = instantiator.from(decorationJson).prefix(typeKeyPrefix).newInstance(EntityInitializer.class); if (type!=null) { diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java index 57106d6987..67f7d5fc10 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java @@ -32,6 +32,7 @@ import org.apache.brooklyn.api.objs.Configurable; import org.apache.brooklyn.camp.brooklyn.BrooklynCampReservedKeys; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal; +import org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.exceptions.Exceptions; @@ -105,11 +106,12 @@ public abstract class BrooklynYamlTypeInstantiator { if (result.isAbsent() && typeKeyPrefix!=null) { // try alternatives if a prefix was specified result = data.getStringKeyMaybe(typeKeyPrefix+"Type"); - if (result.isAbsent()) result = data.getStringKeyMaybe("type"); + if (result.isAbsent()) result = data.getStringKeyMaybe(CampInternalUtils.TYPE_UNAMBIGUOUS_KEY); + if (result.isAbsent()) result = data.getStringKeyMaybe(CampInternalUtils.TYPE_SIMPLE_KEY); } if (result.isAbsent() || result.get()==null) - return Maybe.absent("Missing key 'type'"+(typeKeyPrefix!=null ? " (or '"+getTypedKeyName()+"')" : "")); + return Maybe.absent("Missing key '"+CampInternalUtils.TYPE_SIMPLE_KEY+"'"+(typeKeyPrefix!=null ? " (or '"+getTypedKeyName()+"')" : "")); if (result.get() instanceof String) return Maybe.of((String)result.get()); @@ -119,7 +121,7 @@ public abstract class BrooklynYamlTypeInstantiator { protected String getTypedKeyName() { if (typeKeyPrefix!=null) return typeKeyPrefix+"_type"; - return "type"; + return CampInternalUtils.TYPE_SIMPLE_KEY; } /** as {@link #newInstance(Class)} but inferring the type */ diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java index cbfac1ca7e..e2423967ef 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java @@ -52,6 +52,8 @@ import org.apache.brooklyn.camp.spi.pdp.DeploymentPlan; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog.BrooklynLoaderTracker; import org.apache.brooklyn.core.objs.BasicSpecParameter; +import org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous; +import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; import org.apache.brooklyn.core.typereg.BundleUpgradeParser.CatalogUpgrades; import org.apache.brooklyn.entity.stock.BasicApplicationImpl; @@ -69,6 +71,9 @@ import com.google.common.collect.Iterables; /** package-private; as {@link RegisteredType} becomes standard hopefully we can remove this */ class CampInternalUtils { + final static String TYPE_SIMPLE_KEY = BeanWithTypePlanTransformer.TYPE_SIMPLE_KEY; + final static String TYPE_UNAMBIGUOUS_KEY = BeanWithTypePlanTransformer.TYPE_UNAMBIGUOUS_KEY; + static EntitySpec<? extends Application> createWrapperApp(AssemblyTemplate template, BrooklynClassLoadingContext loader) { BrooklynComponentTemplateResolver resolver = BrooklynComponentTemplateResolver.Factory.newInstance(loader, buildWrapperAppTemplate(template)); EntitySpec<Application> wrapperSpec = resolver.resolveSpec(ImmutableSet.<String>of()); @@ -134,14 +139,14 @@ class CampInternalUtils { static PolicySpec<?> createPolicySpec(BrooklynClassLoadingContext loader, Object policy, Set<String> encounteredCatalogTypes) { Map<String, Object> itemMap; if (policy instanceof String) { - itemMap = ImmutableMap.<String, Object>of("type", policy); + itemMap = ImmutableMap.<String, Object>of(TYPE_SIMPLE_KEY, policy); } else if (policy instanceof Map) { itemMap = (Map<String, Object>) policy; } else { throw new IllegalStateException("Policy expected to be string or map. Unsupported object type " + policy.getClass().getName() + " (" + policy.toString() + ")"); } - String versionedId = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "policy_type", "policyType", "type"), "policy type"); + String versionedId = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "policy_type", "policyType", TYPE_UNAMBIGUOUS_KEY, TYPE_SIMPLE_KEY), "policy type"); PolicySpec<? extends Policy> spec = resolvePolicySpec(versionedId, loader, encounteredCatalogTypes); initConfigAndParameters(spec, itemMap, loader); return spec; @@ -166,14 +171,14 @@ class CampInternalUtils { static EnricherSpec<?> createEnricherSpec(BrooklynClassLoadingContext loader, Object enricher, Set<String> encounteredCatalogTypes) { Map<String, Object> itemMap; if (enricher instanceof String) { - itemMap = ImmutableMap.<String, Object>of("type", enricher); + itemMap = ImmutableMap.<String, Object>of(TYPE_SIMPLE_KEY, enricher); } else if (enricher instanceof Map) { itemMap = (Map<String, Object>) enricher; } else { throw new IllegalStateException("Enricher expected to be string or map. Unsupported object type " + enricher.getClass().getName() + " (" + enricher.toString() + ")"); } - String versionedId = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "enricher_type", "enricherType", "type"), "enricher type"); + String versionedId = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "enricher_type", "enricherType", TYPE_UNAMBIGUOUS_KEY, TYPE_SIMPLE_KEY), "enricher type"); EnricherSpec<? extends Enricher> spec = resolveEnricherSpec(versionedId, loader, encounteredCatalogTypes); initConfigAndParameters(spec, itemMap, loader); return spec; @@ -194,14 +199,14 @@ class CampInternalUtils { private static LocationSpec<?> createLocationSpec(BrooklynClassLoadingContext loader, Object location) { Map<String, Object> itemMap; if (location instanceof String) { - itemMap = ImmutableMap.<String, Object>of("type", location); + itemMap = ImmutableMap.<String, Object>of(TYPE_SIMPLE_KEY, location); } else if (location instanceof Map) { itemMap = (Map<String, Object>) location; } else { throw new IllegalStateException("Location expected to be string or map. Unsupported object type " + location.getClass().getName() + " (" + location.toString() + ")"); } - String type = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "location_type", "locationType", "type"), "location type"); + String type = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "location_type", "locationType", TYPE_UNAMBIGUOUS_KEY, TYPE_SIMPLE_KEY), "location type"); Map<String, Object> brooklynConfig = (Map<String, Object>) itemMap.get(BrooklynCampReservedKeys.BROOKLYN_CONFIG); LocationSpec<?> locationSpec = resolveLocationSpec(type, brooklynConfig, loader); // config loaded twice, but used twice diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java index 4eaa8c41e3..1c412052b7 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java @@ -84,7 +84,8 @@ public class CampTypePlanTransformer extends AbstractTypePlanTransformer { protected <T> double scoreObject(T plan, BiFunction<T, String, Boolean> contains) { if (contains.apply(plan, "services")) return 0.8; - if (contains.apply(plan, "type")) return 0.4; + if (contains.apply(plan, CampInternalUtils.TYPE_SIMPLE_KEY)) return 0.4; + if (contains.apply(plan, CampInternalUtils.TYPE_UNAMBIGUOUS_KEY)) return 0.3; if (contains.apply(plan, "brooklyn.locations")) return 0.7; if (contains.apply(plan, "brooklyn.policies")) return 0.7; if (contains.apply(plan, "brooklyn.enrichers")) return 0.7; diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java index a840a74d1a..3cc01a30e8 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java @@ -363,4 +363,26 @@ public class CustomTypeConfigYamlTest extends AbstractYamlTest { lastDeployedEntity = deployWithTestingCustomTypeObjectConfig(true, true, false, "custom-type", CONF1_ANONYMOUS, false); assertLastDeployedKeysValueIsOurCustomTypeWithFieldValues(CONF1_ANONYMOUS, "foo2", "bar"); } + + @Test + public void testRegisteredType_UsingSpecialFieldInheritedTwoStep() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " version: " + TEST_VERSION, + " items:", + " - id: custom-type-0", + " item:", + " (type): " + CustomTypeConfigYamlTest.TestingCustomType.class.getName(), + " x: foo2"); + addCatalogItems( + "brooklyn.catalog:", + " version: " + TEST_VERSION, + " items:", + " - id: custom-type", + " item:", + " (type): custom-type-0", + " y: bar"); + lastDeployedEntity = deployWithTestingCustomTypeObjectConfig(true, true, false, "custom-type", CONF1_ANONYMOUS, false); + assertLastDeployedKeysValueIsOurCustomTypeWithFieldValues(CONF1_ANONYMOUS, "foo2", "bar"); + } } diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index 2956e24861..10c8ebfe3d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -1623,7 +1623,6 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // then try parsing plan - this will use loader // first use transformer approach try { - Object itemToAttemptO = null; Object itemSpecInstantiated = null; if (ATTEMPT_INSTANTIATION_WITH_LEGACY_PLAN_TO_SPEC_CONVERTERS) { @@ -1633,18 +1632,17 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { .plan(candidateYaml) .libraries(libraryBundles) .build(); - itemToAttemptO = itemToAttempt; itemSpecInstantiated = internalCreateSpecLegacy(mgmt, itemToAttempt, MutableSet.<String>of(), true); + } + if (itemSpecInstantiated!=null) { if (!candidateYaml.contains("services:")) { // 'services:' blueprints still need legacy plan-to-spec converter, don't even debug on that. // for others log.debug("Instantiation of this blueprint was only possible with legacy plan-to-spec converter, may not be supported in future versions:\n" + candidateYaml); } - } - if (itemSpecInstantiated!=null) { catalogItemType = candidateCiType; planYaml = candidateYaml; resolved = true; 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 74fcc2dcbb..f9ab39d210 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 @@ -50,12 +50,16 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; public class AsPropertyIfAmbiguous { private static final Logger LOG = LoggerFactory.getLogger(AsPropertyIfAmbiguous.class); private static Set<String> warnedAmbiguousTypeProperty = MutableSet.of(); + public static final Function<String,String> CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM = t -> "("+t+")"; // prefer this as YAML allows it unquoted + public static final Function<String,String> CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM_ALT = t -> "@" + t; // allow this old form too + /** @deprecated since 1.1 now use transform fn, and prefer wrapped in parens */ public static final String CONFLICTING_TYPE_NAME_PROPERTY_PREFIX = "@"; public interface HasBaseType { @@ -111,8 +115,8 @@ public class AsPropertyIfAmbiguous { String tpn = idMetadata.asProperty; if (tpn==null) tpn = _typePropertyName; if (currentClass!=null && Reflections.findFieldMaybe(currentClass, tpn).isPresent()) { - // the class has a field called 'type'; prefix with an '@' - tpn = CONFLICTING_TYPE_NAME_PROPERTY_PREFIX+tpn; + // the class has a field called 'type'; prefix with a '!' + tpn = CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM.apply(tpn); idMetadata.asProperty = tpn; } return super.writeTypePrefix(g, idMetadata); @@ -184,7 +188,7 @@ public class AsPropertyIfAmbiguous { if (baseType != null ) { if (hasTypePropertyNameAsField(baseType)) { // look for an '@' type -// return cloneWithNewTypePropertyName(CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName).deserializeTypedFromObject(p, ctxt); +// return cloneWithNewTypePropertyName(CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM.apply(_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; @@ -254,7 +258,8 @@ public class AsPropertyIfAmbiguous { } 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; + String typeUnambiguous1 = CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM.apply(_typePropertyName); + String typeUnambiguous2 = CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM_ALT.apply(_typePropertyName); JsonLocation loc = p.currentTokenLocation(); //p.getCurrentLocation(); for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) { @@ -263,7 +268,7 @@ public class AsPropertyIfAmbiguous { // unambiguous property should precede ambiguous property name in cases where property name is required // maintaining the parser and token buffer in the desired states to allow either anywhere is too hard - boolean unambiguousName = name.equals(typeUnambiguous); + boolean unambiguousName = name.equals(typeUnambiguous1) || name.equals(typeUnambiguous2); boolean ambiguousName = !unambiguousName && (!mustUseConflictingTypePrefix && (name.equals(_typePropertyName) || (ignoreCase && name.equalsIgnoreCase(_typePropertyName)))); if (ambiguousName || unambiguousName) { // gotcha! @@ -285,8 +290,8 @@ public class AsPropertyIfAmbiguous { // except if it is the first key in the definition, to facilitate messy places where we say 'type: xxx' as the definition if (warnedAmbiguousTypeProperty.add(typeId)) { LOG.warn("Ambiguous type property '" + _typePropertyName + "' used for '" + typeId + "' as first entry in definition; this looks like a type specification but this could also refer to the property; " + - "using for the former, but specification should have used '" + CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName + "' as key earlier in the map, " + - "or if setting the field is intended put an explicit '" + CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName + "' before it"); + "using for the former, but specification should have used '" + typeUnambiguous1 + "' as key earlier in the map, " + + "or if setting the field is intended put an explicit '" + typeUnambiguous1 + "' before it"); } disallowed = false; } else { @@ -297,7 +302,7 @@ public class AsPropertyIfAmbiguous { } else { if (warnedAmbiguousTypeProperty.add(typeId)) { LOG.warn("Ambiguous type property '" + _typePropertyName + "' used for '" + typeId + "'; a type specification is needed to comply with expectations, but this could also refer to the property; " + - "using for the former, but specification should have used " + CONFLICTING_TYPE_NAME_PROPERTY_PREFIX + _typePropertyName + " as key earlier in the map"); + "using for the former, but specification should have used " + typeUnambiguous1 + " as key earlier in the map"); } disallowed = false; } diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypePlanTransformer.java index 7798e4f0ea..b50e6dfc86 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypePlanTransformer.java @@ -38,19 +38,28 @@ import java.util.Map; public class BeanWithTypePlanTransformer extends AbstractTypePlanTransformer { public static final String FORMAT = BeanWithTypeUtils.FORMAT; + public static final String TYPE_SIMPLE_KEY = "type"; + public static final String TYPE_UNAMBIGUOUS_KEY = AsPropertyIfAmbiguous.CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM.apply("type"); public BeanWithTypePlanTransformer() { super(FORMAT, "Brooklyn YAML serialized with type", - "Bean serialization format read/written as YAML or JSON as per Jackson with one trick that the 'type' field specifies the type or supertype"); + "Bean serialization format read/written as YAML or JSON as per Jackson with one trick that the '"+TYPE_SIMPLE_KEY+"' field specifies the type or supertype"); } @Override protected double scoreForNullFormat(Object o, RegisteredType registeredType, RegisteredTypeLoadingContext registeredTypeLoadingContext) { - if (registeredType.getKind()!= RegisteredTypeKind.SPEC && Strings.toString(o).contains("type:")) { - return 0.2; - } else { - return 0; + if (registeredType.getKind()!= RegisteredTypeKind.SPEC) { + if (Strings.toString(o).contains(TYPE_UNAMBIGUOUS_KEY+":")) { + return 0.8; + } + if (Strings.toString(o).contains(TYPE_SIMPLE_KEY+":")) { + return 0.5; + } + if (Strings.toString(o).contains(AsPropertyIfAmbiguous.CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM_ALT.apply(TYPE_SIMPLE_KEY))) { + return 0.4; + } } + return 0; } @Override diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformer.java index da43415957..f033f458c2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformer.java @@ -26,6 +26,7 @@ import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; +import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.text.Identifiers; import org.apache.brooklyn.util.text.Strings; @@ -93,9 +94,14 @@ public class JavaClassNameTypePlanTransformer extends AbstractTypePlanTransforme if (Iterables.size(yaml) == 1) { Object ym = yaml.iterator().next(); if (ym instanceof Map) { - Object yt = ((Map) ym).get("type"); + Object yt = ((Map) ym).get(BeanWithTypePlanTransformer.TYPE_UNAMBIGUOUS_KEY); if (yt instanceof String) { planData = (String) yt; + } else { + yt = ((Map) ym).get(BeanWithTypePlanTransformer.TYPE_SIMPLE_KEY); + if (yt instanceof String) { + planData = (String) yt; + } } } } diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java index bd66d7578b..be93620f84 100644 --- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.core.resolve.jackson; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.brooklyn.api.entity.EntityInitializer; @@ -34,6 +35,7 @@ import org.apache.brooklyn.core.workflow.WorkflowSensor; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.config.ConfigBag; +import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.testng.Assert; import org.testng.annotations.Test; @@ -141,9 +143,51 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt } public static class SampleBeanWithType extends SampleBean { + @JsonInclude(JsonInclude.Include.NON_NULL) public String type; } + @Test + public void testSerializeAndDesSampleBeanWithType() throws Exception { + // complications when the class has a field type -- how should the key 'type' be interpreted? + // prefer (type) also accept @type need this first + + SampleBeanWithType a = new SampleBeanWithType(); + a.x = "hello"; + a.type = "T"; + String deser = "{\"(type)\":\"" + SampleBeanWithType.class.getName() + "\",\"x\":\"hello\",\"type\":\"T\"}"; + Assert.assertEquals(ser(a), deser); + + Object a2 = deser(deser); + Asserts.assertInstanceOf(a2, SampleBeanWithType.class); + Asserts.assertEquals( ((SampleBeanWithType)a2).x, "hello" ); + Asserts.assertEquals( ((SampleBeanWithType)a2).type, "T" ); + + deser = Strings.replaceAllNonRegex(deser, "!", "@"); // @type accepted + a2 = deser(deser); + Asserts.assertInstanceOf(a2, SampleBeanWithType.class); + Asserts.assertEquals( ((SampleBeanWithType)a2).x, "hello" ); + Asserts.assertEquals( ((SampleBeanWithType)a2).type, "T" ); + + a.type = null; + deser = "{\"(type)\":\""+SampleBeanWithType.class.getName()+"\",\"x\":\"hello\"}"; + Assert.assertEquals(ser(a), deser); + Asserts.assertEquals( ((SampleBeanWithType) deser(deser)).x, "hello" ); + Asserts.assertNull( ((SampleBeanWithType) deser(deser)).type ); + + // if type (not (type)) is first it treats it as the type, but warning about ambiguity + deser = Strings.replaceAllNonRegex(deser, "!", ""); + Asserts.assertEquals( ((SampleBeanWithType) deser(deser)).x, "hello" ); + Asserts.assertNull( ((SampleBeanWithType) deser(deser)).type ); + + // if type is not first and refers to a class with a field 'type`, it treats it as a plain vanilla object (map) + deser = "{\"x\":\"hello\",\"type\":\""+SampleBeanWithType.class.getName()+"\"}"; + Asserts.assertInstanceOf(deser(deser), Map.class); + // but for a bean without a field 'type', that is _not_ the case (but might change that in future) + deser = "{\"x\":\"hello\",\"type\":\""+SampleBean.class.getName()+"\"}"; + Asserts.assertInstanceOf(deser(deser), SampleBean.class); + } + @Test public void testDeserializeEntityInitializerWithTypeField() throws Exception { BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, "samplebean-with-type", "1", SampleBeanWithType.class); @@ -206,7 +250,7 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt @Test public void testDeserializeEntityInitializerWithUnambiguousTypeFieldUsedForDeserialization() throws Exception { RegisteredType rt = BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, "samplebean-with-type", "1", SampleBeanWithType.class); - Object impl = deser("{\"x\":\"mytestsensor\",\"@type\":\"samplebean-with-type\",\"type\":\"other\"}", Object.class); + Object impl = deser("{\"x\":\"mytestsensor\",\"(type)\":\"samplebean-with-type\",\"type\":\"other\"}", Object.class); // above cases don't apply, we get a map with both set as fields Asserts.assertInstanceOf(impl, SampleBeanWithType.class); Asserts.assertEquals( ((SampleBean)impl).x, "mytestsensor"); diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/PerverseSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/PerverseSerializationTest.java index 2102a52150..2c543698f6 100644 --- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/PerverseSerializationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/PerverseSerializationTest.java @@ -40,6 +40,7 @@ public class PerverseSerializationTest implements MapperTestFixture { } private static class BeanWithFieldCalledType { + // also see tests of SampleBeanWithType String type; } @@ -48,7 +49,7 @@ public class PerverseSerializationTest implements MapperTestFixture { BeanWithFieldCalledType a = new BeanWithFieldCalledType(); a.type = "not my type"; Assert.assertEquals(ser(a), - "{\"@type\":\""+ BeanWithFieldCalledType.class.getName()+"\",\"type\":\"not my type\"}"); + "{\"(type)\":\""+ BeanWithFieldCalledType.class.getName()+"\",\"type\":\"not my type\"}"); } private final static class BeanWithFieldCalledTypeHolder {
