tests for deletion of empty wrapper bundles

Author: ahgittin

camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java

@@ -25,13 +25,16 @@
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.util.Map;
 import java.util.zip.ZipEntry;
 
 import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.typereg.ManagedBundle;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
 import org.apache.brooklyn.camp.brooklyn.test.lite.CampYamlLiteTest;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.entity.stock.BasicEntity;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.support.TestResourceUnavailableException;
 import org.apache.brooklyn.util.collections.MutableMap;
@@ -117,4 +120,35 @@ static String bomForLegacySiblingLibraries() {
             "      - classpath://" + OsgiTestResources.BROOKLYN_TEST_MORE_ENTITIES_V2_PATH);
     }
 
+    @Test
+    public void testAddAnonymousBomTwiceDeletesOldEmptyOne() {
+        Map<String, ManagedBundle> b1 = ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles();
+        addCatalogItems(bomAnonymous());
+        Map<String, ManagedBundle> b2_new = MutableMap.copyOf(
+            ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles() );
+        for (String old: b1.keySet()) b2_new.remove(old);
+
+        RegisteredType sample = mgmt().getTypeRegistry().get("sample");
+        Asserts.assertNotNull(sample);
+        Asserts.assertSize(b2_new.values(), 1);
+        
+        addCatalogItems(bomAnonymous());
+        Map<String, ManagedBundle> b3 = MutableMap.copyOf(
+            ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles() );
+        Map<String, ManagedBundle> b3_new = MutableMap.copyOf(b3);
+        for (String old: b1.keySet()) b3_new.remove(old);
+        for (String old: b2_new.keySet()) b3_new.remove(old);
+        Asserts.assertSize(b3_new.values(), 1);
+
+        Asserts.assertFalse(b3_new.keySet().contains( Iterables.getOnlyElement(b2_new.keySet()) ));
+    }
+
+    static String bomAnonymous() {
+        return Strings.lines("brooklyn.catalog:",
+            "    items:",
+            "    - item:",
+            "        id: sample",
+            "        type: "+BasicEntity.class.getName());
+    }
+    
 }

camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java

@@ -25,6 +25,8 @@
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 
+import java.io.File;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.brooklyn.api.catalog.CatalogItem;
@@ -38,6 +40,7 @@
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.StartableApplication;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.mgmt.osgi.OsgiStandaloneTest;
 import org.apache.brooklyn.core.mgmt.persist.BrooklynMementoPersisterToObjectStore;
 import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore;
@@ -84,8 +87,14 @@ enum RebindWithCatalogTestMode {
 
     enum OsgiMode {
         NONE,
+        // we don't test OSGi with this because CAMP isn't available 
+        // (and the library bundles assume CAMP in their catalog.bom)
+        // OSGi rebind isn't the focus of the tests which use this parameter
+        // so it's okay, though a bit ugly
+//        NORMAL,
     }
 
+    boolean useOsgi = false;
     private Boolean defaultEnablementOfFeatureAutoFixatalogRefOnRebind;
 
     @BeforeMethod(alwaysRun=true)
@@ -106,7 +115,7 @@ public void tearDown() throws Exception {
 
     @Override
     protected boolean useOsgi() {
-        return false;
+        return useOsgi || (origManagementContext!=null && ((ManagementContextInternal)origManagementContext).getOsgiManager().isPresent());
     }
 
     @DataProvider
@@ -192,6 +201,8 @@ private String idFromPath(BrooklynObjectType type, String path) {
     public void testRebindWithCatalogAndAppUsingOptions(RebindWithCatalogTestMode mode, OsgiMode osgiMode, RebindOptions options) throws Exception {
         if (osgiMode != OsgiMode.NONE) {
             TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
+            
+            recreateOrigManagementContextWithOsgi();
         }
 
         if (mode == RebindWithCatalogTestMode.REPLACE_CATALOG_WITH_NEWER_VERSION) {
@@ -351,9 +362,33 @@ public void testRebindWithCatalogAndAppUsingOptions(RebindWithCatalogTestMode mo
             }
         }
     }
+
+    protected void recreateOrigManagementContextWithOsgi() {
+        // replace with OSGi context
+        Entities.destroyAll(origManagementContext);
+        try {
+            useOsgi = true;
+            origManagementContext = createOrigManagementContext();
+            origApp = createApp();
+        } finally {
+            useOsgi = false;
+        }
+    }
 
     @Test
-    public void testLongReferenceSequence() throws Exception {
+    public void testLongReferenceSequenceWithoutOsgi() throws Exception {
+        doTestLongReferenceSequence();
+    }
+    
+    @Test(groups="Broken")
+    public void testLongReferenceSequenceWithOsgi() throws Exception {
+        // won't work in OSGi mode because we need CAMP to resolve type "a0"
+        // (poor man's resolver only resolves Java classes)
+        recreateOrigManagementContextWithOsgi();
+        doTestLongReferenceSequence();
+    }
+    
+    private void doTestLongReferenceSequence() throws Exception {
         // adds a0, a1 extending a0, a2 extending a1, ... a9 extending a8
         // osgi rebind of types can fail because bundles are restored in any order
         // and dependencies might not yet be installed;
@@ -373,5 +408,23 @@ public void testLongReferenceSequence() throws Exception {
         Asserts.assertTrue(child instanceof BasicEntity);
         Asserts.assertEquals(child.getCatalogItemId(), "a9:1");
     }
-    
+
+    @Test
+    public void testDeleteEmptyBundleRemovedFromPersistence() throws Exception {
+        recreateOrigManagementContextWithOsgi();
+        
+        String bom = Strings.lines(
+                "brooklyn.catalog:",
+                "  itemType: entity",
+                "  items:",
+                "  - id: sample",
+                "    item:",
+                "      type: " + BasicEntity.class.getName());
+        addCatalogItems(bom);
+        addCatalogItems(bom);
+        rebind();
+        // should only contain one bundle / bundle.jar pair
+        Asserts.assertSize(Arrays.asList( new File(mementoDir, "bundles").list() ), 2);
+    }
+
 }

core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java

@@ -1858,7 +1858,7 @@ public void uninstallEmptyWrapperBundles() {
                 if (isNoBundleOrSimpleWrappingBundle(mgmt, b)) {
                     Iterable<RegisteredType> typesInBundle = osgi.get().getTypesFromBundle(b.getVersionedName());
                     if (Iterables.isEmpty(typesInBundle)) {
-                        log.debug("uninstalling empty wrapper bundle "+b);
+                        log.info("Uninstalling now-empty BOM wrapper bundle "+b.getVersionedName()+" ("+b.getOsgiUniqueUrl()+")");
                         osgi.get().uninstallUploadedBundle(b);
                     }
                 }

core/src/main/java/org/apache/brooklyn/core/catalog/internal/JavaCatalogToSpecTransformer.java

@@ -18,6 +18,7 @@
  */
 package org.apache.brooklyn.core.catalog.internal;
 
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.brooklyn.api.catalog.CatalogItem;
@@ -32,13 +33,18 @@
 import org.apache.brooklyn.api.sensor.EnricherSpec;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContextSequential;
+import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
 import org.apache.brooklyn.core.objs.BasicSpecParameter;
 import org.apache.brooklyn.core.plan.PlanToSpecTransformer;
 import org.apache.brooklyn.core.typereg.UnsupportedTypePlanException;
 import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.yaml.Yamls;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.Iterables;
+
 /**
  * Instantiates classes from a registered type which simply
  * defines the java class name and OSGi bundles to use.
@@ -76,12 +82,26 @@ public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT c
             CatalogItem<T, SpecT> item, Set<String> encounteredTypes) throws UnsupportedTypePlanException {
         @SuppressWarnings("deprecation")
         String javaType = item.getJavaType();
+        boolean poorMansCamp = false;
+        if (javaType == null) {
+            if (Strings.isNonBlank( item.getPlanYaml() )) {
+                try {
+                    Map<?,?> parsed = (Map<?,?>) Iterables.getOnlyElement( Yamls.parseAll(item.getPlanYaml()) );
+                    if ("type".equals(Iterables.getOnlyElement(parsed.keySet()))) {
+                        javaType = (String) Iterables.getOnlyElement(parsed.values());
+                        poorMansCamp = true;
+                    }
+                } catch (Exception e) {
+                    throw new UnsupportedTypePlanException(getClass().getName() + " parses only old-style catalog items containing only 'type: JavaClass' in YAML (or javaType in DTO)", e);
+                }
+            }
+        }
         if (javaType != null) {
-            // TODO This log.warn previously said "Using old-style xml catalog items with java type attribute".
-            // However, nothing in this code is specific to catalog.xml. Perhaps that is the only way this code
-            // can be reached? We should investigate further when we have time - if we can confirm it was only
-            // used for catalog.xml, then this can be deleted.
-            log.warn("Deprecated functionality (since 0.9.0). Using old-style java type attribute for " + item);
+            // TODO "JavaType" should never be used any more; but we do want to support a poor-man's camp
+            // for tests that expect CAMP in core where CAMP module isn't available
+            if (poorMansCamp) {
+                log.warn("Deprecated functionality (since 0.9.0). Using old-style java type attribute for " + item);
+            }
             Class<?> type;
             try {
                 // java types were deprecated before we added osgi support so this isn't necessary,
@@ -90,6 +110,7 @@ public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT c
                 final BrooklynClassLoadingContextSequential ctx = new BrooklynClassLoadingContextSequential(mgmt);
                 ctx.add(CatalogUtils.newClassLoadingContextForCatalogItems(mgmt, item.getCatalogItemId(),
                     item.getCatalogItemIdSearchPath()));
+                ctx.addSecondary(JavaBrooklynClassLoadingContext.create(mgmt));
                 type = ctx.loadClass(javaType);
             } catch (Exception e) {
                 Exceptions.propagateIfFatal(e);
@@ -117,7 +138,7 @@ public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT c
             SpecT untypedSpc = (SpecT) spec;
             return untypedSpc;
         } else {
-            throw new UnsupportedTypePlanException(getClass().getName() + " parses only old-style catalog items containing javaType");
+            throw new UnsupportedTypePlanException(getClass().getName() + " parses only old-style catalog items containing only 'type: JavaClass' or javaType in DTO");
         }
     }
 

core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java

@@ -379,8 +379,20 @@ public static Application rebind(ManagementContext newManagementContext, File me
     }
 
     public static Application rebind(RebindOptions options) throws Exception {
+        boolean hadApps = true;
+        if (options!=null && options.origManagementContext!=null && options.origManagementContext.getApplications().isEmpty()) {
+            // clearly had no apps before, so don't treat as an error
+            hadApps = false;
+        }
         Collection<Application> newApps = rebindAll(options);
-        if (newApps.isEmpty()) throw new IllegalStateException("Application could not be found after rebind; serialization probably failed");
+        if (newApps.isEmpty()) {
+            if (hadApps) {
+                throw new IllegalStateException("Application could not be found after rebind; serialization probably failed");
+            } else {
+                // no apps before; probably testing catalog
+                return null;
+            }
+        }
         Function<Collection<Application>, Application> chooser = options.applicationChooserOnRebind;
         if (chooser != null) {
             return chooser.apply(newApps);