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
The following commit(s) were added to refs/heads/master by this push:
new aefb0d85b4 prevent accidental creation of enricher and policy when
there is a type declared
new 12e1338f60 Merge branch 'master' of
https://gitbox.apache.org/repos/asf/brooklyn-server
aefb0d85b4 is described below
commit aefb0d85b4bf8d9ffa3fea0cce1f5e4012a848cb
Author: Alex Heneveld <[email protected]>
AuthorDate: Mon Mar 27 16:38:13 2023 +0100
prevent accidental creation of enricher and policy when there is a type
declared
also include one more error message category in failure output
---
.../brooklyn/spi/creation/CampInternalUtils.java | 19 ++++---
.../catalog/CatalogOsgiYamlEntityTest.java | 2 +-
.../brooklyn/catalog/CatalogYamlRebindTest.java | 59 ++++++++++++++++++++++
.../catalog/internal/BasicBrooklynCatalog.java | 8 +--
4 files changed, 77 insertions(+), 11 deletions(-)
diff --git
a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java
b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java
index b916e09b6d..168ea86ed7 100644
---
a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java
+++
b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java
@@ -126,7 +126,7 @@ class CampInternalUtils {
//Would ideally re-use the PolicySpecResolver
//but it is CAMP specific and there is no easy way to get hold of it.
Object policy;
- if
(plan.getCustomAttributes().containsKey(BasicBrooklynCatalog.POLICIES_KEY)) {
+ if
(plan.getCustomAttributes().containsKey(BasicBrooklynCatalog.POLICIES_KEY) &&
!plan.getCustomAttributes().containsKey("type")) {
Object policies =
checkNotNull(plan.getCustomAttributes().get(BasicBrooklynCatalog.POLICIES_KEY),
"policy config");
if (!(policies instanceof Iterable<?>)) {
throw new IllegalStateException("The value of " +
BasicBrooklynCatalog.POLICIES_KEY + " must be an Iterable.");
@@ -160,7 +160,7 @@ class CampInternalUtils {
DeploymentPlan plan = makePlanFromYaml(loader.getManagementContext(),
yamlPlan);
Object enricher;
- if
(plan.getCustomAttributes().containsKey(BasicBrooklynCatalog.ENRICHERS_KEY)) {
+ if
(plan.getCustomAttributes().containsKey(BasicBrooklynCatalog.ENRICHERS_KEY) &&
!plan.getCustomAttributes().containsKey("type")) {
//Would ideally re-use the EnricherSpecResolver
//but it is CAMP specific and there is no easy way to get hold of
it.
Object enrichers =
checkNotNull(plan.getCustomAttributes().get(BasicBrooklynCatalog.ENRICHERS_KEY),
"enricher config");
@@ -195,13 +195,18 @@ class CampInternalUtils {
}
static LocationSpec<?> createLocationSpec(String yamlPlan,
BrooklynClassLoadingContext loader, Set<String> encounteredTypes) {
+ Object location;
DeploymentPlan plan = makePlanFromYaml(loader.getManagementContext(),
yamlPlan);
- Object locations =
checkNotNull(plan.getCustomAttributes().get(BasicBrooklynCatalog.LOCATIONS_KEY),
"location config");
- if (!(locations instanceof Iterable<?>)) {
- throw new IllegalStateException("The value of " +
BasicBrooklynCatalog.LOCATIONS_KEY + " must be an Iterable.");
- }
+ if (!plan.getCustomAttributes().containsKey("type")) {
+ Object locations =
checkNotNull(plan.getCustomAttributes().get(BasicBrooklynCatalog.LOCATIONS_KEY),
"location config");
+ if (!(locations instanceof Iterable<?>)) {
+ throw new IllegalStateException("The value of " +
BasicBrooklynCatalog.LOCATIONS_KEY + " must be an Iterable.");
+ }
- Object location = Iterables.getOnlyElement((Iterable<?>)locations);
+ location = Iterables.getOnlyElement((Iterable<?>) locations);
+ } else {
+ location = yamlPlan;
+ }
return createLocationSpec(loader, location);
}
diff --git
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
index d95837c699..4476d5eb10 100644
---
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
+++
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
@@ -554,7 +554,7 @@ public class CatalogOsgiYamlEntityTest extends
AbstractYamlTest {
" type: " + symbolicName + ".callee");
fail();
} catch (Exception e) {
- assertTrue(e.toString().contains("recursive"), "Unexpected error
message: "+e);
+ Asserts.expectedFailureContainsIgnoreCase(e, "recursive");
}
}
diff --git
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java
index 7fec6000cb..6bb57deb76 100644
---
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java
+++
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java
@@ -35,6 +35,7 @@ import
org.apache.brooklyn.api.mgmt.rebind.RebindManager.RebindFailureMode;
import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoPersister;
import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.api.sensor.Enricher;
import org.apache.brooklyn.api.typereg.RegisteredType;
import org.apache.brooklyn.camp.brooklyn.AbstractYamlRebindTest;
import org.apache.brooklyn.core.BrooklynFeatureEnablement;
@@ -52,6 +53,7 @@ import org.apache.brooklyn.core.mgmt.rebind.RebindOptions;
import org.apache.brooklyn.core.mgmt.rebind.transformer.CompoundTransformer;
import org.apache.brooklyn.core.test.policy.TestEnricher;
import org.apache.brooklyn.core.test.policy.TestPolicy;
+import org.apache.brooklyn.enricher.stock.Transformer;
import org.apache.brooklyn.entity.stock.BasicEntity;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.test.support.TestResourceUnavailableException;
@@ -462,4 +464,61 @@ public class CatalogYamlRebindTest extends
AbstractYamlRebindTest {
Asserts.assertSize(Arrays.asList( new File(mementoDir,
"bundles").list() ), 2);
}
+ @Test
+ public void testBrooklynEnrichersSectionWithValidTypePicksUpType() throws
Exception {
+ recreateOrigManagementContextWithOsgi();
+
+ String bom = Strings.lines(
+ "brooklyn.catalog:",
+ " items:",
+ " - id: sample",
+ " item:",
+ " type: " + BasicEntity.class.getName(),
+ " brooklyn.enrichers:",
+ " - type: "+ Transformer.class.getName());
+ addCatalogItems(bom);
+ RegisteredType rt = mgmt().getTypeRegistry().get("sample");
+ Asserts.assertFalse(rt.getSuperTypes().contains(Enricher.class));
+ Asserts.assertTrue(rt.getSuperTypes().contains(Entity.class));
+ rebind();
+ // should only contain one bundle / bundle.jar pair
+ Asserts.assertSize(Arrays.asList( new File(mementoDir,
"bundles").list() ), 2);
+ }
+
+ @Test
+ public void
testBrooklynEnrichersSectionWithNoTypePicksUpEnricherLegacySyntax() throws
Exception {
+ recreateOrigManagementContextWithOsgi();
+
+ String bom = Strings.lines(
+ "brooklyn.catalog:",
+ " items:",
+ " - id: sample",
+ " item:",
+ " brooklyn.enrichers:",
+ " - type: "+ Transformer.class.getName());
+ addCatalogItems(bom);
+ RegisteredType rt = mgmt().getTypeRegistry().get("sample");
+ Asserts.assertTrue(rt.getSuperTypes().contains(Enricher.class));
+ Asserts.assertFalse(rt.getSuperTypes().contains(Entity.class));
+ rebind();
+ // should only contain one bundle / bundle.jar pair
+ Asserts.assertSize(Arrays.asList( new File(mementoDir,
"bundles").list() ), 2);
+ }
+
+ @Test
+ public void testBrooklynEnrichersSectionWithInvalidTypeGivesError() throws
Exception {
+ recreateOrigManagementContextWithOsgi();
+
+ // this used to ignore the invalid type and run the brooklyn.enrichers
code on its own
+ String bom = Strings.lines(
+ "brooklyn.catalog:",
+ " items:",
+ " - id: sample",
+ " item:",
+ " type: invalid_type",
+ " brooklyn.enrichers:",
+ " - type: "+ Transformer.class.getName());
+ Asserts.assertFailsWith(() -> addCatalogItems(bom), e ->
Asserts.expectedFailureContains(e, "invalid_type"));
+ }
+
}
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 4be8e65385..ea521a1a9a 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
@@ -42,6 +42,7 @@ import
org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle;
import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
import org.apache.brooklyn.api.entity.Application;
import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
+import org.apache.brooklyn.api.location.LocationSpec;
import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
import org.apache.brooklyn.api.objs.BrooklynObject;
@@ -1642,9 +1643,9 @@ public class BasicBrooklynCatalog implements
BrooklynCatalog {
// deprecated old style
@SuppressWarnings("rawtypes")
CatalogItem itemToAttempt =
createItemBuilder(candidateCiType, getIdWithRandomDefault(), DEFAULT_VERSION)
- .plan(candidateYaml)
- .libraries(libraryBundles)
- .build();
+ .plan(candidateYaml)
+ .libraries(libraryBundles)
+ .build();
itemSpecInstantiated = internalCreateSpecLegacy(mgmt,
itemToAttempt, MutableSet.<String>of(), true);
}
@@ -1723,6 +1724,7 @@ public class BasicBrooklynCatalog implements
BrooklynCatalog {
}
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
+ errors.add(e); // add this because it finds more errors,
such as recursive
}
}
