more code review, lots of minor tidies around type registry; thanks @neykov
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/db024f4e Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/db024f4e Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/db024f4e Branch: refs/heads/master Commit: db024f4e3323e803b421c0e808b7649c4b871d35 Parents: 90e8911 Author: Alex Heneveld <[email protected]> Authored: Tue Nov 17 11:07:46 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Tue Nov 17 12:30:39 2015 +0000 ---------------------------------------------------------------------- .../api/typereg/BrooklynTypeRegistry.java | 12 +-- .../core/catalog/internal/CatalogUtils.java | 7 ++ .../core/mgmt/EntityManagementUtils.java | 2 +- .../AbstractCustomImplementationPlan.java | 52 ----------- ...actFormatSpecificTypeImplementationPlan.java | 52 +++++++++++ .../core/typereg/BasicBrooklynTypeRegistry.java | 28 ++++-- .../core/typereg/BasicRegisteredType.java | 2 + .../typereg/BrooklynTypePlanTransformer.java | 8 +- .../JavaClassNameTypePlanTransformer.java | 7 +- .../typereg/RegisteredTypeLoadingContexts.java | 8 +- .../core/typereg/RegisteredTypePredicates.java | 18 +--- .../brooklyn/core/typereg/RegisteredTypes.java | 76 ++++++++++------- .../core/typereg/TypePlanTransformers.java | 2 +- .../internal/SpecParameterInMetaTest.java | 16 ++++ .../internal/StaticTypePlanTransformer.java | 15 +++- .../core/plan/XmlPlanToSpecTransformer.java | 3 +- .../core/test/BrooklynMgmtUnitTestSupport.java | 3 + .../typereg/ExampleXmlTypePlanTransformer.java | 2 +- .../JavaClassNameTypePlanTransformerTest.java | 90 ++++++++++++++++++++ .../typereg/JavaTypePlanTransformerTest.java | 90 -------------------- .../api/AssemblyTemplateSpecInstantiator.java | 3 - .../BrooklynAssemblyTemplateInstantiator.java | 8 -- .../brooklyn/spi/creation/CampResolver.java | 49 ++++++----- .../spi/creation/CampTypePlanTransformer.java | 6 +- .../CatalogOsgiVersionMoreEntityTest.java | 6 +- .../catalog/CatalogYamlLocationTest.java | 2 +- .../test/lite/TestAppAssemblyInstantiator.java | 4 - .../apache/brooklyn/util/text/Identifiers.java | 16 +++- .../brooklyn/util/text/IdentifiersTest.java | 11 ++- 29 files changed, 326 insertions(+), 272 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java b/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java index ec5db91..42aa8ec 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java @@ -42,7 +42,7 @@ public interface BrooklynTypeRegistry { Iterable<RegisteredType> getAll(); Iterable<RegisteredType> getAll(Predicate<? super RegisteredType> filter); - // XXX remove `context` parameter? + // TODO should we remove the `context` parameter from all these? i don't think it's useful /** @return The item matching the given given * {@link RegisteredType#getSymbolicName() symbolicName} * and optionally {@link RegisteredType#getVersion()}, @@ -61,10 +61,10 @@ public interface BrooklynTypeRegistry { // NB the seemingly more correct generics <T,SpecT extends AbstractBrooklynObjectSpec<T,SpecT>> // cause compile errors, not in Eclipse, but in maven (?) - // TODO do these belong here, or in a separate master TypePlanTransformer ? - <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, Class<SpecT> optionalSpecSuperType); - <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpecFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalContext, Class<SpecT> optionalSpecSuperType); - <T> T createBean(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, Class<T> optionalResultSuperType); - <T> T createBeanFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalConstraint, Class<T> optionalBeanSuperType); + // TODO do these belong here, or in a separate master TypePlanTransformer ? see also BrooklynTypePlanTransformer + <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, @Nullable Class<SpecT> optionalSpecSuperType); + <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpecFromPlan(@Nullable String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalContext, @Nullable Class<SpecT> optionalSpecSuperType); + <T> T createBean(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, @Nullable Class<T> optionalResultSuperType); + <T> T createBeanFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalConstraint, @Nullable Class<T> optionalBeanSuperType); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java index 4a54800..f144b7c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java @@ -72,6 +72,10 @@ public class CatalogUtils { return newClassLoadingContext(mgmt, item.getId(), item.getLibraries(), null); } + /** made @Beta in 0.9.0 because we're not sure to what extent to support stacking loaders; + * only a couple places currently rely on such stacking, in general the item and the bundles *are* the context, + * and life gets hard if we support complex stacking! */ + @Beta public static BrooklynClassLoadingContext newClassLoadingContext(ManagementContext mgmt, RegisteredType item, BrooklynClassLoadingContext loader) { return newClassLoadingContext(mgmt, item.getId(), item.getLibraries(), loader); } @@ -92,6 +96,7 @@ public class CatalogUtils { return newClassLoadingContext(mgmt, catalogItemId, libraries, null); } + @Deprecated /** @deprecated since 0.9.0; becoming private because we should now always have a registered type callers can pass instead of the catalog item id */ public static BrooklynClassLoadingContext newClassLoadingContext(@Nullable ManagementContext mgmt, String catalogItemId, Collection<? extends OsgiBundleWithUrl> libraries, BrooklynClassLoadingContext loader) { BrooklynClassLoadingContextSequential result = new BrooklynClassLoadingContextSequential(mgmt); @@ -100,10 +105,12 @@ public class CatalogUtils { } if (loader !=null) { + // TODO determine whether to support stacking result.add(loader); } BrooklynClassLoadingContext threadLocalLoader = BrooklynLoaderTracker.getLoader(); if (threadLocalLoader != null) { + // TODO and determine if this is needed/wanted result.add(threadLocalLoader); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java index 0f8da61..ef461db 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java @@ -277,7 +277,7 @@ public class EntityManagementUtils { return canPromoteChildrenInWrappedApplication(app); } - /** returns true if the spec is for an empty-ish wrapper app, + /** returns true if the spec is for a wrapper app with no important settings, wrapping a single child. * for use when adding from a plan specifying multiple entities but nothing significant at the application level. * @see #WRAPPER_APP_MARKER */ public static boolean canPromoteChildrenInWrappedApplication(EntitySpec<? extends Application> spec) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCustomImplementationPlan.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCustomImplementationPlan.java b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCustomImplementationPlan.java deleted file mode 100644 index d9dde39..0000000 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCustomImplementationPlan.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.brooklyn.core.typereg; - -import org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan; - -/** Abstract superclass for plans to create {@link TypeImplementationPlan} with strong types on - * {@link #getPlanData()} and ensuring the correct format (or null for no format) */ -public abstract class AbstractCustomImplementationPlan<T> extends BasicTypeImplementationPlan { - - public AbstractCustomImplementationPlan(String format, T data) { - super(format, data); - } - public AbstractCustomImplementationPlan(String expectedFormat, Class<T> expectedDataType, TypeImplementationPlan otherPlan) { - super(expectedFormat!=null ? expectedFormat : otherPlan.getPlanFormat(), otherPlan.getPlanData()); - if (!expectedDataType.isInstance(otherPlan.getPlanData())) { - throw new IllegalArgumentException("Plan "+otherPlan+" does not have "+expectedDataType+" data so cannot cast to "+this); - } - if (expectedFormat!=null && otherPlan.getPlanFormat()!=null) { - if (!otherPlan.getPlanFormat().equals(expectedFormat)) { - throw new IllegalArgumentException("Plan "+otherPlan+" in wrong format "+otherPlan.getPlanFormat()+", when expecting "+expectedFormat); - } - } - } - - @Override - public String getPlanFormat() { - return format; - } - - @SuppressWarnings("unchecked") - @Override - public T getPlanData() { - return (T)super.getPlanData(); - } -} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractFormatSpecificTypeImplementationPlan.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractFormatSpecificTypeImplementationPlan.java b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractFormatSpecificTypeImplementationPlan.java new file mode 100644 index 0000000..9ce4e55 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractFormatSpecificTypeImplementationPlan.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.typereg; + +import org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan; + +/** Abstract superclass for plans to create {@link TypeImplementationPlan} with strong types on + * {@link #getPlanData()} and ensuring the correct format (or null for no format) */ +public abstract class AbstractFormatSpecificTypeImplementationPlan<T> extends BasicTypeImplementationPlan { + + public AbstractFormatSpecificTypeImplementationPlan(String format, T data) { + super(format, data); + } + public AbstractFormatSpecificTypeImplementationPlan(String expectedFormat, Class<T> expectedDataType, TypeImplementationPlan otherPlan) { + super(expectedFormat!=null ? expectedFormat : otherPlan.getPlanFormat(), otherPlan.getPlanData()); + if (!expectedDataType.isInstance(otherPlan.getPlanData())) { + throw new IllegalArgumentException("Plan "+otherPlan+" does not have "+expectedDataType+" data so cannot cast to "+this); + } + if (expectedFormat!=null && otherPlan.getPlanFormat()!=null) { + if (!otherPlan.getPlanFormat().equals(expectedFormat)) { + throw new IllegalArgumentException("Plan "+otherPlan+" in wrong format "+otherPlan.getPlanFormat()+", when expecting "+expectedFormat); + } + } + } + + @Override + public String getPlanFormat() { + return format; + } + + @SuppressWarnings("unchecked") + @Override + public T getPlanData() { + return (T)super.getPlanData(); + } +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java index 1bc4a9f..2f2d4f5 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java @@ -18,6 +18,8 @@ */ package org.apache.brooklyn.core.typereg; +import java.util.Set; + import javax.annotation.Nullable; import org.apache.brooklyn.api.catalog.BrooklynCatalog; @@ -27,6 +29,7 @@ import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry; import org.apache.brooklyn.api.typereg.RegisteredType; +import org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan; import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.catalog.internal.CatalogItemBuilder; @@ -99,18 +102,27 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry { return get(symbolicNameWithOptionalVersion, (RegisteredTypeLoadingContext)null); } - @SuppressWarnings({ "deprecation", "unchecked", "rawtypes" }) @Override public <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec(RegisteredType type, @Nullable RegisteredTypeLoadingContext constraint, Class<SpecT> specSuperType) { Preconditions.checkNotNull(type, "type"); if (type.getKind()!=RegisteredTypeKind.SPEC) { throw new IllegalStateException("Cannot create spec from type "+type+" (kind "+type.getKind()+")"); } + return createSpec(type, type.getPlan(), type.getSymbolicName(), type.getVersion(), type.getSuperTypes(), constraint, specSuperType); + } + + @SuppressWarnings({ "deprecation", "unchecked", "rawtypes" }) + private <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec( + RegisteredType type, + TypeImplementationPlan plan, + @Nullable String symbolicName, @Nullable String version, Set<Object> superTypes, + @Nullable RegisteredTypeLoadingContext constraint, Class<SpecT> specSuperType) { + // TODO type is only used to call to "transform"; we should perhaps change transform so it doesn't need the type? if (constraint!=null) { if (constraint.getExpectedKind()!=null && constraint.getExpectedKind()!=RegisteredTypeKind.SPEC) { throw new IllegalStateException("Cannot create spec with constraint "+constraint); } - if (constraint.getAlreadyEncounteredTypes().contains(type.getSymbolicName())) { + if (constraint.getAlreadyEncounteredTypes().contains(symbolicName)) { // avoid recursive cycle // TODO implement using java if permitted } @@ -122,7 +134,7 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry { // fallback: look up in (legacy) catalog // TODO remove once all transformers are available in the new style - CatalogItem item = (CatalogItem) mgmt.getCatalog().getCatalogItem(type.getSymbolicName(), type.getVersion()); + CatalogItem item = symbolicName!=null ? (CatalogItem) mgmt.getCatalog().getCatalogItem(symbolicName, version) : null; if (item==null) { // if not in catalog (because loading a new item?) then look up item based on type // (only really used in tests; possibly also for any recursive legacy transformers we might have to create a CI; cross that bridge when we come to it) @@ -132,9 +144,9 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry { result.get(); } item = CatalogItemBuilder.newItem(ciType, - type.getSymbolicName()!=null ? type.getSymbolicName() : Identifiers.makeRandomId(8), - type.getVersion()!=null ? type.getVersion() : BasicBrooklynCatalog.DEFAULT_VERSION) - .plan(RegisteredTypes.getImplementationDataStringForSpec(type)) + symbolicName!=null ? symbolicName : Identifiers.makeRandomId(8), + version!=null ? version : BasicBrooklynCatalog.DEFAULT_VERSION) + .plan((String)plan.getPlanData()) .build(); } try { @@ -145,10 +157,10 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry { try { result.get(); // above will throw -- so won't come here - throw new IllegalStateException("should have failed getting type resolution for "+type); + throw new IllegalStateException("should have failed getting type resolution for "+symbolicName); } catch (Exception e0) { // prefer older exception, until the new transformer is the primary pathway - throw Exceptions.create("Unable to instantiate "+type, MutableList.of(e0, e)); + throw Exceptions.create("Unable to instantiate "+(symbolicName==null ? "item" : symbolicName), MutableList.of(e0, e)); } } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java index 54b04a3..3905d65 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java @@ -30,6 +30,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.javalang.JavaClassNames; +import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableSet; /** Instances are usually created by methods in {@link RegisteredTypes}. */ @@ -113,6 +114,7 @@ public class BasicRegisteredType implements RegisteredType { return ImmutableSet.copyOf(superTypes); } + @Beta // TODO depending how useful this is, it might be better to replace by a static WeakHashMap in RegisteredTypes public ConfigBag getCache() { return cache; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java index e185d3f..5609cf9 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java @@ -30,6 +30,8 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import org.apache.brooklyn.core.mgmt.ManagementContextInjectable; +import com.google.common.annotations.Beta; + /** * Interface for use by schemes which provide the capability to transform plans * (serialized descriptions) to brooklyn objecs and specs. @@ -75,8 +77,12 @@ public interface BrooklynTypePlanTransformer extends ManagementContextInjectable * Implementations should either return null or throw {@link UnsupportedTypePlanException} * if the {@link RegisteredType#getPlan()} is not supported. */ @Nullable Object create(@Nonnull RegisteredType type, @Nonnull RegisteredTypeLoadingContext context); - + + // TODO sketch methods for loading *catalog* definitions. note some potential overlap + // with BrooklynTypeRegistery.createXxxFromPlan + @Beta double scoreForTypeDefinition(String formatCode, Object catalogData); + @Beta List<RegisteredType> createFromTypeDefinition(String formatCode, Object catalogData); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformer.java ---------------------------------------------------------------------- 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 5e6877b..29a4ec1 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 @@ -37,8 +37,8 @@ public class JavaClassNameTypePlanTransformer extends AbstractTypePlanTransforme public static final String FORMAT = "java-type-name"; - public static class JavaTypeNameImplementation extends AbstractCustomImplementationPlan<String> { - public JavaTypeNameImplementation(String javaType) { super(FORMAT, javaType); } + public static class JavaClassNameTypeImplementationPlan extends AbstractFormatSpecificTypeImplementationPlan<String> { + public JavaClassNameTypeImplementationPlan(String javaType) { super(FORMAT, javaType); } } public JavaClassNameTypePlanTransformer() { @@ -47,8 +47,9 @@ public class JavaClassNameTypePlanTransformer extends AbstractTypePlanTransforme @Override protected double scoreForNullFormat(Object planData, RegisteredType type, RegisteredTypeLoadingContext context) { + // the "good" regex doesn't allow funny unicode chars; we'll accept that for now if (type.getPlan().getPlanData() instanceof String && - ((String)type.getPlan().getPlanData()).matches(Identifiers.JAVA_BINARY_REGEX)) { + ((String)type.getPlan().getPlanData()).matches(Identifiers.JAVA_GOOD_BINARY_REGEX)) { return 0.1; } return 0; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeLoadingContexts.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeLoadingContexts.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeLoadingContexts.java index 594bdfd..0368ba3 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeLoadingContexts.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeLoadingContexts.java @@ -41,7 +41,7 @@ import com.google.common.collect.ImmutableSet; public class RegisteredTypeLoadingContexts { - private static final Logger log = LoggerFactory.getLogger(RegisteredTypeLoadingContexts.BasicRegisteredTypeLoadingContext.class); + private static final Logger log = LoggerFactory.getLogger(RegisteredTypeLoadingContexts.class); /** Immutable (from caller's perspective) record of a constraint */ public final static class BasicRegisteredTypeLoadingContext implements RegisteredTypeLoadingContext { @@ -58,7 +58,7 @@ public class RegisteredTypeLoadingContexts { this.kind = source.getExpectedKind(); this.expectedSuperType = source.getExpectedJavaSuperType(); this.encounteredTypes = source.getAlreadyEncounteredTypes(); - this.loader = (BrooklynClassLoadingContext) source.getLoader(); + this.loader = source.getLoader(); } @Override @@ -204,14 +204,14 @@ public class RegisteredTypeLoadingContexts { BrooklynObjectType best = null; for (BrooklynObjectType t: BrooklynObjectType.values()) { - if (t.getSpecType()==null) continue; + if (t.getInterfaceType()==null) continue; if (!t.getInterfaceType().isAssignableFrom(targetSuperType)) continue; // on equality, exit immediately if (t.getInterfaceType().equals(targetSuperType)) return t.getSpecType(); // else pick which is best if (best==null) { best = t; continue; } // if t is more specific, it is better (handles case when e.g. a Policy is a subclass of Entity) - if (best.getSpecType().isAssignableFrom(t.getSpecType())) { best = t; continue; } + if (best.getInterfaceType().isAssignableFrom(t.getInterfaceType())) { best = t; continue; } } if (best==null) { log.warn("Unexpected target supertype ("+targetSuperType+"); unable to infer spec type"); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypePredicates.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypePredicates.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypePredicates.java index 2e7c038..bc81d8e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypePredicates.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypePredicates.java @@ -121,31 +121,17 @@ public class RegisteredTypePredicates { private final Predicate<Class<?>> filter; @SuppressWarnings({ "rawtypes", "unchecked" }) - private <T> AnySuperTypeMatches(Predicate filter) { + private AnySuperTypeMatches(Predicate filter) { this.filter = filter; } @Override public boolean apply(@Nullable RegisteredType item) { if (item==null) return false; - for (Object o: item.getSuperTypes()) { - if (o instanceof Class) { - if (filter.apply((Class<?>)o)) return true; - } - } - for (Object o: item.getSuperTypes()) { - if (o instanceof RegisteredType) { - if (apply((RegisteredType)o)) return true; - } - } - return false; + return RegisteredTypes.isAnyTypeOrSuperSatisfying(item.getSuperTypes(), filter); } } public static final Predicate<RegisteredType> IS_APPLICATION = assignableFrom(Application.class); - // TODO do we need this? introduced already deprecated in 0.9.0 so can be removed, or enabled - @Deprecated - public static final Predicate<RegisteredType> IS_TEMPLATE = IS_APPLICATION; - public static final Predicate<RegisteredType> IS_ENTITY = assignableFrom(Entity.class); public static final Predicate<RegisteredType> IS_LOCATION = assignableFrom(Location.class); public static final Predicate<RegisteredType> IS_POLICY = assignableFrom(Policy.class); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java index 1e48e46..6fd993a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java @@ -37,14 +37,15 @@ import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; import org.apache.brooklyn.core.config.ConfigKeys; -import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer.JavaTypeNameImplementation; +import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer.JavaClassNameTypeImplementationPlan; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.yaml.Yamls; import com.google.common.annotations.Beta; import com.google.common.base.Function; -import com.google.common.base.Preconditions; +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.reflect.TypeToken; @@ -54,7 +55,7 @@ import com.google.common.reflect.TypeToken; * Use {@link #bean(String, String, TypeImplementationPlan, Class)} and {@link #spec(String, String, TypeImplementationPlan, Class)} * to create {@link RegisteredType} instances. * <p> - * See {@link #isSubTypeOf(RegisteredType, Class)} or {@link #isSubTypeOf(RegisteredType, RegisteredType)} to + * See {@link #isAssignableFrom(RegisteredType, Class)} or {@link #isAssignableFrom(RegisteredType, RegisteredType)} to * inspect the type hierarchy. */ public class RegisteredTypes { @@ -80,7 +81,7 @@ public class RegisteredTypes { if (item.getPlanYaml()!=null) { impl = new BasicTypeImplementationPlan(null, item.getPlanYaml()); } else if (item.getJavaType()!=null) { - impl = new JavaTypeNameImplementation(item.getJavaType()); + impl = new JavaClassNameTypeImplementationPlan(item.getJavaType()); } else { throw new IllegalStateException("Unsupported catalog item "+item+" when trying to create RegisteredType"); } @@ -99,11 +100,14 @@ public class RegisteredTypes { return type; } - /** Preferred mechanism for defining a bean {@link RegisteredType} */ + /** Preferred mechanism for defining a bean {@link RegisteredType}. */ public static RegisteredType bean(String symbolicName, String version, TypeImplementationPlan plan, @Nullable Class<?> superType) { return addSuperType(new BasicRegisteredType(RegisteredTypeKind.BEAN, symbolicName, version, plan), superType); } + /** Preferred mechanism for defining a spec {@link RegisteredType}. */ + // TODO we currently allow symbolicName and version to be null for the purposes of creation, internal only in BasicBrooklynTypeRegistry.createSpec + // (ideally the API in TypePlanTransformer can be changed so even that is not needed) public static RegisteredType spec(String symbolicName, String version, TypeImplementationPlan plan, @Nullable Class<?> superType) { return addSuperType(new BasicRegisteredType(RegisteredTypeKind.SPEC, symbolicName, version, plan), superType); } @@ -113,12 +117,11 @@ public class RegisteredTypes { * @param mgmt */ @Beta // TODO should this be on the AbstractTypePlanTransformer ? - public static Class<?> loadActualJavaType(String javaTypeName, ManagementContext mgmt, RegisteredType type, RegisteredTypeLoadingContext context) throws Exception { + public static Class<?> loadActualJavaType(String javaTypeName, ManagementContext mgmt, RegisteredType type, RegisteredTypeLoadingContext context) { Class<?> result = ((BasicRegisteredType)type).getCache().get(ACTUAL_JAVA_TYPE); if (result!=null) return result; result = CatalogUtils.newClassLoadingContext(mgmt, type, context==null ? null : context.getLoader()).loadClass( javaTypeName ); - Preconditions.checkNotNull(result, "Could not load class "+javaTypeName+"; returned null (should have thrown a different exception!)"); ((BasicRegisteredType)type).getCache().put(ACTUAL_JAVA_TYPE, result); return result; @@ -135,7 +138,7 @@ public class RegisteredTypes { @Beta public static RegisteredType addSuperType(RegisteredType type, @Nullable RegisteredType superType) { if (superType!=null) { - if (isSubTypeOf(superType, type)) { + if (isAssignableFrom(superType, type)) { throw new IllegalStateException(superType+" declares "+type+" as a supertype; cannot set "+superType+" as a supertype of "+type); } ((BasicRegisteredType)type).superTypes.add(superType); @@ -143,12 +146,12 @@ public class RegisteredTypes { return type; } - /** returns the implementation data for a spec if it is a string (e.g. plan yaml or java class name); else false */ + /** returns the implementation data for a spec if it is a string (e.g. plan yaml or java class name); else throws */ @Beta public static String getImplementationDataStringForSpec(RegisteredType item) { if (item==null || item.getPlan()==null) return null; Object data = item.getPlan().getPlanData(); - if (!(data instanceof String)) return null; + if (!(data instanceof String)) throw new IllegalStateException("Expected plan data for "+item+" to be a string"); return (String)data; } @@ -167,7 +170,7 @@ public class RegisteredTypes { /** Returns a wrapped map, if the object is YAML which parses as a map; * otherwise returns absent capable of throwing an error with more details */ @SuppressWarnings("unchecked") - public static Maybe<Map<Object,Object>> getAsYamlMap(Object planData) { + public static Maybe<Map<?,?>> getAsYamlMap(Object planData) { if (!(planData instanceof String)) return Maybe.absent("not a string"); Iterable<Object> result; try { @@ -180,29 +183,18 @@ public class RegisteredTypes { if (!ri.hasNext()) return Maybe.absent("YAML has no elements in it"); Object r1 = ri.next(); if (ri.hasNext()) return Maybe.absent("YAML has multiple elements in it"); - if (r1 instanceof Map) return Maybe.of((Map<Object,Object>)r1); + if (r1 instanceof Map) return (Maybe<Map<?,?>>)(Maybe<?>) Maybe.of(r1); return Maybe.absent("YAML does not contain a map"); } /** * Queries recursively the supertypes of {@link RegisteredType} to see whether it - * declares a supertype compatible with the given {@link Class} */ - public static boolean isSubTypeOf(RegisteredType type, Class<?> superType) { - return isSubTypeOf(type.getSuperTypes(), superType); - } - - /** - * Queries recursively the given types (either {@link Class} or {@link RegisteredType}) - * to see whether any are compatible with the given {@link Class} */ - public static boolean isSubTypeOf(Set<Object> allKnownTypes, Class<?> superType) { - for (Object st: allKnownTypes) { - if (st instanceof Class) { - if (superType.isAssignableFrom((Class<?>)st)) return true; - } - } - for (Object st: allKnownTypes) { + * inherits from the given {@link RegisteredType} */ + public static boolean isAssignableFrom(RegisteredType type, RegisteredType superType) { + if (type.equals(superType)) return true; + for (Object st: type.getSuperTypes()) { if (st instanceof RegisteredType) { - if (isSubTypeOf((RegisteredType)st, superType)) return true; + if (isAssignableFrom((RegisteredType)st, superType)) return true; } } return false; @@ -210,12 +202,30 @@ public class RegisteredTypes { /** * Queries recursively the supertypes of {@link RegisteredType} to see whether it - * declares a supertype compatible with the given {@link Class} */ - public static boolean isSubTypeOf(RegisteredType type, RegisteredType superType) { - if (type.equals(superType)) return true; - for (Object st: type.getSuperTypes()) { + * inherits from the given {@link Class} */ + public static boolean isAssignableFrom(RegisteredType type, Class<?> superType) { + return isAnyTypeAssignableFrom(type.getSuperTypes(), superType); + } + + /** + * Queries recursively the given types (either {@link Class} or {@link RegisteredType}) + * to see whether any inherit from the given {@link Class} */ + public static boolean isAnyTypeAssignableFrom(Set<Object> candidateTypes, Class<?> superType) { + return isAnyTypeOrSuperSatisfying(candidateTypes, Predicates.assignableFrom(superType)); + } + + /** + * Queries recursively the given types (either {@link Class} or {@link RegisteredType}) + * to see whether any java superclasses satisfy the given {@link Predicate} */ + public static boolean isAnyTypeOrSuperSatisfying(Set<Object> candidateTypes, Predicate<Class<?>> filter) { + for (Object st: candidateTypes) { + if (st instanceof Class) { + if (filter.apply((Class<?>)st)) return true; + } + } + for (Object st: candidateTypes) { if (st instanceof RegisteredType) { - if (isSubTypeOf((RegisteredType)st, superType)) return true; + if (isAnyTypeOrSuperSatisfying(((RegisteredType)st).getSuperTypes(), filter)) return true; } } return false; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java b/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java index a082a31..b554e09 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java @@ -98,7 +98,7 @@ public class TypePlanTransformers { Map<Double, Collection<BrooklynTypePlanTransformer>> tree = new TreeMap<Double, Collection<BrooklynTypePlanTransformer>>(byScoreMulti.asMap()); List<Collection<BrooklynTypePlanTransformer>> highestFirst = new ArrayList<Collection<BrooklynTypePlanTransformer>>(tree.values()); Collections.reverse(highestFirst); - return MutableList.copyOf(Iterables.concat(highestFirst)).asUnmodifiable(); + return ImmutableList.copyOf(Iterables.concat(highestFirst)); } /** transforms the given type to an instance, if possible http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/catalog/internal/SpecParameterInMetaTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/SpecParameterInMetaTest.java b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/SpecParameterInMetaTest.java index 5437765..8fc5dd1 100644 --- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/SpecParameterInMetaTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/SpecParameterInMetaTest.java @@ -55,8 +55,12 @@ public class SpecParameterInMetaTest { public void setUp() { mgmt = LocalManagementContextForTests.newInstanceWithOsgi(); catalog = mgmt.getCatalog(); + + // TODO ugly, but we need the legacy style TestToSpecTransformer currently to be able to do the transformation; + // remove that class and the refs to PlanToSpecFactory here when we're entirely migrated to new-style transformers StaticTypePlanTransformer.forceInstall(); PlanToSpecFactory.forceAvailable(TestToSpecTransformer.class, JavaCatalogToSpecTransformer.class); + specId = StaticTypePlanTransformer.registerSpec(EntitySpec.create(BasicEntity.class)); } @@ -79,6 +83,18 @@ public class SpecParameterInMetaTest { // RegisteredType type = mgmt.getTypeRegistry().get(specId); // Assert.assertNotNull(type); // } + /* TODO - remove above when @ahgittin and @neykov agree-- discussion from https://github.com/apache/incubator-brooklyn/pull/1017: + * + * Being able to create a spec from a plan is different from adding a catalog item so don't agree, it's a separate + * thing. The mechanism could be used for application specs as well, it's not specific to the catalog. + * + * Could add a utility method somewhere to add a catalog item for the registered spec, but not useful for the + * following tests. + * + * I don't quite follow. In general if a plan refers to a type, I'd expect that type in the catalog (or a + * java class). While a transformer can define other rules for instantiating types, I'm not sure that's good + * practice. (Except it's okay for tests.) + */ @Test public void testYamlInputsParsed() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/catalog/internal/StaticTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/StaticTypePlanTransformer.java b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/StaticTypePlanTransformer.java index c5568ca..613fe35 100644 --- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/StaticTypePlanTransformer.java +++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/StaticTypePlanTransformer.java @@ -31,19 +31,25 @@ import org.apache.brooklyn.core.typereg.TypePlanTransformers; import org.apache.brooklyn.util.text.Identifiers; /** - * Resolves previously registered specs by id. - * First create a spec and register it, keeping the returned ID: + * Allows a caller to register a spec (statically) and get a UID for it -- * <pre> {@code * String specId = StaticTypePlanTransformer.registerSpec(EntitySpec.create(BasicEntity.class)); * }</pre> - * - * Then build a plan to be resolved such as: + * and then build a plan referring to that type name, such as: * <pre> {@code * brooklyn.catalog: * id: test.inputs * version: 0.0.1 * item: <specId> * } </pre> + * <p> + * For use when testing type plan resolution. + * <p> + * This is different to {@link JavaClassNameTypePlanTransformer} as that one + * does a <code>Class.forName(typeName)</code> to create specs, and this one uses a static registry. + * <p> + * Use {@link #forceInstall()} to set up and {@link #clearForced()} after use (in a finally or "AfterTest" block) + * to prevent interference with other tests. */ public class StaticTypePlanTransformer extends AbstractTypePlanTransformer { @@ -59,6 +65,7 @@ public class StaticTypePlanTransformer extends AbstractTypePlanTransformer { public static void clearForced() { TypePlanTransformers.clearForced(); + REGISTERED_SPECS.clear(); } public static String registerSpec(AbstractBrooklynObjectSpec<?, ?> spec) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java b/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java index 063df64..b31e614 100644 --- a/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java +++ b/core/src/test/java/org/apache/brooklyn/core/plan/XmlPlanToSpecTransformer.java @@ -43,7 +43,8 @@ import org.w3c.dom.Node; public class XmlPlanToSpecTransformer implements PlanToSpecTransformer { // this is REPLACED by ExampleXmlTypePlanTransformer - + // TODO remove when PlanToSpecTransformer is removed + @SuppressWarnings("unused") private ManagementContext mgmt; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/test/BrooklynMgmtUnitTestSupport.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynMgmtUnitTestSupport.java b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynMgmtUnitTestSupport.java index 956ad63..9742bce 100644 --- a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynMgmtUnitTestSupport.java +++ b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynMgmtUnitTestSupport.java @@ -50,6 +50,9 @@ public class BrooklynMgmtUnitTestSupport { if (mgmt != null) Entities.destroyAll(mgmt); } catch (Throwable t) { LOG.error("Caught exception in tearDown method", t); + // we should fail here, except almost always that masks a primary failure in the test itself, + // so it would be extremely unhelpful to do so. if we could check if test has not already failed, + // that would be ideal, but i'm not sure if that's possible with TestNG. ? } finally { mgmt = null; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/typereg/ExampleXmlTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/ExampleXmlTypePlanTransformer.java b/core/src/test/java/org/apache/brooklyn/core/typereg/ExampleXmlTypePlanTransformer.java index 01b80a1..23035c4 100644 --- a/core/src/test/java/org/apache/brooklyn/core/typereg/ExampleXmlTypePlanTransformer.java +++ b/core/src/test/java/org/apache/brooklyn/core/typereg/ExampleXmlTypePlanTransformer.java @@ -71,7 +71,7 @@ public class ExampleXmlTypePlanTransformer extends AbstractTypePlanTransformer { } private static boolean isApplicationExpected(RegisteredType type, RegisteredTypeLoadingContext context) { - return RegisteredTypes.isSubTypeOf(type, Application.class) || + return RegisteredTypes.isAssignableFrom(type, Application.class) || (context.getExpectedJavaSuperType()!=null && context.getExpectedJavaSuperType().isAssignableFrom(Application.class)); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformerTest.java b/core/src/test/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformerTest.java new file mode 100644 index 0000000..23dc2f3 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/typereg/JavaClassNameTypePlanTransformerTest.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.typereg; + +import org.apache.brooklyn.api.typereg.RegisteredType; +import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport; +import org.testng.Assert; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class JavaClassNameTypePlanTransformerTest extends BrooklynMgmtUnitTestSupport { + + public static class NoArg { + public String name() { return "no-arg"; } + } + + protected RegisteredType type; + protected BrooklynTypePlanTransformer transformer; + + @BeforeMethod(alwaysRun=true) + @Override + public void setUp() throws Exception { + super.setUp(); + type = newNoArgRegisteredType(JavaClassNameTypePlanTransformer.FORMAT); + transformer = newTransformer(); + } + + protected RegisteredType newNoArgRegisteredType(String format) { + return RegisteredTypes.bean("no-arg", "1.0", new BasicTypeImplementationPlan(format, NoArg.class.getName()), null); + } + + protected BrooklynTypePlanTransformer newTransformer() { + BrooklynTypePlanTransformer xf = new JavaClassNameTypePlanTransformer(); + xf.injectManagementContext(mgmt); + return xf; + } + + @Test + public void testScoreJavaType() { + double score = transformer.scoreForType(type, null); + Assert.assertEquals(score, 1, 0.00001); + } + + @Test + public void testCreateJavaType() { + Object obj = transformer.create(type, null); + Assert.assertTrue(obj instanceof NoArg, "obj is "+obj); + Assert.assertEquals(((NoArg)obj).name(), "no-arg"); + } + + @Test + public void testScoreJavaTypeWithNullFormat() { + type = newNoArgRegisteredType(null); + double score = transformer.scoreForType(type, null); + Assert.assertEquals(score, 0.1, 0.00001); + } + + @Test + public void testCreateJavaTypeWithNullFormat() { + type = newNoArgRegisteredType(null); + Object obj = transformer.create(type, null); + Assert.assertTrue(obj instanceof NoArg, "obj is "+obj); + Assert.assertEquals(((NoArg)obj).name(), "no-arg"); + } + + @Test + public void testScoreJavaTypeWithOtherFormat() { + type = newNoArgRegisteredType("crazy-format"); + double score = transformer.scoreForType(type, null); + Assert.assertEquals(score, 0, 0.00001); + // we don't test creation; it may or may not succeed, but with score 0 it shouldn't get invoked + } + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/core/src/test/java/org/apache/brooklyn/core/typereg/JavaTypePlanTransformerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/JavaTypePlanTransformerTest.java b/core/src/test/java/org/apache/brooklyn/core/typereg/JavaTypePlanTransformerTest.java deleted file mode 100644 index def79d8..0000000 --- a/core/src/test/java/org/apache/brooklyn/core/typereg/JavaTypePlanTransformerTest.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.brooklyn.core.typereg; - -import org.apache.brooklyn.api.typereg.RegisteredType; -import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport; -import org.testng.Assert; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; - -public class JavaTypePlanTransformerTest extends BrooklynMgmtUnitTestSupport { - - public static class NoArg { - public String name() { return "no-arg"; } - } - - protected RegisteredType type; - protected BrooklynTypePlanTransformer transformer; - - @BeforeMethod(alwaysRun=true) - @Override - public void setUp() throws Exception { - super.setUp(); - type = newNoArgRegisteredType(JavaClassNameTypePlanTransformer.FORMAT); - transformer = newTransformer(); - } - - protected RegisteredType newNoArgRegisteredType(String format) { - return RegisteredTypes.bean("no-arg", "1.0", new BasicTypeImplementationPlan(format, NoArg.class.getName()), null); - } - - protected BrooklynTypePlanTransformer newTransformer() { - BrooklynTypePlanTransformer xf = new JavaClassNameTypePlanTransformer(); - xf.injectManagementContext(mgmt); - return xf; - } - - @Test - public void testScoreJavaType() { - double score = transformer.scoreForType(type, null); - Assert.assertEquals(score, 1, 0.00001); - } - - @Test - public void testCreateJavaType() { - Object obj = transformer.create(type, null); - Assert.assertTrue(obj instanceof NoArg, "obj is "+obj); - Assert.assertEquals(((NoArg)obj).name(), "no-arg"); - } - - @Test - public void testScoreJavaTypeWithNullFormat() { - type = newNoArgRegisteredType(null); - double score = transformer.scoreForType(type, null); - Assert.assertEquals(score, 0.1, 0.00001); - } - - @Test - public void testCreateJavaTypeWithNullFormat() { - type = newNoArgRegisteredType(null); - Object obj = transformer.create(type, null); - Assert.assertTrue(obj instanceof NoArg, "obj is "+obj); - Assert.assertEquals(((NoArg)obj).name(), "no-arg"); - } - - @Test - public void testScoreJavaTypeWithOtherFormat() { - type = newNoArgRegisteredType("crazy-format"); - double score = transformer.scoreForType(type, null); - Assert.assertEquals(score, 0, 0.00001); - // we don't test creation; it may or may not succeed, but with score 0 it shouldn't get invoked - } - -} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/api/AssemblyTemplateSpecInstantiator.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/api/AssemblyTemplateSpecInstantiator.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/api/AssemblyTemplateSpecInstantiator.java index 87bd381..8540663 100644 --- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/api/AssemblyTemplateSpecInstantiator.java +++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/api/AssemblyTemplateSpecInstantiator.java @@ -29,9 +29,6 @@ import org.apache.brooklyn.camp.spi.AssemblyTemplate; import org.apache.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator; public interface AssemblyTemplateSpecInstantiator extends AssemblyTemplateInstantiator { - - @Deprecated /** @deprecaed since 0.9.0 include encountered types */ - EntitySpec<? extends Application> createApplicationSpec(AssemblyTemplate template, CampPlatform platform, BrooklynClassLoadingContext loader); /** * Gets the single item returned by {@link #createServiceSpecs} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java index 6c7a479..5840440 100644 --- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java +++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java @@ -77,14 +77,6 @@ public class BrooklynAssemblyTemplateInstantiator implements AssemblyTemplateSpe public EntitySpec<? extends Application> createApplicationSpec( AssemblyTemplate template, CampPlatform platform, - BrooklynClassLoadingContext loader) { - return createApplicationSpec(template, platform, loader, MutableSet.<String>of()); - } - - @Override - public EntitySpec<? extends Application> createApplicationSpec( - AssemblyTemplate template, - CampPlatform platform, BrooklynClassLoadingContext loader, Set<String> encounteredTypeSymbolicNames) { log.debug("CAMP creating application instance for {} ({})", template.getId(), template); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java index e6c514a..b9e63d1 100644 --- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java +++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java @@ -39,6 +39,7 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.text.Strings; +import org.python.google.common.collect.Iterables; import com.google.common.collect.ImmutableSet; @@ -48,20 +49,22 @@ class CampResolver { private RegisteredType type; private RegisteredTypeLoadingContext context; - /** whether to allow parsing of the 'full' syntax for applications, - * where items are wrapped in a "services:" block, and if the wrapper is an application, - * to promote it */ - boolean allowApplicationFullSyntax = true; - - /** whether to allow parsing of the legacy 'full' syntax, - * where a non-application items are wrapped: - * <li> in a "services:" block for entities, - * <li> in a "brooklyn.locations" or "brooklyn.policies" block for locations and policies */ - boolean allowLegacyFullSyntax = true; - - /** whether to allow parsing of the type syntax, where an item is a map with a "type:" field, - * i.e. not wrapped in any "services:" or "brooklyn.{locations,policies}" block */ - boolean allowTypeSyntax = true; + // TODO we have a few different modes, detailed below; this logic should be moved to the new transformer + // and allow specifying which modes are permitted to be in effect? +// /** whether to allow parsing of the 'full' syntax for applications, +// * where items are wrapped in a "services:" block, and if the wrapper is an application, +// * to promote it */ +// boolean allowApplicationFullSyntax = true; +// +// /** whether to allow parsing of the legacy 'full' syntax, +// * where a non-application items are wrapped: +// * <li> in a "services:" block for entities, +// * <li> in a "brooklyn.locations" or "brooklyn.policies" block for locations and policies */ +// boolean allowLegacyFullSyntax = true; +// +// /** whether to allow parsing of the type syntax, where an item is a map with a "type:" field, +// * i.e. not wrapped in any "services:" or "brooklyn.{locations,policies}" block */ +// boolean allowTypeSyntax = true; public CampResolver(ManagementContext mgmt, RegisteredType type, RegisteredTypeLoadingContext context) { this.mgmt = mgmt; @@ -97,19 +100,20 @@ class CampResolver { String planYaml = RegisteredTypes.getImplementationDataStringForSpec(item); MutableSet<Object> supers = MutableSet.copyOf(item.getSuperTypes()); supers.addIfNotNull(expectedType); - if (RegisteredTypes.isSubTypeOf(supers, Policy.class)) { + if (RegisteredTypes.isAnyTypeAssignableFrom(supers, Policy.class)) { spec = CampInternalUtils.createPolicySpec(planYaml, loader, encounteredTypes); - } else if (RegisteredTypes.isSubTypeOf(supers, Location.class)) { + } else if (RegisteredTypes.isAnyTypeAssignableFrom(supers, Location.class)) { spec = CampInternalUtils.createLocationSpec(planYaml, loader, encounteredTypes); - } else if (RegisteredTypes.isSubTypeOf(supers, Application.class)) { + } else if (RegisteredTypes.isAnyTypeAssignableFrom(supers, Application.class)) { spec = createEntitySpecFromServicesBlock(planYaml, loader, encounteredTypes, true); - } else if (RegisteredTypes.isSubTypeOf(supers, Entity.class)) { + } else if (RegisteredTypes.isAnyTypeAssignableFrom(supers, Entity.class)) { spec = createEntitySpecFromServicesBlock(planYaml, loader, encounteredTypes, false); } else { - // try any of them??? - throw new IllegalStateException("Cannot detect spec type from "+item.getSuperTypes()+" for "+item+"\n"+planYaml); } + if (expectedType!=null && !expectedType.isAssignableFrom(spec.getType())) { + throw new IllegalStateException("Creating spec from "+item+", got "+spec.getType()+" which is incompatible with expected "+expectedType); + } ((AbstractBrooklynObjectSpec<?, ?>)spec).catalogItemId(item.getId()); @@ -127,9 +131,8 @@ class CampResolver { if (instantiator instanceof AssemblyTemplateSpecInstantiator) { EntitySpec<? extends Application> appSpec = ((AssemblyTemplateSpecInstantiator)instantiator).createApplicationSpec(at, camp, loader, encounteredTypes); - if (!isApplication && EntityManagementUtils.canPromoteChildrenInWrappedApplication(appSpec) && appSpec.getChildren().size()==1) { - CampInternalUtils.resetSpecIfTemplateHasNoExplicitParameters(at, appSpec); - EntitySpec<?> childSpec = appSpec.getChildren().get(0); + if (!isApplication && EntityManagementUtils.canPromoteChildrenInWrappedApplication(appSpec)) { + EntitySpec<?> childSpec = Iterables.getOnlyElement(appSpec.getChildren()); EntityManagementUtils.mergeWrapperParentSpecToChildEntity(appSpec, childSpec); return childSpec; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java index f287fec..afeba41 100644 --- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java +++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java @@ -25,7 +25,7 @@ import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan; import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; -import org.apache.brooklyn.core.typereg.AbstractCustomImplementationPlan; +import org.apache.brooklyn.core.typereg.AbstractFormatSpecificTypeImplementationPlan; import org.apache.brooklyn.core.typereg.AbstractTypePlanTransformer; import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan; import org.apache.brooklyn.core.typereg.RegisteredTypes; @@ -47,7 +47,7 @@ public class CampTypePlanTransformer extends AbstractTypePlanTransformer { @Override protected double scoreForNullFormat(Object planData, RegisteredType type, RegisteredTypeLoadingContext context) { - Maybe<Map<Object, Object>> plan = RegisteredTypes.getAsYamlMap(planData); + Maybe<Map<?,?>> plan = RegisteredTypes.getAsYamlMap(planData); if (plan.isAbsent()) return 0; if (plan.get().containsKey("services")) return 0.8; if (plan.get().containsKey("type")) return 0.4; @@ -87,7 +87,7 @@ public class CampTypePlanTransformer extends AbstractTypePlanTransformer { return null; } - public static class CampTypeImplementationPlan extends AbstractCustomImplementationPlan<String> { + public static class CampTypeImplementationPlan extends AbstractFormatSpecificTypeImplementationPlan<String> { public CampTypeImplementationPlan(TypeImplementationPlan otherPlan) { super(FORMATS.get(0), String.class, otherPlan); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java index 215fd1a..0a1b232 100644 --- a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java +++ b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java @@ -241,17 +241,17 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest { for (RegisteredType item: items) { Object spec = types.createSpec(item, null, null); int match = 0; - if (RegisteredTypes.isSubTypeOf(item, Entity.class)) { + if (RegisteredTypes.isAssignableFrom(item, Entity.class)) { assertTrue(spec instanceof EntitySpec, "Not an EntitySpec: " + spec); BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType()); match++; } - if (RegisteredTypes.isSubTypeOf(item, Policy.class)) { + if (RegisteredTypes.isAssignableFrom(item, Policy.class)) { assertTrue(spec instanceof PolicySpec, "Not a PolicySpec: " + spec); BrooklynTypes.getDefinedBrooklynType(((PolicySpec<?>)spec).getType()); match++; } - if (RegisteredTypes.isSubTypeOf(item, Location.class)) { + if (RegisteredTypes.isAssignableFrom(item, Location.class)) { assertTrue(spec instanceof LocationSpec, "Not a LocationSpec: " + spec); BrooklynTypes.getDefinedBrooklynType(((LocationSpec<?>)spec).getType()); match++; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java index 52d48e4..7fa8896 100644 --- a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java +++ b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java @@ -111,7 +111,7 @@ public class CatalogYamlLocationTest extends AbstractYamlTest { private void assertAdded(String symbolicName, String expectedJavaType) { RegisteredType item = mgmt().getTypeRegistry().get(symbolicName, TEST_VERSION); assertEquals(item.getSymbolicName(), symbolicName); - Assert.assertTrue(RegisteredTypes.isSubTypeOf(item, Location.class), "Expected Location, not "+item.getSuperTypes()); + Assert.assertTrue(RegisteredTypes.isAssignableFrom(item, Location.class), "Expected Location, not "+item.getSuperTypes()); assertEquals(countCatalogLocations(), 1); // Item added to catalog should automatically be available in location registry http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/TestAppAssemblyInstantiator.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/TestAppAssemblyInstantiator.java b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/TestAppAssemblyInstantiator.java index 6f9de6e..91dfb0e 100644 --- a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/TestAppAssemblyInstantiator.java +++ b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/TestAppAssemblyInstantiator.java @@ -62,10 +62,6 @@ public class TestAppAssemblyInstantiator extends BasicAssemblyTemplateInstantiat } @Override - public EntitySpec<? extends Application> createApplicationSpec(AssemblyTemplate template, CampPlatform platform, BrooklynClassLoadingContext loader) { - return createApplicationSpec(template, platform, loader, MutableSet.<String>of()); - } - @Override public EntitySpec<? extends Application> createApplicationSpec(AssemblyTemplate template, CampPlatform platform, BrooklynClassLoadingContext loader, Set<String> encounteredCatalogTypes) { EntitySpec<TestApplication> app = EntitySpec.create(TestApplication.class) http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java index b389f9c..c2ec4a5 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java @@ -23,12 +23,20 @@ import java.util.Random; public class Identifiers { private static Random random = new Random(); - + + /** @see #JAVA_GOOD_PACKAGE_OR_CLASS_REGEX */ public static final String JAVA_GOOD_START_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_"; + /** @see #JAVA_GOOD_PACKAGE_OR_CLASS_REGEX */ public static final String JAVA_GOOD_NONSTART_CHARS = JAVA_GOOD_START_CHARS+"1234567890"; - public static final String JAVA_SEGMENT_REGEX = "["+JAVA_GOOD_START_CHARS+"]"+"["+JAVA_GOOD_NONSTART_CHARS+"]*"; - public static final String JAVA_PACKAGE_OR_CLASS_REGEX = "("+JAVA_SEGMENT_REGEX+"\\."+")*"+JAVA_SEGMENT_REGEX; - public static final String JAVA_BINARY_REGEX = JAVA_PACKAGE_OR_CLASS_REGEX+"(\\$["+JAVA_GOOD_NONSTART_CHARS+"]+)*"; + /** @see #JAVA_GOOD_PACKAGE_OR_CLASS_REGEX */ + public static final String JAVA_GOOD_SEGMENT_REGEX = "["+JAVA_GOOD_START_CHARS+"]"+"["+JAVA_GOOD_NONSTART_CHARS+"]*"; + /** regex for a java package or class name using "good" chars, that is no accents or funny unicodes. + * see http://docs.oracle.com/javase/specs/jls/se7/html/jls-3.html#jls-3.8 for the full set supported by the spec; + * but it's rare to deviate from this subset and it causes problems when charsets aren't respected (tsk tsk but not uncommon!). + * our use cases so far only require testing for "good" names. */ + public static final String JAVA_GOOD_PACKAGE_OR_CLASS_REGEX = "("+JAVA_GOOD_SEGMENT_REGEX+"\\."+")*"+JAVA_GOOD_SEGMENT_REGEX; + /** as {@link #JAVA_GOOD_PACKAGE_OR_CLASS_REGEX} but allowing a dollar sign inside a class name (e.g. Foo$1) */ + public static final String JAVA_GOOD_BINARY_REGEX = JAVA_GOOD_PACKAGE_OR_CLASS_REGEX+"(\\$["+JAVA_GOOD_NONSTART_CHARS+"]+)*"; public static final String JAVA_GENERATED_IDENTIFIER_START_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; public static final String JAVA_GENERATED_IDENTIFIERNONSTART_CHARS = JAVA_GENERATED_IDENTIFIER_START_CHARS+"1234567890"; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/db024f4e/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java index 8f4463f..7b4f999 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java @@ -88,8 +88,15 @@ public class IdentifiersTest { @Test public void testJavaClassRegex() { - Assert.assertTrue("foo".matches(Identifiers.JAVA_BINARY_REGEX)); - Assert.assertTrue("foo.bar.Baz$1".matches(Identifiers.JAVA_BINARY_REGEX)); + Assert.assertTrue("foo".matches(Identifiers.JAVA_GOOD_SEGMENT_REGEX)); + Assert.assertFalse("foo.bar.Baz".matches(Identifiers.JAVA_GOOD_SEGMENT_REGEX)); + + Assert.assertTrue("foo".matches(Identifiers.JAVA_GOOD_PACKAGE_OR_CLASS_REGEX)); + Assert.assertTrue("foo.bar.Baz".matches(Identifiers.JAVA_GOOD_PACKAGE_OR_CLASS_REGEX)); + Assert.assertFalse("foo.bar.Baz$1".matches(Identifiers.JAVA_GOOD_PACKAGE_OR_CLASS_REGEX)); + + Assert.assertTrue("foo".matches(Identifiers.JAVA_GOOD_BINARY_REGEX)); + Assert.assertTrue("foo.bar.Baz$1".matches(Identifiers.JAVA_GOOD_BINARY_REGEX)); } }
