This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 2954e500a4d9deca35f034bea6b20a7168c04aef Author: Alex Heneveld <[email protected]> AuthorDate: Tue Aug 22 12:22:26 2023 +0100 test and fix for issue where `item: URL` in catalog.bom breaks with comments --- .../CatalogYamlEntityOsgiTypeRegistryTest.java | 28 +++++++++++++++------- .../brooklyn/catalog/CatalogYamlEntityTest.java | 14 +++++++++++ .../resources/yaml-ref-classpath-with-comment.bom | 24 +++++++++++++++++++ .../src/test/resources/yaml-ref-classpath.bom | 25 +++++++++++++++++++ .../src/test/resources/yaml-ref-policy.yaml | 19 +++++++++++++++ .../catalog/internal/BasicBrooklynCatalog.java | 13 +++++----- .../org/apache/brooklyn/util/yaml/YamlsTest.java | 6 +++++ 7 files changed, 115 insertions(+), 14 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java index d73917f679..6fa14321d7 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java @@ -18,9 +18,8 @@ */ package org.apache.brooklyn.camp.brooklyn.catalog; -import java.util.Collection; -import java.util.List; -import java.util.Map; +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.spi.creation.CampTypePlanTransformer; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; @@ -36,11 +35,11 @@ import org.apache.brooklyn.util.yaml.Yamls; import org.testng.Assert; import org.testng.annotations.Test; -import com.google.common.base.Predicates; -import com.google.common.collect.Iterables; +import java.util.Collection; +import java.util.List; +import java.util.Map; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; /** Variant of parent tests using OSGi, bundles, and type registry, instead of lightweight non-osgi catalog */ @Test @@ -66,8 +65,8 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends CatalogYamlEntityTest // this is the default because some "bundles" aren't resolvable or library BOMs loadable in test context CatalogItemsInstallationMode.BUNDLE_BUT_NOT_STARTED) { case ADD_YAML_ITEMS_UNBUNDLED: - super.addCatalogItems(catalogYaml); - break; + return super.addCatalogItems(catalogYaml); + case BUNDLE_BUT_NOT_STARTED: skipStart = true; // continue to below @@ -254,4 +253,17 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends CatalogYamlEntityTest public void testCatalogItemIdInReferencedItems() throws Exception { super.testCatalogItemIdInReferencedItems(); } + + @Test + public void addFromClasspath() { + itemsInstallMode = CatalogItemsInstallationMode.ADD_YAML_ITEMS_UNBUNDLED; + super.addFromClasspath(); + } + + @Test + public void addFromClasspathWithComment() { + itemsInstallMode = CatalogItemsInstallationMode.ADD_YAML_ITEMS_UNBUNDLED; + super.addFromClasspathWithComment(); + } + } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java index 572b945677..5f29fb7dd0 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java @@ -29,6 +29,7 @@ import org.apache.brooklyn.core.entity.Dumper; import org.apache.brooklyn.core.mgmt.BrooklynTags; import org.apache.brooklyn.core.mgmt.BrooklynTags.SpecSummary; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.core.ResourceUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import static org.testng.Assert.assertEquals; @@ -927,4 +928,17 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { " item:", " type: " + serviceType); } + + @Test + public void addFromClasspath() { + Collection<RegisteredType> typesW = addCatalogItems(ResourceUtils.create(this).getResourceAsString("classpath://yaml-ref-classpath.bom")); + Asserts.assertThat(Iterables.getOnlyElement(typesW), r -> r.getId().equals("simple:0.1.2")); + } + + @Test + public void addFromClasspathWithComment() { + Collection<RegisteredType> typesW = addCatalogItems(ResourceUtils.create(this).getResourceAsString("classpath://yaml-ref-classpath-with-comment.bom")); + Asserts.assertThat(Iterables.getOnlyElement(typesW), r -> r.getId().equals("simple:0.1.2")); + } + } diff --git a/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath-with-comment.bom b/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath-with-comment.bom new file mode 100644 index 0000000000..6e7e93ad64 --- /dev/null +++ b/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath-with-comment.bom @@ -0,0 +1,24 @@ +# 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. + +brooklyn.catalog: + id: yaml-ref-classpath + version: 0.1.2 + items: + - id: simple + item: classpath://yaml-ref-policy.yaml +# comment at end diff --git a/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath.bom b/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath.bom new file mode 100644 index 0000000000..b14dcf6755 --- /dev/null +++ b/camp/camp-brooklyn/src/test/resources/yaml-ref-classpath.bom @@ -0,0 +1,25 @@ +# 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. + +brooklyn.catalog: + id: yaml-ref-classpath + version: 0.1.2 + items: + - id: simple + item: classpath://yaml-ref-entity.yaml + + diff --git a/camp/camp-brooklyn/src/test/resources/yaml-ref-policy.yaml b/camp/camp-brooklyn/src/test/resources/yaml-ref-policy.yaml new file mode 100644 index 0000000000..4e8f4e0fb9 --- /dev/null +++ b/camp/camp-brooklyn/src/test/resources/yaml-ref-policy.yaml @@ -0,0 +1,19 @@ +# 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. +# +name: test-policy +type: org.apache.brooklyn.core.test.policy.TestPolicy 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 ea521a1a9a..b601fad8d5 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 @@ -767,22 +767,23 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { log.debug("Analyzing item " + loggedId + " for addition to catalog"); Exception resolutionError = null; - if (sourceYaml.trim().matches("[A-Za-z0-9]+:[^\\s]+")) { + String itemAsString = item instanceof String ? (String) item : null; + if (itemAsString!=null && itemAsString.matches("[A-Za-z0-9]+:[^\\s]+")) { // if sourceYaml is one word and looks like a URL, then read it as a URL first BrooklynClassLoadingContext loader = getClassLoadingContext("catalog item url loader", parentMetadata, libraryBundles); - log.debug("Catalog load, loading referenced item at "+url+" for "+loggedId+" as part of "+(containingBundle==null ? "non-bundled load" : containingBundle.getVersionedName())+" ("+(resultNewFormat!=null ? resultNewFormat.size() : resultLegacyFormat!=null ? resultLegacyFormat.size() : "(unknown)")+" items before load)"); - if (sourceYaml.startsWith("http")) { + log.debug("Catalog load, loading referenced item at "+item+" for "+loggedId+" as part of "+(containingBundle==null ? "non-bundled load" : containingBundle.getVersionedName())+" ("+(resultNewFormat!=null ? resultNewFormat.size() : resultLegacyFormat!=null ? resultLegacyFormat.size() : "(unknown)")+" items before load)"); + if (itemAsString.startsWith("http")) { // give greater visibility to these - log.info("Loading external referenced item at "+url+" for "+loggedId+" as part of "+(containingBundle==null ? "non-bundled load" : containingBundle.getVersionedName())); + log.info("Loading external referenced item at "+item+" for "+loggedId+" as part of "+(containingBundle==null ? "non-bundled load" : containingBundle.getVersionedName())); } try { - sourceYaml = ResourceUtils.create(loader).getResourceAsString(sourceYaml.trim()); + sourceYaml = ResourceUtils.create(loader).getResourceAsString(itemAsString.trim()); item = Yamls.parseAll(sourceYaml).iterator().next(); } catch (Exception e) { Exceptions.propagateIfFatal(e); // don't throw, but include in list of proposed errors - resolutionError = new IllegalStateException("Unable to load '"+sourceYaml+"' as URL", e); + resolutionError = new IllegalStateException("Unable to load '"+itemAsString+"' as URL", e); } } PlanInterpreterInferringType planInterpreter = new PlanInterpreterInferringType(id, item, sourceYaml, itemType, format, diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/yaml/YamlsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/yaml/YamlsTest.java index 46057086ef..b1d868b289 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/yaml/YamlsTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/yaml/YamlsTest.java @@ -209,6 +209,12 @@ public class YamlsTest { Asserts.assertEquals(Yamls.lastDocumentFunction().apply(null), null); } + @Test + public void testCommentsAtEnd() { + String input = " item: x\n#comment"; + Asserts.assertEquals(Yamls.getTextOfYamlAtPath(input, "item").getMatchedYamlTextOrWarn(), "x\n#comment"); + } + // convenience, since running with older TestNG IDE plugin will fail (older snakeyaml dependency); // if you run as a java app it doesn't bring in the IDE TestNG jar version, and it works public static void main(String[] args) {
