prevent recursive loops in just-in-time resolution with loading context on validateType API
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/4b91ca42 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/4b91ca42 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/4b91ca42 Branch: refs/heads/master Commit: 4b91ca4295a9e120ad70aae4769acd5e197abf05 Parents: e7c87ff Author: Alex Heneveld <[email protected]> Authored: Tue Aug 29 11:25:13 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Tue Aug 29 12:05:23 2017 +0100 ---------------------------------------------------------------------- .../brooklyn/api/catalog/BrooklynCatalog.java | 3 ++- .../catalog/internal/BasicBrooklynCatalog.java | 23 +++++++++++--------- .../core/mgmt/ha/OsgiArchiveInstaller.java | 2 +- .../core/typereg/BasicBrooklynTypeRegistry.java | 23 ++++++++++++-------- 4 files changed, 30 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4b91ca42/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java index aad8c1d..cbc5b2b 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java @@ -27,6 +27,7 @@ import javax.annotation.Nullable; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.RegisteredType; +import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; @@ -120,7 +121,7 @@ public interface BrooklynCatalog { * for the given registered type. */ @Beta // method may move elsewhere - Collection<Throwable> validateType(RegisteredType typeToValidate); + Collection<Throwable> validateType(RegisteredType typeToValidate, @Nullable RegisteredTypeLoadingContext optionalConstraint); /** http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4b91ca42/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- 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 16618b0..564b023 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 @@ -53,6 +53,7 @@ import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl; import org.apache.brooklyn.api.typereg.RegisteredType; +import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import org.apache.brooklyn.core.catalog.CatalogPredicates; import org.apache.brooklyn.core.catalog.internal.CatalogClasspathDo.CatalogScanningModes; import org.apache.brooklyn.core.mgmt.BrooklynTags; @@ -1477,7 +1478,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { log.debug("Catalog load, starting validation cycle, "+typesRemainingToValidate.size()+" to validate"); Map<RegisteredType,Collection<Throwable>> result = MutableMap.of(); for (RegisteredType t: typesRemainingToValidate) { - Collection<Throwable> tr = validateType(t); + Collection<Throwable> tr = validateType(t, null); if (!tr.isEmpty()) { result.put(t, tr); } @@ -1494,8 +1495,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } @Override @Beta - public Collection<Throwable> validateType(RegisteredType typeToValidate) { - ReferenceWithError<RegisteredType> result = resolve(typeToValidate); + public Collection<Throwable> validateType(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint) { + ReferenceWithError<RegisteredType> result = resolve(typeToValidate, constraint); if (result.hasError()) { if (RegisteredTypes.isTemplate(typeToValidate)) { // ignore for templates @@ -1517,7 +1518,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { * 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. */ @Beta - public ReferenceWithError<RegisteredType> resolve(RegisteredType typeToValidate) { + public ReferenceWithError<RegisteredType> resolve(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint) { Throwable inconsistentSuperTypesError=null, specError=null, beanError=null; List<Throwable> guesserErrors = MutableList.of(); @@ -1555,7 +1556,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // try spec instantiation if we know the BO Type (no point otherwise) resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate); try { - resultO = ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createSpec(resultT, null, boType.getSpecType()); + resultO = ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createSpec(resultT, constraint, boType.getSpecType()); } catch (Exception e) { Exceptions.propagateIfFatal(e); specError = e; @@ -1570,7 +1571,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // try it as a bean resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.BEAN, typeToValidate); try { - resultO = ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createBean(resultT, null, superJ); + resultO = ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createBean(resultT, constraint, superJ); } catch (Exception e) { Exceptions.propagateIfFatal(e); beanError = e; @@ -1581,10 +1582,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // ignore if we couldn't resolve as spec } - if (resultO==null) try { + if (resultO==null && (constraint==null || constraint.getAlreadyEncounteredTypes().isEmpty())) try { // try the legacy PlanInterpreterGuessingType // (this is the only place where we will guess specs, so it handles - // most of our traditional catalog items in BOMs) + // most of our traditional catalog items in BOMs); + // but do not allow this to run if we are expanding a nested definition as that may fail to find recursive loops + // (the legacy routines this uses don't support that type of context) String yaml = RegisteredTypes.getImplementationDataStringForSpec(typeToValidate); PlanInterpreterGuessingType guesser = new PlanInterpreterGuessingType(typeToValidate.getSymbolicName(), Iterables.getOnlyElement( Yamls.parseAll(yaml) ), yaml, null, CatalogItemDtoAbstract.parseLibraries( typeToValidate.getLibraries() ), null); @@ -1622,7 +1625,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (changedSomething) { // try again with new plan or supertype info - return resolve(resultT); + return resolve(resultT, constraint); } else if (Objects.equal(boType, BrooklynObjectType.of(ciType))) { if (specError==null) { @@ -1646,7 +1649,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (resultO!=null && resultT!=null) { 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 - return resolve(RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate)); + return resolve(RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, typeToValidate), constraint); } RegisteredTypes.cacheActualJavaType(resultT, resultO.getClass()); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4b91ca42/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java index 8cdb39e..c451363 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java @@ -444,7 +444,7 @@ class OsgiArchiveInstaller { } catch (Exception e) { // unable to install new items; rollback bundles // and reload replaced items - log.warn("Error adding Brooklyn items from bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle and items, then re-throwing error: "+e); + log.warn("Error adding Brooklyn items from bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle and items, then re-throwing error: "+Exceptions.collapseText(e)); rollbackBundle(); if (itemsFromOldBundle!=null) { // add back all itemsFromOldBundle (when replacing a bundle) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4b91ca42/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java index 8d0b143..5f8e361 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 @@ -193,16 +193,21 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry { return createSpec(type, type.getPlan(), type.getSymbolicName(), type.getVersion(), type.getSuperTypes(), constraint, specSuperType); } else if (type.getKind()==RegisteredTypeKind.UNRESOLVED) { - // try just-in-time validation - Collection<Throwable> validationErrors = mgmt.getCatalog().validateType(type); - if (!validationErrors.isEmpty()) { - throw new ReferencedUnresolvedTypeException(type, true, Exceptions.create(validationErrors)); - } - type = mgmt.getTypeRegistry().get(type.getSymbolicName(), type.getVersion()); - if (type==null || type.getKind()==RegisteredTypeKind.UNRESOLVED) { - throw new ReferencedUnresolvedTypeException(type); + if (constraint.getAlreadyEncounteredTypes().contains(type.getSymbolicName())) { + throw new UnsupportedTypePlanException("Cannot create spec from type "+type+" (kind "+type.getKind()+"), recursive reference following "+constraint.getAlreadyEncounteredTypes()); + + } else { + // try just-in-time validation + Collection<Throwable> validationErrors = mgmt.getCatalog().validateType(type, constraint); + if (!validationErrors.isEmpty()) { + throw new ReferencedUnresolvedTypeException(type, true, Exceptions.create(validationErrors)); + } + type = mgmt.getTypeRegistry().get(type.getSymbolicName(), type.getVersion()); + if (type==null || type.getKind()==RegisteredTypeKind.UNRESOLVED) { + throw new ReferencedUnresolvedTypeException(type); + } + return createSpec(type, constraint, specSuperType); } - return createSpec(type, constraint, specSuperType); } else { throw new UnsupportedTypePlanException("Cannot create spec from type "+type+" (kind "+type.getKind()+")");
