This is an automated email from the ASF dual-hosted git repository. duncangrant 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 acd7821 ensure good errors about beans, and some other transforms, are returned to caller new 3d05e66 Merge pull request #1111 from ahgittin/better-errors-on-bean-transforms acd7821 is described below commit acd782154ba9042e14b57ce91491865b67a96159 Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> AuthorDate: Tue Sep 29 10:49:41 2020 +0100 ensure good errors about beans, and some other transforms, are returned to caller --- .../camp/brooklyn/CustomTypeConfigYamlTest.java | 20 ++++++++++++++ .../catalog/internal/BasicBrooklynCatalog.java | 31 +++++++++++++--------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java index cee00f8..0b83a34 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java @@ -184,4 +184,24 @@ public class CustomTypeConfigYamlTest extends AbstractYamlTest { // can make jackson serialize and deserialize them specially, either pass-through or as strings TBD // see reference to DslSerializationAsToString in BeanWithTypeUtils + + @Test + public void testRegisteredTypeMalformed_GoodError() throws Exception { + // in the above case, fields are correctly inherited from ancestors and overridden + Asserts.assertFailsWith(() -> { + addCatalogItems( + "brooklyn.catalog:", + " version: " + TEST_VERSION, + " items:", + " - id: custom-type", +// " itemType: bean", // optional + " format: bean-with-type", + " item:", + " type: " + CustomTypeConfigYamlTest.TestingCustomType.class.getName(), + " x: {}"); + }, e -> { + Asserts.expectedFailureContainsIgnoreCase(e, "bean", "custom-type", "cannot deserialize", "string", "\"x\""); + return true; + }); + } } 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 f37ac04..84a4806 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 @@ -39,10 +39,8 @@ import java.util.Set; import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.zip.ZipEntry; - import javax.annotation.Nonnull; import javax.annotation.Nullable; - import org.apache.brooklyn.api.catalog.BrooklynCatalog; import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle; @@ -1270,13 +1268,16 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { boolean onlyNewStyleTransformer = format != null || catalogItemType == CatalogItemType.BEAN; if (transformedResult.isPresent() || onlyNewStyleTransformer) { planYaml = itemYaml; - if (catalogItemType!=CatalogItemType.BEAN && catalogItemType!=CatalogItemType.TEMPLATE) { - // for specs types, _also_ do the legacy and let it set resolution, - // as it is better at spotting some types of errors (recursive ones) + if (catalogItemType!=CatalogItemType.BEAN && catalogItemType!=CatalogItemType.TEMPLATE && !"bean-with-type".equals(format)) { + // for spec types, _also_ run the legacy resolution because it is better at spotting some types of errors (recursive ones); + // note this code will also run if there was an error when format was specified (other than bean-with-type) and we couldn't determine it was a bean resolved = false; attemptLegacySpecTransformersForVariousSpecTypes(); } else { resolved = transformedResult.isPresent() || catalogItemType == CatalogItemType.TEMPLATE; + if (!resolved) { + errors.add(Maybe.Absent.getException(transformedResult)); + } } return this; } @@ -1323,8 +1324,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } private Maybe<Object> attemptPlanTranformer() { + Exception errorInBean = null; try { - Exception e = null; boolean suspicionOfABean = false; Set<? extends OsgiBundleWithUrl> searchBundles = MutableSet.copyOf(libraryBundles) @@ -1350,10 +1351,10 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { try { t = mgmt.getTypeRegistry().createBeanFromPlan(format, itemYaml, constraint, null); catalogItemType = CatalogItemType.BEAN; - } catch (Exception eS) { - Exceptions.propagateIfFatal(eS); - // if we were speculatively trying bean then rety as plan - e = eS; + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + // if we were speculatively trying bean then retry as plan + errorInBean = e; } } @@ -1366,7 +1367,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } if (t==null) { - if (e!=null) throw e; + if (errorInBean!=null) throw errorInBean; throw new IllegalStateException("Type registry creation returned null"); } @@ -1374,7 +1375,13 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return Maybe.of(t); } catch (Exception e) { - return Maybe.absent(e); + Exceptions.propagateIfFatal(e); + MutableList<Throwable> exceptions = MutableList.<Throwable>of().appendIfNotNull(errorInBean, e); + return Maybe.absent(() -> { + return Exceptions.propagate("Unable to parse definition of "+ + (itemId!=null ? itemId : "plan:\n"+itemYaml+"\n"), + exceptions); + }); } }