diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java index 265813a370..1d3498258b 100644 --- a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java +++ b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java @@ -56,6 +56,9 @@ public interface BrooklynObject extends Identifiable, Configurable { *

* In some cases this may be set heuristically from context and so may not be accurate. * Callers can set an explicit catalog item ID if inferencing is not correct. + *

+ * This should conform to OSGi specs for symbolic_name:version + * but weaker semantics are usually allowed so long as neither segment contains a : or whitespace. */ String getCatalogItemId(); 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 7e9b56279b..abc2e8ea87 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 @@ -30,7 +30,6 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.api.framework.FrameworkLookup; import org.apache.brooklyn.api.location.LocationSpec; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext; @@ -39,8 +38,6 @@ import org.apache.brooklyn.camp.brooklyn.BrooklynCampConstants; import org.apache.brooklyn.camp.brooklyn.BrooklynCampReservedKeys; import org.apache.brooklyn.camp.brooklyn.spi.creation.service.CampServiceSpecResolver; -import org.apache.brooklyn.camp.brooklyn.spi.creation.service.ServiceTypeResolver; -import org.apache.brooklyn.camp.brooklyn.spi.creation.service.ServiceTypeResolverAdaptor; import org.apache.brooklyn.camp.spi.AbstractResource; import org.apache.brooklyn.camp.spi.ApplicationComponentTemplate; import org.apache.brooklyn.camp.spi.AssemblyTemplate; @@ -79,10 +76,9 @@ import com.google.common.collect.Maps; /** - * This generates instances of a template resolver that use a {@link ServiceTypeResolver} + * This generates instances of a template resolver that use a {@link EntitySpecResolver} * to parse the {@code serviceType} line in the template. */ -@SuppressWarnings("deprecation") // Because of ServiceTypeResolver public class BrooklynComponentTemplateResolver { private static final Logger log = LoggerFactory.getLogger(BrooklynComponentTemplateResolver.class); @@ -197,10 +193,7 @@ public EntitySpec resolveSpec(Set encounteredRegis private List getServiceTypeResolverOverrides() { List overrides = new ArrayList<>(); - Iterable loader = FrameworkLookup.lookupAll(ServiceTypeResolver.class, mgmt.getCatalogClassLoader()); - for (ServiceTypeResolver resolver : loader) { - overrides.add(new ServiceTypeResolverAdaptor(this, resolver)); - } + // none for now -- previously supported ServiceTypeResolver service return overrides; } diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/BrooklynServiceTypeResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/BrooklynServiceTypeResolver.java deleted file mode 100644 index f9c07e4e2a..0000000000 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/BrooklynServiceTypeResolver.java +++ /dev/null @@ -1,78 +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.camp.brooklyn.spi.creation.service; - -import javax.annotation.Nullable; - -import org.apache.brooklyn.api.catalog.CatalogItem; -import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.api.mgmt.ManagementContext; -import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynComponentTemplateResolver; -import org.apache.brooklyn.camp.spi.PlatformComponentTemplate; -import org.apache.brooklyn.core.catalog.internal.CatalogUtils; -import org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider; -import org.apache.brooklyn.core.resolve.entity.AbstractEntitySpecResolver; -import org.apache.brooklyn.util.text.Strings; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * This converts {@link PlatformComponentTemplate} instances whose type is prefixed {@code brooklyn:} - * to Brooklyn {@link EntitySpec} instances. - * - * @deprecated since 0.9.0, use {@link AbstractEntitySpecResolver} instead - */ -@Deprecated -public class BrooklynServiceTypeResolver implements ServiceTypeResolver { - - @SuppressWarnings("unused") - private static final Logger LOG = LoggerFactory.getLogger(ServiceTypeResolver.class); - - public BrooklynServiceTypeResolver() { - } - - @Override - public String getTypePrefix() { return DEFAULT_TYPE_PREFIX; } - - @Override - public String getBrooklynType(String serviceType) { - return Strings.removeFromStart(serviceType, getTypePrefix() + ":").trim(); - } - - @Nullable - @Override - public CatalogItem> getCatalogItem(BrooklynComponentTemplateResolver resolver, String serviceType) { - String type = getBrooklynType(serviceType); - if (type != null) { - return getCatalogItemImpl(resolver.getManagementContext(), type); - } else { - return null; - } - } - - @Override - public void decorateSpec(BrooklynComponentTemplateResolver resolver, EntitySpec spec) { - } - - protected CatalogItem> getCatalogItemImpl(ManagementContext mgmt, String brooklynType) { - brooklynType = DeserializingClassRenamesProvider.INSTANCE.findMappedName(brooklynType); - return CatalogUtils.getCatalogItemOptionalVersion(mgmt, Entity.class, brooklynType); - } -} diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolver.java deleted file mode 100644 index 14f855a605..0000000000 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolver.java +++ /dev/null @@ -1,77 +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.camp.brooklyn.spi.creation.service; - -import java.util.ServiceLoader; - -import org.apache.brooklyn.api.catalog.CatalogItem; -import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynComponentTemplateResolver; -import org.apache.brooklyn.core.resolve.entity.EntitySpecResolver; - -/** - * Resolves and decorates {@link EntitySpec entity specifications} based on the {@code serviceType} in a template. - *

- * The {@link #getTypePrefix()} method returns a string that should match the beginning of the - * service type. The resolver implementation will use the rest of the service type information - * to create and decorate an approprate {@link EntitySpec entity}. - *

- * The resolvers are loaded using the {@link ServiceLoader} mechanism, allowing external libraries - * to add extra service type implementations that will be picked up at runtime. - * - * @see BrooklynServiceTypeResolver - * @see ChefServiceTypeResolver - * - * @deprecated since 0.9.0, {@link EntitySpecResolver} instead. - */ -@Deprecated -public interface ServiceTypeResolver { - - String DEFAULT_TYPE_PREFIX = "brooklyn"; - - /** - * The service type prefix the resolver is responsible for. - */ - String getTypePrefix(); - - /** - * The name of the Java type that Brooklyn will instantiate to create the - * service. This can be generated from parts of the service type information - * or may be a fixed value. - */ - String getBrooklynType(String serviceType); - - /** - * Returns the {@link CatalogItem} if there is one for the given type. - *

- * If no type, callers should fall back to default classloading. - */ - CatalogItem> getCatalogItem(BrooklynComponentTemplateResolver resolver, String serviceType); - - /** - * Takes the provided {@link EntitySpec} and decorates it appropriately for the service type. - *

- * This includes setting configuration and adding policies, enrichers and initializers. - * - * @see BrooklynServiceTypeResolver#decorateSpec(BrooklynComponentTemplateResolver, EntitySpec) - */ - void decorateSpec(BrooklynComponentTemplateResolver resolver, EntitySpec spec); - -} diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverAdaptor.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverAdaptor.java deleted file mode 100644 index d4cf6e955c..0000000000 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverAdaptor.java +++ /dev/null @@ -1,70 +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.camp.brooklyn.spi.creation.service; - -import java.util.Set; - -import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext; -import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynComponentTemplateResolver; -import org.apache.brooklyn.core.resolve.entity.AbstractEntitySpecResolver; -import org.apache.brooklyn.core.resolve.entity.EntitySpecResolver; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.base.Splitter; - -@SuppressWarnings("deprecation") -public class ServiceTypeResolverAdaptor extends AbstractEntitySpecResolver { - private static final Logger log = LoggerFactory.getLogger(ServiceTypeResolverAdaptor.class); - private ServiceTypeResolver serviceTypeResolver; - private BrooklynComponentTemplateResolver resolver; - - public ServiceTypeResolverAdaptor(BrooklynComponentTemplateResolver resolver, ServiceTypeResolver serviceTypeResolver) { - super(serviceTypeResolver.getTypePrefix()); - this.serviceTypeResolver = serviceTypeResolver; - this.resolver = resolver; - } - - @Override - public boolean accepts(String type, BrooklynClassLoadingContext loader) { - if (type.indexOf(':') != -1) { - String prefix = Splitter.on(":").splitToList(type).get(0); - return prefix.equals(serviceTypeResolver.getTypePrefix()); - } else { - return false; - } - } - - @SuppressWarnings({ "unchecked", "rawtypes" }) - @Override - public EntitySpec resolve(String type, BrooklynClassLoadingContext loader, Set encounteredTypes) { - // Assume this is interface! Only known implementation is DockerServiceTypeResolver. - String brooklynType = serviceTypeResolver.getBrooklynType(type); - Class javaType = loader.loadClass(brooklynType, Entity.class); - if (!javaType.isInterface()) { - log.warn("Using " + ServiceTypeResolver.class.getSimpleName() + " with a non-interface type - this usage is not supported. Use " + EntitySpecResolver.class.getSimpleName() + " instead."); - } - EntitySpec spec = EntitySpec.create((Class)javaType); - serviceTypeResolver.decorateSpec(resolver, spec); - return spec; - } - -} diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverTest.java deleted file mode 100644 index cbaccf050e..0000000000 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/ServiceTypeResolverTest.java +++ /dev/null @@ -1,39 +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.camp.brooklyn.spi.creation.service; - -import static org.testng.Assert.assertEquals; - -import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest; -import org.apache.brooklyn.entity.stock.BasicEntity; -import org.testng.annotations.Test; - - -public class ServiceTypeResolverTest extends AbstractYamlTest { - - @Test - public void testAddCatalogItemVerySimple() throws Exception { - EntitySpec spec = createAppEntitySpec( - "services:", - "- type: \"test-resolver:" + BasicEntity.class.getName() + "\""); - assertEquals(spec.getChildren().get(0).getFlags().get("resolver"), TestServiceTypeResolver.class); - } - -} diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/TestServiceTypeResolver.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/TestServiceTypeResolver.java deleted file mode 100644 index 7fd6d8a91e..0000000000 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/TestServiceTypeResolver.java +++ /dev/null @@ -1,54 +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.camp.brooklyn.spi.creation.service; - -import org.apache.brooklyn.api.catalog.CatalogItem; -import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntitySpec; -import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynComponentTemplateResolver; -import org.apache.brooklyn.core.catalog.internal.CatalogUtils; -import org.apache.brooklyn.util.text.Strings; - -@SuppressWarnings("deprecation") -public class TestServiceTypeResolver implements ServiceTypeResolver { - - private static final String PREFIX = "test-resolver"; - - @Override - public String getTypePrefix() { - return PREFIX; - } - - @Override - public String getBrooklynType(String serviceType) { - return Strings.removeFromStart(serviceType, PREFIX + ":"); - } - - @SuppressWarnings("unchecked") - @Override - public CatalogItem> getCatalogItem(BrooklynComponentTemplateResolver resolver, String serviceType) { - return (CatalogItem>) CatalogUtils.getCatalogItemOptionalVersion(resolver.getManagementContext(), getBrooklynType(serviceType)); - } - - @Override - public void decorateSpec(BrooklynComponentTemplateResolver resolver, EntitySpec spec) { - spec.configure("resolver", TestServiceTypeResolver.class); - } - -} diff --git a/camp/camp-brooklyn/src/test/resources/META-INF/services/org.apache.brooklyn.camp.brooklyn.spi.creation.service.ServiceTypeResolver b/camp/camp-brooklyn/src/test/resources/META-INF/services/org.apache.brooklyn.camp.brooklyn.spi.creation.service.ServiceTypeResolver deleted file mode 100644 index 3bba7fb58b..0000000000 --- a/camp/camp-brooklyn/src/test/resources/META-INF/services/org.apache.brooklyn.camp.brooklyn.spi.creation.service.ServiceTypeResolver +++ /dev/null @@ -1,19 +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. -# -org.apache.brooklyn.camp.brooklyn.spi.creation.service.TestServiceTypeResolver \ No newline at end of file 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 5715b66def..3817665c44 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 @@ -46,6 +46,7 @@ import org.apache.brooklyn.core.mgmt.internal.CampYamlParser; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.typereg.BrooklynTypePlanTransformer; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; @@ -601,14 +602,34 @@ private void collectCatalogItems(String sourceYaml, Map itemMetadata, List< // if symname not set, infer from: id, then name, then item id, then item name if (Strings.isBlank(symbolicName)) { if (Strings.isNonBlank(id)) { - if (CatalogUtils.looksLikeVersionedId(id)) { + if (RegisteredTypeNaming.isGoodTypeColonVersion(id)) { symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id); + } else if (CatalogUtils.looksLikeVersionedId(id)) { + // use of above method is deprecated in 0.12; this block can be removed in 0.13 + log.warn("Discouraged version syntax in id '"+id+"'; version should comply with OSGi specs (#.#.#.qualifier or portion) or specify symbolic name and version explicitly"); + symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id); + } else if (RegisteredTypeNaming.isUsableTypeColonVersion(id)) { + log.warn("Deprecated type naming syntax in id '"+id+"'; colons not allowed in type name as it is used to indicate version"); + // deprecated in 0.12; from 0.13 this can change to treat part after the colon as version, also see line to set version below + // (may optionally warn or disallow if we want to require OSGi versions) + // symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id); + symbolicName = id; } else { symbolicName = id; } } else if (Strings.isNonBlank(name)) { - if (CatalogUtils.looksLikeVersionedId(name)) { + if (RegisteredTypeNaming.isGoodTypeColonVersion(name)) { + log.warn("Deprecated use of 'name' key to define '"+name+"'; version should be specified within 'id' key or with 'version' key, not this tag"); + // deprecated in 0.12; remove in 0.13 + symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(name); + } else if (CatalogUtils.looksLikeVersionedId(name)) { + log.warn("Deprecated use of 'name' key to define '"+name+"'; version should be specified within 'id' key or with 'version' key, not this tag"); + // deprecated in 0.12; remove in 0.13 symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(name); + } else if (RegisteredTypeNaming.isUsableTypeColonVersion(name)) { + log.warn("Deprecated type naming syntax in id '"+id+"'; colons not allowed in type name as it is used to indicate version"); + // deprecated in 0.12; throw error if we want in 0.13 + symbolicName = name; } else { symbolicName = name; } @@ -624,18 +645,35 @@ private void collectCatalogItems(String sourceYaml, Map itemMetadata, List< } } + String versionFromId = null; + if (RegisteredTypeNaming.isGoodTypeColonVersion(id)) { + versionFromId = CatalogUtils.getVersionFromVersionedId(id); + } else if (CatalogUtils.looksLikeVersionedId(id)) { + log.warn("Discouraged version syntax in id '"+id+"'; version should comply with OSGi specs (#.#.#.qualifier or portion) or specify symbolic name and version explicitly"); + // remove in 0.13 + versionFromId = CatalogUtils.getVersionFromVersionedId(id); + } else if (RegisteredTypeNaming.isUsableTypeColonVersion(id)) { + // deprecated in 0.12, with warning above; from 0.13 this can be uncommented to treat part after the colon as version + // (may optionally warn or disallow if we want to require OSGi versions) + // if comparable section above is changed, change this to: + // versionFromId = CatalogUtils.getVersionFromVersionedId(id); + } + // if version not set, infer from: id, then from name, then item version - if (CatalogUtils.looksLikeVersionedId(id)) { - String versionFromId = CatalogUtils.getVersionFromVersionedId(id); - if (versionFromId != null && Strings.isNonBlank(version) && !versionFromId.equals(version)) { + if (versionFromId!=null) { + if (Strings.isNonBlank(version) && !versionFromId.equals(version)) { throw new IllegalArgumentException("Discrepency between version set in id " + versionFromId + " and version property " + version); } version = versionFromId; } + if (Strings.isBlank(version)) { if (CatalogUtils.looksLikeVersionedId(name)) { + // deprecated in 0.12, remove in 0.13 + log.warn("Deprecated use of 'name' key to define '"+name+"'; version should be specified within 'id' key or with 'version' key, not this tag"); version = CatalogUtils.getVersionFromVersionedId(name); - } else if (Strings.isBlank(version)) { + } + if (Strings.isBlank(version)) { version = setFromItemIfUnset(version, itemAsMap, "version"); version = setFromItemIfUnset(version, itemAsMap, "template_version"); if (version==null) { @@ -855,24 +893,50 @@ private boolean attemptType(String key, CatalogItemType candidateCiType) { } // first look in collected items, if a key is given String type = (String) item.get("type"); - String version = null; - if (CatalogUtils.looksLikeVersionedId(type)) { - version = CatalogUtils.getVersionFromVersionedId(type); - type = CatalogUtils.getSymbolicNameFromVersionedId(type); - } + if (type!=null && key!=null) { for (CatalogItemDtoAbstract candidate: itemsDefinedSoFar) { if (candidateCiType == candidate.getCatalogItemType() && (type.equals(candidate.getSymbolicName()) || type.equals(candidate.getId()))) { - if (version==null || version.equals(candidate.getVersion())) { - // matched - exit - catalogItemType = candidateCiType; - planYaml = candidateYaml; - resolved = true; - return true; + // matched - exit + catalogItemType = candidateCiType; + planYaml = candidateYaml; + resolved = true; + return true; + } + } + } + { + // legacy routine; should be the same as above code added in 0.12 because: + // if type is symbolic_name, the type will match above, and version will be null so any version allowed to match + // if type is symbolic_name:version, the id will match, and the version will also have to match + // SHOULD NEVER NEED THIS - remove during or after 0.13 + String typeWithId = type; + String version = null; + if (CatalogUtils.looksLikeVersionedId(type)) { + version = CatalogUtils.getVersionFromVersionedId(type); + type = CatalogUtils.getSymbolicNameFromVersionedId(type); + } + if (type!=null && key!=null) { + for (CatalogItemDtoAbstract candidate: itemsDefinedSoFar) { + if (candidateCiType == candidate.getCatalogItemType() && + (type.equals(candidate.getSymbolicName()) || type.equals(candidate.getId()))) { + if (version==null || version.equals(candidate.getVersion())) { + log.error("Lookup of '"+type+"' version '"+version+"' only worked using legacy routines; please advise Brooklyn community so they understand why"); + // matched - exit + catalogItemType = candidateCiType; + planYaml = candidateYaml; + resolved = true; + return true; + } } } } + + type = typeWithId; + // above line is a change to behaviour; previously we proceeded below with the version dropped in code above; + // but that seems like a bug as the code below will have ignored version. + // likely this means we are now stricter about loading things that reference new versions, but correctly so. } // then try parsing plan - this will use loader diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java index 4644d8d901..3f69e677ad 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.core.mgmt.rebind.BasicCatalogItemRebindSupport; import org.apache.brooklyn.core.objs.AbstractBrooklynObject; import org.apache.brooklyn.core.relations.EmptyRelationSupport; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; @@ -386,6 +387,16 @@ protected void setTags(Set tags) { /** * Parses an instance of CatalogLibrariesDto from the given List. Expects the list entries * to be either Strings or Maps of String -> String. Will skip items that are not. + *

+ * If a string is supplied, this tries heuristically to identify whether a reference is a bundle or a URL, as follows: + * - if the string contains a slash, it is treated as a URL (or classpath reference), e.g. /file.txt; + * - if the string is {@link RegisteredTypeNaming#isGoodTypeColonVersion(String)} with an OSGi version it is treated as a bundle, e.g. file:1; + * - if the string is ambiguous (has a single colon) a warning is given, + * and typically it is treated as a URL because OSGi versions are needed here, e.g. file:v1 is a URL, + * but for a transitional period (possibly ending in 0.13 as warning is introduced in 0.12) for compatibility with previous versions, + * versions starting with a number trigger bundle resolution, e.g. file:1.txt is a bundle for now + * (but in all these cases warnings are logged) + * - otherwise (multiple colons, or no colons) it is treated like a URL */ public static Collection parseLibraries(Collection possibleLibraries) { Collection dto = MutableList.of(); @@ -405,16 +416,28 @@ public static Collection parseLibraries(Collection possibleLib //Infer reference type (heuristically) if (inlineRef.contains("/") || inlineRef.contains("\\")) { - //looks like an url/file path + //looks like an url/file path (note these chars now formally disallowed in type names) name = null; version = null; url = inlineRef; + } else if (RegisteredTypeNaming.isGoodTypeColonVersion(inlineRef)) { + //looks like a name+version ref + name = CatalogUtils.getSymbolicNameFromVersionedId(inlineRef); + version = CatalogUtils.getVersionFromVersionedId(inlineRef); + url = null; } else if (CatalogUtils.looksLikeVersionedId(inlineRef)) { + LOG.warn("Reference to library "+inlineRef+" is being treated as type but deprecated version syntax " + + "means in subsequent versions it will be treated as a URL."); //looks like a name+version ref name = CatalogUtils.getSymbolicNameFromVersionedId(inlineRef); version = CatalogUtils.getVersionFromVersionedId(inlineRef); url = null; } else { + if (RegisteredTypeNaming.isUsableTypeColonVersion(inlineRef)) { + LOG.warn("Ambiguous library reference "+inlineRef+" is being treated as a URL even though" + + " it looks a bit like a bundle with a non-osgi-version. Use strict OSGi versions to treat as a bundle. " + + "To suppress this message and force URL use a slash in the reference "); + } //assume it to be relative url name = null; version = null; 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 47dab6d5f7..bc744a0980 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 @@ -46,6 +46,7 @@ import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.util.collections.MutableList; @@ -232,6 +233,11 @@ public static void logDebugOrTraceIfRebinding(Logger log, String message, Object log.debug(message, args); } + /** @deprecated since 0.12.0 - the "version starts with number" test this does is hokey; use + * either {@link RegisteredTypeNaming#isUsableTypeColonVersion(String)} for weak enforcement + * or {@link RegisteredTypeNaming#isGoodTypeColonVersion(String)} for OSGi enforcement. */ + // several references, but all deprecated, most safe to remove after a cycle or two and bad verisons have been warned + @Deprecated public static boolean looksLikeVersionedId(String versionedId) { if (versionedId==null) return false; int fi = versionedId.indexOf(VERSION_DELIMITER); @@ -248,16 +254,14 @@ public static boolean looksLikeVersionedId(String versionedId) { // e.g. foo:1 or foo:1.1 or foo:1_SNAPSHOT all supported, but not e.g. foo:bar (or chef:cookbook or docker:my/image) return false; } + if (!RegisteredTypeNaming.isUsableTypeColonVersion(versionedId)) { + // arguments that contain / or whitespace will pass here but calling code will likely be changed not to support it + log.warn("Reference '"+versionedId+"' is being treated as a versioned type but it " + + "contains deprecated characters (slashes or whitespace); likely to be unsupported in future versions."); + } return true; } - /** @deprecated since 0.9.0 use {@link #getSymbolicNameFromVersionedId(String)} */ - // all uses removed - @Deprecated - public static String getIdFromVersionedId(String versionedId) { - return getSymbolicNameFromVersionedId(versionedId); - } - public static String getSymbolicNameFromVersionedId(String versionedId) { if (versionedId == null) return null; int versionDelimiterPos = versionedId.lastIndexOf(VERSION_DELIMITER); @@ -284,7 +288,7 @@ public static String getVersionedId(String id, String version) { } /** @deprecated since 0.9.0 use {@link BrooklynTypeRegistry#get(String, org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind, Class)} */ - // only a handful of items remaining, and those require a CatalogItem + // only a handful of items remaining, requiring a CatalogItem: two in REST, one in rebind, and one in ClassLoaderUtils @Deprecated public static CatalogItem getCatalogItemOptionalVersion(ManagementContext mgmt, String versionedId) { if (versionedId == null) return null; @@ -305,7 +309,7 @@ public static boolean isBestVersion(ManagementContext mgmt, CatalogItem ite } /** @deprecated since 0.9.0 use {@link BrooklynTypeRegistry#get(String, org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind, Class)} */ - // only a handful of items remaining, and those require a CatalogItem + // only one item left, in deprecated service resolver @Deprecated public static CatalogItem getCatalogItemOptionalVersion(ManagementContext mgmt, Class type, String versionedId) { if (looksLikeVersionedId(versionedId)) { diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java index e92a774dbe..a80b14a00e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java @@ -99,6 +99,7 @@ import org.apache.brooklyn.core.objs.proxy.InternalPolicyFactory; import org.apache.brooklyn.core.policy.AbstractPolicy; import org.apache.brooklyn.core.typereg.BasicManagedBundle; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; @@ -1086,7 +1087,9 @@ protected LoadedClass load(Class bTyp // See https://issues.apache.org/jira/browse/BROOKLYN-149 // This is a dangling reference to the catalog item (which will have been logged by lookupCatalogItem). // Try loading as any version. - if (CatalogUtils.looksLikeVersionedId(catalogItemId)) { + if (RegisteredTypeNaming.isUsableTypeColonVersion(catalogItemId) || + // included through 0.12 so legacy type names are accepted (with warning) + CatalogUtils.looksLikeVersionedId(catalogItemId)) { String symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(catalogItemId); catalogItem = rebindContext.lookup().lookupCatalogItem(symbolicName); 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 0c2c87ba2a..468664b842 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 @@ -153,7 +153,9 @@ public RegisteredType get(String symbolicNameWithOptionalVersion, RegisteredType @Override public Maybe getMaybe(String symbolicNameWithOptionalVersion, RegisteredTypeLoadingContext context) { Maybe r1 = null; - if (CatalogUtils.looksLikeVersionedId(symbolicNameWithOptionalVersion)) { + if (RegisteredTypeNaming.isUsableTypeColonVersion(symbolicNameWithOptionalVersion) || + // included through 0.12 so legacy type names are accepted (with warning) + CatalogUtils.looksLikeVersionedId(symbolicNameWithOptionalVersion)) { String symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(symbolicNameWithOptionalVersion); String version = CatalogUtils.getVersionFromVersionedId(symbolicNameWithOptionalVersion); r1 = getSingle(symbolicName, version, context); diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java new file mode 100644 index 0000000000..8e11ae976f --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java @@ -0,0 +1,112 @@ +/* + * 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; + +/** + * Methods for testing validity of names and decomposing them. + * Mostly based on OSGi, specifically sections 1.3.2 and 3.2.5 of + * osgi.core-4.3.0 spec (https://www.osgi.org/release-4-version-4-3-download/). */ +public class RegisteredTypeNaming { + + private static final String USABLE_REGEX = "[^:\\s/\\\\]+"; + + private final static String DOT = "\\."; + + public final static String OSGI_TOKEN_CHARS = "A-Za-z0-9_-"; + public final static String OSGI_TOKEN_REGEX = "[" + OSGI_TOKEN_CHARS + "]+"; + public final static String OSGI_SYMBOLIC_NAME_REGEX = OSGI_TOKEN_REGEX + "(" + DOT + OSGI_TOKEN_REGEX + ")*"; + public final static String NUMBER = "[0-9]+"; + public final static String QUALIFIER = OSGI_TOKEN_REGEX; + public final static String VERSION_REGEX = + NUMBER + + "(" + DOT + NUMBER + + "(" + DOT + NUMBER + + "(" + DOT + QUALIFIER + + ")?" + + ")?" + + ")?"; + + private static boolean isUsable(String candidate) { + return candidate!=null && candidate.matches(USABLE_REGEX); + } + + /** + * For type names we currently work with any non-empty string that does not contain + * a ':' or whitespace or forward slash or backslash. + * However we discourage things that are not OSGi symbolic names; + * see {@link #isGoodTypeName(String)}. + * In some places (eg bundles) the use of OSGi symbolic names may be enforced. + */ + public static boolean isUsableTypeName(String candidate) { + return isUsable(candidate); + } + + /** + * We recommend type names be OSGi symbolic names, such as: + * + * com.acme-group.1-Virtual-Machine + * + * Note that this is more permissive than Java, allowing hyphens and + * allowing segments to start with numbers. + * However it is also more restrictive: OSGi does not allow + * accented characters or most punctuation. Only hyphens and underscores are allowed + * in segment names, and periods are allowed only as segment separators. + */ + public static boolean isGoodTypeName(String candidate) { + return isUsable(candidate) && candidate.matches(OSGI_SYMBOLIC_NAME_REGEX); + } + + /** + * For versions we currently work with any non-empty string that does not contain a ':' or whitespace. + * However we discourage things that are not OSGi versions; see {@link #isOsgiLegalVersion(String)}. + * In some places (eg bundles) the use of OSGi version syntax may be enforced. + */ + public static boolean isUsableVersion(String candidate) { + return isUsable(candidate); + } + + /** True if the argument matches the OSGi version spec, of the form + * MAJOR.MINOR.POINT.QUALIFIER or part thereof (not ending in a period though), + * where the first three are whole numbers and the final piece is any valid OSGi token + * (something satisfying {@link #isGoodTypeName(String)} with no periods). + */ + public static boolean isOsgiLegalVersion(String candidate) { + return candidate!=null && candidate.matches(VERSION_REGEX); + } + + /** True if the argument has exactly one colon, and the part before + * satisfies {@link #isUsableTypeName(String)} and the part after {@link #isUsableVersion(String)}. */ + public static boolean isUsableTypeColonVersion(String candidate) { + // simplify regex, rather than decomposing and calling the methods referenced in the javadoc + return candidate!=null && candidate.matches(USABLE_REGEX + ":" + USABLE_REGEX); + } + + /** True if the argument has exactly one colon, and the part before + * satisfies {@link #isGoodTypeName(String)} and the part after + * {@link #isOsgiLegalVersion(String)}. */ + public static boolean isGoodTypeColonVersion(String candidate) { + if (candidate==null) return false; + int idx = candidate.indexOf(':'); + if (idx<=0) return false; + if (!isGoodTypeName(candidate.substring(0, idx))) return false; + if (!isOsgiLegalVersion(candidate.substring(idx+1))) return false; + return true; + } + +} diff --git a/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java index f6079398c0..a6be53d081 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java @@ -253,6 +253,17 @@ private Maybe loadClass(String name, LoaderDispatcher dispatcher, Stri if (entity != null && mgmt != null) { String catalogItemId = entity.getCatalogItemId(); if (catalogItemId != null) { +// RegisteredType type = mgmt.getTypeRegistry().get(catalogItemId); +// if (type != null) { +// BrooklynClassLoadingContextSequential loader = new BrooklynClassLoadingContextSequential(mgmt); +// loader.add(newClassLoadingContextForCatalogItems(mgmt, type.getId(), +// type.getContainingBundle() + type.getLibraries() ?); +// cls = dispatcher.tryLoadFrom(loader, className); +// if (cls.isPresent()) { +// return cls; +// } + // TODO prefer above to below, but need to reconcile item.searchPath with RegisteredType? + // or use entity search path ? CatalogItem item = CatalogUtils.getCatalogItemOptionalVersion(mgmt, catalogItemId); if (item != null) { BrooklynClassLoadingContextSequential loader = new BrooklynClassLoadingContextSequential(mgmt); @@ -262,6 +273,7 @@ private Maybe loadClass(String name, LoaderDispatcher dispatcher, Stri if (cls.isPresent()) { return cls; } + } else { log.warn("Entity " + entity + " refers to non-existent catalog item " + catalogItemId + ". Trying to load class " + name); diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java index 0ca2171bed..3c09bb9e09 100644 --- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java @@ -35,6 +35,7 @@ import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import com.google.common.base.Predicates; import com.google.common.collect.Iterables; @@ -54,19 +55,21 @@ public void tearDown() throws Exception { if (managementContext != null) Entities.destroyAll(managementContext); } + /** @deprecated since 0.12.0; replaced with {@link RegisteredTypeNaming} */ @Test - public void testParsingVersion() { - assertVersionParsesAs("foo:1", "foo", "1"); - assertVersionParsesAs("foo", null, null); - assertVersionParsesAs("foo:1.1", "foo", "1.1"); - assertVersionParsesAs("foo:1_SNAPSHOT", "foo", "1_SNAPSHOT"); - assertVersionParsesAs("foo:10.9.8_SNAPSHOT", "foo", "10.9.8_SNAPSHOT"); - assertVersionParsesAs("foo:bar", null, null); - assertVersionParsesAs("chef:cookbook", null, null); - assertVersionParsesAs("http://foo:8080", null, null); + @Deprecated + public void testLegacyParsingVersion() { + assertLegacyVersionParsesAs("foo:1", "foo", "1"); + assertLegacyVersionParsesAs("foo", null, null); + assertLegacyVersionParsesAs("foo:1.1", "foo", "1.1"); + assertLegacyVersionParsesAs("foo:1_SNAPSHOT", "foo", "1_SNAPSHOT"); + assertLegacyVersionParsesAs("foo:10.9.8_SNAPSHOT", "foo", "10.9.8_SNAPSHOT"); + assertLegacyVersionParsesAs("foo:bar", null, null); + assertLegacyVersionParsesAs("chef:cookbook", null, null); + assertLegacyVersionParsesAs("http://foo:8080", null, null); } - private static void assertVersionParsesAs(String versionedId, String id, String version) { + private static void assertLegacyVersionParsesAs(String versionedId, String id, String version) { if (version==null) { Assert.assertFalse(CatalogUtils.looksLikeVersionedId(versionedId)); } else { diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java b/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java new file mode 100644 index 0000000000..81b69bf029 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java @@ -0,0 +1,87 @@ +/* + * 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.testng.Assert; +import org.testng.annotations.Test; + +@Test +public class RegisteredTypeNamingTest { + + public void testNames() { + assertName("foo", true, true); + assertName("foo-bar", true, true); + assertName("a-package.1-foo-bar", true, true); + + assertName("", false, false); + assertName(null, false, false); + assertName("foo:1", false, false); + assertName("foo bar", false, false); + + assertName(".foo", true, false); + assertName("package..foo", true, false); + assertName("package..foo", true, false); + assertName("!$&", true, false); + } + + public void testVersions() { + assertVersion("1", true, true); + assertVersion("1.0.0", true, true); + assertVersion("1.0.0.SNAPSHOT", true, true); + + assertVersion("", false, false); + assertVersion(null, false, false); + assertVersion("1:1", false, false); + + assertVersion("1.SNAPSHOT", true, false); + assertVersion("1.0.0_SNAPSHOT", true, false); + assertVersion(".1", true, false); + assertVersion("v1", true, false); + assertVersion("!$&", true, false); + } + + public void testNameColonVersion() { + assertNameColonVersion("foo:1", true, true); + assertNameColonVersion("1:1", true, true); + assertNameColonVersion("a-package.1-foo-bar:1.0.0.SNAPSHOT_dev", true, true); + + assertNameColonVersion("", false, false); + assertNameColonVersion(null, false, false); + assertNameColonVersion("foo:", false, false); + assertNameColonVersion(":1", false, false); + + assertNameColonVersion("foo:1.SNAPSHOT", true, false); + assertNameColonVersion("foo:v1", true, false); + assertNameColonVersion("foo...bar!:1", true, false); + } + + private void assertName(String candidate, boolean isUsable, boolean isGood) { + Assert.assertEquals(RegisteredTypeNaming.isUsableTypeName(candidate), isUsable, "usable name '"+candidate+"'"); + Assert.assertEquals(RegisteredTypeNaming.isGoodTypeName(candidate), isGood, "good name '"+candidate+"'"); + } + private void assertVersion(String candidate, boolean isUsable, boolean isGood) { + Assert.assertEquals(RegisteredTypeNaming.isUsableVersion(candidate), isUsable, "usable version '"+candidate+"'"); + Assert.assertEquals(RegisteredTypeNaming.isOsgiLegalVersion(candidate), isGood, "good version '"+candidate+"'"); + } + private void assertNameColonVersion(String candidate, boolean isUsable, boolean isGood) { + Assert.assertEquals(RegisteredTypeNaming.isUsableTypeColonVersion(candidate), isUsable, "usable name:version '"+candidate+"'"); + Assert.assertEquals(RegisteredTypeNaming.isGoodTypeColonVersion(candidate), isGood, "good name:version '"+candidate+"'"); + } + +} diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java index fcbe671e72..8a15020d36 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/EntityTransformer.java @@ -32,6 +32,7 @@ import org.apache.brooklyn.api.objs.SpecParameter; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.render.RendererHints; +import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.rest.domain.EnricherConfigSummary; import org.apache.brooklyn.rest.domain.EntityConfigSummary; import org.apache.brooklyn.rest.domain.EntitySummary; @@ -90,19 +91,15 @@ public static EntitySummary entitySummary(Entity entity, UriBuilder ub) { .put("spec", URI.create(entityUri + "/spec")) ; - if (entity.getIconUrl()!=null) + if (RegisteredTypes.getIconUrl(entity)!=null) lb.put("iconUrl", URI.create(entityUri + "/icon")); if (entity.getCatalogItemId() != null) { String versionedId = entity.getCatalogItemId(); URI catalogUri; - if (CatalogUtils.looksLikeVersionedId(versionedId)) { - String symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(versionedId); - String version = CatalogUtils.getVersionFromVersionedId(versionedId); - catalogUri = serviceUriBuilder(ub, CatalogApi.class, "getEntity").build(symbolicName, version); - } else { - catalogUri = serviceUriBuilder(ub, CatalogApi.class, "getEntity_0_7_0").build(versionedId); - } + String symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(versionedId); + String version = CatalogUtils.getVersionFromVersionedId(versionedId); + catalogUri = serviceUriBuilder(ub, CatalogApi.class, "getEntity").build(symbolicName, version); lb.put("catalog", catalogUri); } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtils.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtils.java index 600c90f968..66467a10cd 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtils.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtils.java @@ -22,7 +22,6 @@ import static org.apache.brooklyn.rest.util.WebResourceUtils.notFound; import java.util.ArrayList; -import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -33,7 +32,6 @@ import javax.ws.rs.core.MediaType; import org.apache.brooklyn.api.catalog.BrooklynCatalog; -import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.entity.Application; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; @@ -46,9 +44,6 @@ import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent; import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent.Scope; import org.apache.brooklyn.config.ConfigKey; -import org.apache.brooklyn.core.catalog.CatalogPredicates; -import org.apache.brooklyn.core.catalog.internal.CatalogItemComparator; -import org.apache.brooklyn.core.catalog.internal.CatalogUtils; import org.apache.brooklyn.core.entity.Attributes; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityInternal; @@ -57,7 +52,6 @@ import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.StringAndArgument; import org.apache.brooklyn.core.objs.BrooklynTypes; -import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.enricher.stock.Enrichers; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.rest.domain.ApplicationSpec; @@ -75,9 +69,7 @@ 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.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -215,20 +207,6 @@ private class FindItemAndClass { @SuppressWarnings({ "unchecked" }) private FindItemAndClass inferFrom(String type) { RegisteredType item = mgmt.getTypeRegistry().get(type); - if (item==null) { - // deprecated attempt to load an item not in the type registry - - // although the method called was deprecated in 0.7.0, its use here was not warned until 0.9.0; - // therefore this behaviour should not be changed until after 0.9.0; - // at which point it should try a pojo load (see below) - item = getCatalogItemForType(type); - if (item!=null) { - log.warn("Creating application for requested type `"+type+" using item "+item+"; " - + "the registered type name ("+item.getSymbolicName()+") should be used from the spec instead, " - + "or the type registered under its own name. " - + "Future versions will likely change semantics to attempt a POJO load of the type instead."); - } - } if (item != null) { return setAs( @@ -253,50 +231,6 @@ private FindItemAndClass setAs(Class clazz, String catalogItem this.catalogItemId = catalogItemId; return this; } - - @Deprecated // see caller - private RegisteredType getCatalogItemForType(String typeName) { - final RegisteredType resultI; - if (CatalogUtils.looksLikeVersionedId(typeName)) { - //All catalog identifiers of the form aaaa:bbbb are composed of symbolicName+version. - //No javaType is allowed as part of the identifier. - resultI = mgmt.getTypeRegistry().get(typeName); - } else { - //Usually for catalog items with javaType (that is items from catalog.xml) - //the symbolicName and javaType match because symbolicName (was ID) - //is not specified explicitly. But could be the case that there is an item - //whose symbolicName is explicitly set to be different from the javaType. - //Note that in the XML the attribute is called registeredTypeName. - Iterable> resultL = mgmt.getCatalog().getCatalogItems(CatalogPredicates.javaType(Predicates.equalTo(typeName))); - if (!Iterables.isEmpty(resultL)) { - //Push newer versions in front of the list (not that there should - //be more than one considering the items are coming from catalog.xml). - resultI = RegisteredTypes.of(sortVersionsDesc(resultL).iterator().next()); - if (log.isDebugEnabled() && Iterables.size(resultL)>1) { - log.debug("Found "+Iterables.size(resultL)+" matches in catalog for type "+typeName+"; returning the result with preferred version, "+resultI); - } - } else { - //As a last resort try searching for items with the same symbolicName supposedly - //different from the javaType. - resultI = mgmt.getTypeRegistry().get(typeName, BrooklynCatalog.DEFAULT_VERSION); - if (resultI != null) { - if (resultI.getSuperTypes().isEmpty()) { - //Catalog items scanned from the classpath (using reflection and annotations) now - //get yaml spec rather than a java type. Can't use those when creating apps from - //the legacy app spec format. - log.warn("Unable to find catalog item for type "+typeName + - ". There is an existing catalog item with ID " + resultI.getId() + - " but it doesn't define a class type."); - return null; - } - } - } - } - return resultI; - } - private Collection> sortVersionsDesc(Iterable> versions) { - return ImmutableSortedSet.orderedBy(CatalogItemComparator.getInstance()).addAll(versions).build(); - } } public Application create(ApplicationSpec spec) { diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java index 5e8b7f9555..bd7a7405e6 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/WebResourceUtils.java @@ -30,6 +30,7 @@ import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; +import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.rest.domain.ApiError; import org.apache.brooklyn.rest.util.json.BrooklynJacksonJsonProvider; import org.apache.brooklyn.util.exceptions.Exceptions; @@ -175,7 +176,7 @@ public static Object getValueForDisplay(ObjectMapper mapper, Object value, boole } public static String getPathFromVersionedId(String versionedId) { - if (CatalogUtils.looksLikeVersionedId(versionedId)) { + if (RegisteredTypeNaming.isUsableTypeColonVersion(versionedId)) { String symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(versionedId); String version = CatalogUtils.getVersionFromVersionedId(versionedId); return Urls.encode(symbolicName) + "/" + Urls.encode(version); diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtilsTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtilsTest.java index 9b176712de..57a18ab4c6 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtilsTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/util/BrooklynRestResourceUtilsTest.java @@ -41,6 +41,7 @@ import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.rest.domain.ApplicationSpec; import org.apache.brooklyn.rest.domain.EntitySpec; +import org.testng.Assert; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -83,17 +84,17 @@ public void testCreateAppFromImplClass() { @Test public void testCreateAppFromCatalogByType() { - createAppFromCatalog(SampleNoOpApplication.class.getName()); + createAppFromCatalog(SampleNoOpApplication.class.getName(), false); } @Test public void testCreateAppFromCatalogByName() { - createAppFromCatalog("app.noop"); + createAppFromCatalog("app.noop", true); } @Test public void testCreateAppFromCatalogById() { - createAppFromCatalog("app.noop:0.0.1"); + createAppFromCatalog("app.noop:0.0.1", true); } @Test @@ -103,11 +104,12 @@ public void testCreateAppFromCatalogByTypeMultipleItems() { .javaType(SampleNoOpApplication.class.getName()) .build(); managementContext.getCatalog().addItem(item); - createAppFromCatalog(SampleNoOpApplication.class.getName()); + createAppFromCatalog(SampleNoOpApplication.class.getName(), false); + createAppFromCatalog("app.noop", true); } @SuppressWarnings("deprecation") - private void createAppFromCatalog(String type) { + private void createAppFromCatalog(String type, boolean expectCatalogItemIdSet) { CatalogTemplateItemDto item = CatalogItemBuilder.newTemplate("app.noop", "0.0.1") .javaType(SampleNoOpApplication.class.getName()) .build(); @@ -120,22 +122,28 @@ private void createAppFromCatalog(String type) { .build(); Application app = util.create(spec); - assertEquals(app.getCatalogItemId(), "app.noop:0.0.1"); + if (expectCatalogItemIdSet) { + assertEquals(app.getCatalogItemId(), "app.noop:0.0.1"); + } else { + // since 0.12.0 we no longer reverse-lookup java types to try to find a registered type + // (as per warnings in several previous versions) + Assert.assertNull(app.getCatalogItemId()); + } } @Test public void testEntityAppFromCatalogByType() { - createEntityFromCatalog(BasicEntity.class.getName()); + createEntityFromCatalog(BasicEntity.class.getName(), false); } @Test public void testEntityAppFromCatalogByName() { - createEntityFromCatalog("app.basic"); + createEntityFromCatalog("app.basic", true); } @Test public void testEntityAppFromCatalogById() { - createEntityFromCatalog("app.basic:0.0.1"); + createEntityFromCatalog("app.basic:0.0.1", true); } @Test @@ -145,11 +153,12 @@ public void testEntityAppFromCatalogByTypeMultipleItems() { .javaType(SampleNoOpApplication.class.getName()) .build(); managementContext.getCatalog().addItem(item); - createEntityFromCatalog(BasicEntity.class.getName()); + createEntityFromCatalog(BasicEntity.class.getName(), false); + createEntityFromCatalog("app.basic", true); } @SuppressWarnings("deprecation") - private void createEntityFromCatalog(String type) { + private void createEntityFromCatalog(String type, boolean expectCatalogItemIdSet) { String symbolicName = "app.basic"; String version = "0.0.1"; CatalogTemplateItemDto item = CatalogItemBuilder.newTemplate(symbolicName, version) @@ -165,7 +174,13 @@ private void createEntityFromCatalog(String type) { Application app = util.create(spec); Entity entity = Iterables.getOnlyElement(app.getChildren()); - assertEquals(entity.getCatalogItemId(), CatalogUtils.getVersionedId(symbolicName, version)); + if (expectCatalogItemIdSet) { + assertEquals(entity.getCatalogItemId(), CatalogUtils.getVersionedId(symbolicName, version)); + } else { + // since 0.12.0 we no longer reverse-lookup java types to try to find a registered type + // (as per warnings in several previous versions) + Assert.assertNull(entity.getCatalogItemId()); + } } @Test