[brooklyn-server] 02/03: give better/earlier error if 'services' and 'type' supplied in same block

[heneveld]

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 ffdb9b349bc4b1313456653ca97ad4649b17b6ed
Author: Alex Heneveld <a...@cloudsoft.io>
AuthorDate: Fri Oct 6 15:55:24 2023 +0100

    give better/earlier error if 'services' and 'type' supplied in same block
---
 .../camp/brooklyn/ApplicationsYamlTest.java        | 15 +++++++++++
 .../catalog/internal/BasicBrooklynCatalog.java     | 30 +++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
index 656a478962..561578acda 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
@@ -446,6 +446,21 @@ public class ApplicationsYamlTest extends AbstractYamlTest 
{
         e -> Asserts.expectedFailureContains(e, "not_a_real_service"));
     }
 
+    @Test
+    public void testGoodErrorOnServicesBlockWithType() throws Exception {
+        Asserts.assertFailsWith(() -> {
+                    addCatalogItems(
+                            "brooklyn.catalog:",
+                            "  id: simple-test",
+                            "  version: " + TEST_VERSION,
+                            "type: foo",
+                            "services:",
+                            "- type: "+BasicEntity.class.getName());
+                    RegisteredType t = 
mgmt().getTypeRegistry().get("simple-test", TEST_VERSION);
+                    return t+" - "+t.getSuperTypes();
+                },
+                e -> Asserts.expectedFailureContains(e, "Blueprint contains 
both a 'services' block and a 'type' specification"));
+    }
 
     @Override
     protected Logger getLogger() {
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 b601fad8d5..e3a9208bf0 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
@@ -980,8 +980,12 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
                 if (!tags.contains(BrooklynTags.CATALOG_TEMPLATE)) {
                     if (requireValidation) {
                         throw Exceptions.propagate(resolutionError);
+                    } else {
+                        // normally if validation not requested (eg because we 
are adding multiple bundles and might have forward references,
+                        // we add (below) as unresolved, and then do 
validation later; but that validation doesn't check basic problems,
+                        // on those it makes sense to fail fast.
+                        planInterpreter.checkResolution(true);
                     }
-                    // warn? add as "unresolved" ? just do nothing?
                 }
             }
 
@@ -1364,6 +1368,11 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         }
 
         public void resolve() {
+            resolveWithoutChecking();
+            checkResolution(false);
+        }
+
+        public void resolveWithoutChecking() {
             try {
                 currentlyResolvingType.set(Strings.isBlank(itemId) ? itemYaml 
: itemId);
 
@@ -1414,6 +1423,25 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
             }
         }
 
+        public void checkResolution(boolean failOnBasicProblems) {
+            if (!failOnBasicProblems && !isResolved()) return;
+
+            if (item!=null) {
+                if (CatalogItemType.ENTITY.equals(catalogItemType)) {
+                    if (item.containsKey("services")) {
+                        if (item.containsKey("type")) {
+                            resolved = false;
+                            IllegalArgumentException error = new 
IllegalArgumentException("Blueprint contains both a 'services' block and a 
'type' specification; not permitted");
+                            if (failOnBasicProblems) throw error;
+                            this.errors.add(error);
+                            return;
+                        }
+                    }
+                }
+
+            }
+        }
+
         private void attemptLegacySpecTransformersForVariousSpecTypes() {
             attemptLegacySpecTransformersForType(CatalogItemType.ENTITY);