You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/12/08 00:24:56 UTC
[brooklyn-server] 02/03: allow kind to change during validation
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 2f429a1c0d55482f9ccbc70302bf70dfc04c3d15
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Dec 7 22:56:46 2022 +0000
allow kind to change during validation
make the last failing test pass
---
.../catalog/NestedRefsCatalogYamlTest.java | 9 ++----
.../catalog/internal/BasicBrooklynCatalog.java | 32 ++++++++++++++--------
.../brooklyn/core/typereg/RegisteredTypes.java | 24 ++++++++++------
3 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
index c2f88b7e76..6bbcb140c5 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
@@ -128,14 +128,9 @@ public class NestedRefsCatalogYamlTest extends AbstractYamlTest {
/* test that invoke-effector doesn't cause confusion */
void doTestAddAfterWorkflow(String url) {
-
// add invoke-effector bean via a bundle
// WorkflowBasicTest.addWorkflowStepTypes(mgmt());
Collection<RegisteredType> typesW = addCatalogItems(ResourceUtils.create(this).getResourceAsString("classpath://nested-refs-catalog-yaml-workflow-test.bom.yaml"));
- Map<RegisteredType, Collection<Throwable>> resultW = mgmt().getCatalog().validateTypes(typesW);
- if (!resultW.isEmpty()) {
- Asserts.fail("Should be empty: " + resultW);
- }
// now add it as a type, in various ways
Collection<RegisteredType> types = addCatalogItems(ResourceUtils.create(this).getResourceAsString(url));
@@ -144,7 +139,7 @@ public class NestedRefsCatalogYamlTest extends AbstractYamlTest {
Asserts.assertEquals(mgmt().getTypeRegistry().get("invoke-effector", RegisteredTypeLoadingContexts.loader(
new OsgiBrooklynClassLoadingContext(mgmt(), types.iterator().next().getId(), null))).getContainingBundle(), "test-bundle:0.1.0-SNAPSHOT");
- // which should allow it to validate
+ // re-do the validation to be sure
Map<RegisteredType, Collection<Throwable>> result = mgmt().getCatalog().validateTypes(types);
if (!result.isEmpty()) {
Asserts.fail("Failed validation should be empty: " + result);
@@ -161,7 +156,7 @@ public class NestedRefsCatalogYamlTest extends AbstractYamlTest {
doTestAddAfterWorkflow("classpath://nested-refs-catalog-yaml-test-2.bom.yaml");
}
- @Test(groups = "Broken") // need to defer loading until all items pre-parsed, or correct loading later
+ @Test // allow validation to change the kind once all peers are loaded
public void testAddAmbiguousCatalogItemsPreferFromSameBundleEvenIfDefinedLater() throws Exception {
doTestAddAfterWorkflow("classpath://nested-refs-catalog-yaml-test-3.bom.yaml");
}
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 35f60b23ab..fdfb87b253 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
@@ -65,6 +65,7 @@ import org.apache.brooklyn.core.typereg.*;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ClassLoaderUtils;
import org.apache.brooklyn.util.core.ResourceUtils;
import org.apache.brooklyn.util.core.flags.BrooklynTypeNameResolution.BrooklynTypeNameResolver;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
@@ -911,6 +912,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
final String deprecated = getFirstAs(catalogMetadata, String.class, "deprecated").orNull();
final Boolean catalogDeprecated = Boolean.valueOf(setFromItemIfUnset(deprecated, itemAsMap, "deprecated"));
+ // provisional resolution - will be done again during validation, and even the kind might change, eg if there is a local bundle item
+ // indicating a different preferred supertype as compared with something else stored in type registry
planInterpreter.resolve();
if (!planInterpreter.isResolved()) {
// don't throw yet, we may be able to add it in an unresolved state
@@ -1908,7 +1911,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
}
Collection<Throwable> tr = validateType(t, null, true);
if (!tr.isEmpty()) {
- result.put(t, tr);
+ // reload it in case it changed
+ result.put(mgmt.getTypeRegistry().get(t.getId(), RegisteredTypeLoadingContexts.loader(CatalogUtils.newClassLoadingContext(mgmt, t))),
+ tr);
}
}
String msg = (typesRemainingToValidate.size()-result.size())+" validated, "+result.size()+" unvalidated";
@@ -1928,7 +1933,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
public Collection<Throwable> validateType(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint, boolean allowUnresolved) {
try {
currentlyValidatingType.set(typeToValidate);
- ReferenceWithError<RegisteredType> result = validateResolve(typeToValidate, constraint);
+ ReferenceWithError<RegisteredType> result = validateResolve(typeToValidate, constraint, true);
if (result.hasError()) {
if (allowUnresolved && RegisteredTypes.isTemplate(typeToValidate)) {
// ignore for templates
@@ -1952,7 +1957,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
* The argument may be changed (e.g. its kind set, supertypes set), and normal usage is to add
* a type in an "unresolved" state if things may need to reference it, then call resolve here,
* then replace what was added with the argument given here. */
- ReferenceWithError<RegisteredType> validateResolve(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint) {
+ ReferenceWithError<RegisteredType> validateResolve(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint, boolean allowChangingKind) {
Throwable inconsistentSuperTypesError=null, specError=null, beanError=null;
List<Throwable> guesserErrors = MutableList.of();
@@ -1988,7 +1993,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
Object resultO = null;
if (resultO==null && boType!=null) try {
// try spec instantiation if we know the BO Type (no point otherwise)
- resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate);
+ resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate, allowChangingKind);
try {
resultO = mgmt.getTypeRegistry().createSpec(resultT, constraint, boType.getSpecType());
} catch (Exception e) {
@@ -2003,7 +2008,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
if (resultO==null) try {
// try it as a bean
- resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.BEAN, typeToValidate);
+ resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.BEAN, typeToValidate, allowChangingKind);
try {
resultO = ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createBean(resultT, constraint, superJ);
if (typeToValidate.getKind()!=RegisteredTypeKind.BEAN && isDubiousBeanType(resultO)) {
@@ -2031,10 +2036,15 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
log.trace("Validating {}: \n{}", typeToValidate, yaml);
CatalogBundle bundle = typeToValidate.getContainingBundle() != null ? CatalogItemDtoAbstract.parseLibraries(Arrays.asList(typeToValidate.getContainingBundle())).iterator().next() : null;
- CatalogItemType itemType = boType!=null ? CatalogItemType.ofTargetClass(boType.getInterfaceType()) : null;
- if (itemType==null && typeToValidate.getKind() == RegisteredTypeKind.BEAN) {
- itemType = CatalogItemType.BEAN;
+ CatalogItemType itemType = null;
+
+ if (!allowChangingKind) {
+ itemType = boType!=null ? CatalogItemType.ofTargetClass(boType.getInterfaceType()) : null;
+ if (itemType==null && typeToValidate.getKind() == RegisteredTypeKind.BEAN) {
+ itemType = CatalogItemType.BEAN;
+ }
}
+
String format = typeToValidate.getPlan().getPlanFormat();
PlanInterpreterInferringType guesser = new PlanInterpreterInferringType(typeToValidate.getSymbolicName(), Iterables.getOnlyElement( Yamls.parseAll(yaml) ),
yaml, itemType, format, bundle, CatalogItemDtoAbstract.parseLibraries( typeToValidate.getLibraries() ), constraint, null);
@@ -2059,7 +2069,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
supers = MutableSet.copyOf(supers);
supers.add(boType.getInterfaceType());
// didn't know type before, retry now that we know the type
- resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, resultT);
+ resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, resultT, allowChangingKind);
RegisteredTypes.addSuperTypes(resultT, supers);
changedSomething = true;
}
@@ -2074,7 +2084,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
if (changedSomething) {
log.debug("Re-resolving "+resultT+" following detection of change");
// try again with new plan or supertype info
- return validateResolve(resultT, constraint);
+ return validateResolve(resultT, constraint, false);
} else if (Objects.equal(boType, BrooklynObjectType.of(ciType))) {
if (specError==null) {
@@ -2099,7 +2109,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
if (resultO instanceof BrooklynObject) {
// if it was a bean that points at a BO then switch it to a spec and try to re-validate
log.debug("Re-resolving "+resultT+" following detection of bean where spec was expected");
- return validateResolve(RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate), constraint);
+ return validateResolve(RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate), constraint, false);
}
Class<?> resultS;
if (resultT.getKind() == RegisteredTypeKind.SPEC) {
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 5a416b84c1..ec88e89ae6 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
@@ -237,7 +237,10 @@ public class RegisteredTypes {
}
@Beta
public static RegisteredType copyResolved(RegisteredTypeKind kind, RegisteredType t) {
- if (t.getKind()!=null && t.getKind()!=RegisteredTypeKind.UNRESOLVED && t.getKind()!=kind) {
+ return copyResolved(kind, t, false);
+ }
+ public static RegisteredType copyResolved(RegisteredTypeKind kind, RegisteredType t, boolean allowChangingKind) {
+ if (!allowChangingKind && t.getKind()!=null && t.getKind()!=RegisteredTypeKind.UNRESOLVED && t.getKind()!=kind) {
throw new IllegalStateException("Cannot copy resolve "+t+" ("+t.getKind()+") as "+kind);
}
return newInstance(kind, t.getSymbolicName(), t.getVersion(), t.getPlan(),
@@ -528,14 +531,17 @@ public class RegisteredTypes {
RegisteredType first = ti.next();
if (!ti.hasNext()) return first; //only
- Collection<? extends OsgiBundleWithUrl> preferredBundles = context.getLoader().getBundles();
- for (OsgiBundleWithUrl b: preferredBundles) {
- Iterable<RegisteredType> typesInBundle = Iterables.filter(types, t -> Objects.equal(
- VersionedName.fromString(t.getContainingBundle()), b.getVersionedName()));
- if (typesInBundle.iterator().hasNext()) {
- if (log.isTraceEnabled()) log.trace("Preferring "+typesInBundle+" from "+types+" because its bundle is in the explicit loader list");
- types = typesInBundle;
- break;
+ if (context!=null && context.getLoader()!=null) {
+ Collection<? extends OsgiBundleWithUrl> preferredBundles = context.getLoader().getBundles();
+ for (OsgiBundleWithUrl b : preferredBundles) {
+ Iterable<RegisteredType> typesInBundle = Iterables.filter(types, t -> t.getContainingBundle() != null && Objects.equal(
+ VersionedName.fromString(t.getContainingBundle()).toOsgiString(), b.getVersionedName().toOsgiString()));
+ if (typesInBundle.iterator().hasNext()) {
+ if (log.isTraceEnabled())
+ log.trace("Preferring " + typesInBundle + " from " + types + " because its bundle is in the explicit loader list");
+ types = typesInBundle;
+ break;
+ }
}
}