You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by tb...@apache.org on 2017/10/17 09:19:31 UTC

[1/6] brooklyn-server git commit: BasicLauncher: improve test coverage

Repository: brooklyn-server
Updated Branches:
  refs/heads/master 7c6f7d416 -> efa4af664


BasicLauncher: improve test coverage


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/85991614
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/85991614
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/85991614

Branch: refs/heads/master
Commit: 85991614a652da20642788039682a45b93d91a8e
Parents: 4ef74fa
Author: Aled Sage <al...@gmail.com>
Authored: Fri Oct 6 09:32:49 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Oct 17 09:02:51 2017 +0100

----------------------------------------------------------------------
 launcher/pom.xml                                |   7 +
 .../AbstractBrooklynLauncherRebindTest.java     | 183 ++++++++++++++++
 .../BrooklynLauncherHighAvailabilityTest.java   | 118 ++--------
 .../BrooklynLauncherRebindAppsTest.java         |  42 ++++
 .../BrooklynLauncherRebindCatalogOsgiTest.java  | 219 +++++++++++++++++++
 .../BrooklynLauncherRebindCatalogTest.java      | 111 ++--------
 6 files changed, 488 insertions(+), 192 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/pom.xml
----------------------------------------------------------------------
diff --git a/launcher/pom.xml b/launcher/pom.xml
index 3b20978..c691fc2 100644
--- a/launcher/pom.xml
+++ b/launcher/pom.xml
@@ -214,6 +214,13 @@
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.brooklyn</groupId>
+            <artifactId>brooklyn-utils-common</artifactId>
+            <version>${project.version}</version>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
new file mode 100644
index 0000000..1406a7f
--- /dev/null
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.zip.ZipEntry;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.api.catalog.BrooklynCatalog;
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
+import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
+import org.apache.brooklyn.api.typereg.RegisteredType;
+import org.apache.brooklyn.core.mgmt.persist.PersistMode;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.core.osgi.BundleMaker;
+import org.apache.brooklyn.util.os.Os;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.text.Identifiers;
+import org.apache.brooklyn.util.time.Duration;
+import org.apache.commons.collections.IteratorUtils;
+import org.osgi.framework.Constants;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+
+import com.google.common.base.Charsets;
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.io.Files;
+
+public abstract class AbstractBrooklynLauncherRebindTest {
+
+    protected List<BrooklynLauncher> launchers = Lists.newCopyOnWriteArrayList();
+    protected List<File> tmpFiles = new ArrayList<>();
+
+    protected String persistenceDir;
+    
+    @BeforeMethod(alwaysRun=true)
+    public void setUp() throws Exception {
+        launchers.clear();
+        tmpFiles.clear();
+        persistenceDir = newTempPersistenceContainerName();
+    }
+
+    @AfterMethod(alwaysRun=true)
+    public void tearDown() throws Exception {
+        for (File file : tmpFiles) {
+            if (file.exists()) file.delete();
+        }
+        for (BrooklynLauncher launcher : launchers) {
+            launcher.terminate();
+        }
+        launchers.clear();
+        if (persistenceDir != null) Os.deleteRecursively(persistenceDir);
+    }
+
+    protected boolean useOsgi() {
+        return false;
+    }
+    
+    protected boolean reuseOsgi() {
+        return true;
+    }
+    
+    protected BrooklynLauncher newLauncherForTests() {
+        return newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.DISABLED);
+    }
+    
+    protected BrooklynLauncher newLauncherForTests(PersistMode persistMode, HighAvailabilityMode haMode) {
+        BrooklynLauncher launcher = BrooklynLauncher.newInstance()
+                .brooklynProperties(LocalManagementContextForTests.builder(true).setOsgiEnablementAndReuse(useOsgi(), reuseOsgi()).buildProperties())
+                .persistMode(persistMode)
+                .highAvailabilityMode(haMode)
+                .persistPeriod(Duration.millis(10))
+                .haHeartbeatPeriod(Duration.millis(10))
+                .persistenceDir(persistenceDir)
+                .webconsole(false);
+        launchers.add(launcher);
+        return launcher;
+    }
+
+    protected String newTempPersistenceContainerName() {
+        File persistenceDirF = Files.createTempDir();
+        Os.deleteOnExitRecursively(persistenceDirF);
+        return persistenceDirF.getAbsolutePath();
+    }
+    
+    protected File newTmpFile(String contents) throws Exception {
+        File file = java.nio.file.Files.createTempFile("brooklynLauncherRebindTest-"+Identifiers.makeRandomId(4), "txt").toFile();
+        tmpFiles.add(file);
+        Files.write(contents, file, Charsets.UTF_8);
+        return file;
+    }
+    
+    protected File newTmpBundle(Map<String, byte[]> files, VersionedName bundleName) {
+        Map<ZipEntry, InputStream> zipEntries = new LinkedHashMap<>();
+        for (Map.Entry<String, byte[]> entry : files.entrySet()) {
+            zipEntries.put(new ZipEntry(entry.getKey()), new ByteArrayInputStream(entry.getValue()));
+        }
+        
+        BundleMaker bundleMaker = new BundleMaker(new ResourceUtils(this));
+        File bf = bundleMaker.createTempZip("test", zipEntries);
+        tmpFiles.add(bf);
+        
+        if (bundleName!=null) {
+            bf = bundleMaker.copyAddingManifest(bf, MutableMap.of(
+                "Manifest-Version", "2.0",
+                Constants.BUNDLE_SYMBOLICNAME, bundleName.getSymbolicName(),
+                Constants.BUNDLE_VERSION, bundleName.getOsgiVersion().toString()));
+            tmpFiles.add(bf);
+        }
+        return bf;
+        
+//        ReferenceWithError<OsgiBundleInstallationResult> b = ((ManagementContextInternal)mgmt).getOsgiManager().get().install(
+//                new FileInputStream(bf) );
+//
+//            b.checkNoError();
+
+    }
+    
+    protected void assertCatalogConsistsOfIds(BrooklynLauncher launcher, Iterable<VersionedName> ids) {
+        BrooklynTypeRegistry typeRegistry = launcher.getServerDetails().getManagementContext().getTypeRegistry();
+        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
+        assertTypeRegistryConsistsOfIds(typeRegistry.getAll(), ids);
+        assertCatalogConsistsOfIds(catalog.getCatalogItems(), ids);
+    }
+
+    protected void assertCatalogConsistsOfIds(Iterable<CatalogItem<Object, Object>> catalogItems, Iterable<VersionedName> ids) {
+        Iterable<VersionedName> idsFromItems = Iterables.transform(catalogItems, new Function<CatalogItem<?,?>, VersionedName>() {
+            @Nullable
+            @Override
+            public VersionedName apply(CatalogItem<?, ?> input) {
+                return VersionedName.fromString(input.getCatalogItemId());
+            }
+        });
+        Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));
+    }
+
+    protected void assertTypeRegistryConsistsOfIds(Iterable<RegisteredType> types, Iterable<VersionedName> ids) {
+        Iterable<VersionedName> idsFromItems = Iterables.transform(types, new Function<RegisteredType, VersionedName>() {
+            @Nullable
+            @Override
+            public VersionedName apply(RegisteredType input) {
+                return input.getVersionedName();
+            }
+        });
+        Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));
+    }
+
+    private static <T> boolean compareIterablesWithoutOrderMatters(Iterable<T> a, Iterable<T> b) {
+        List<T> aList = IteratorUtils.toList(a.iterator());
+        List<T> bList = IteratorUtils.toList(b.iterator());
+
+        return aList.containsAll(bList) && bList.containsAll(aList);
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherHighAvailabilityTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherHighAvailabilityTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherHighAvailabilityTest.java
index 6bc2b02..441af8b 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherHighAvailabilityTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherHighAvailabilityTest.java
@@ -18,68 +18,36 @@
  */
 package org.apache.brooklyn.launcher;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
 import org.apache.brooklyn.api.mgmt.ha.ManagementPlaneSyncRecordPersister;
-import org.apache.brooklyn.core.internal.BrooklynProperties;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.mgmt.persist.PersistMode;
-import org.apache.brooklyn.core.mgmt.rebind.RebindTestUtils;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.core.test.entity.TestApplication;
-import org.apache.brooklyn.launcher.BrooklynLauncher;
 import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.time.Duration;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertTrue;
-import static org.testng.Assert.fail;
-
-import java.io.File;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Predicates;
 import com.google.common.collect.Iterables;
-import com.google.common.io.Files;
 
-public class BrooklynLauncherHighAvailabilityTest {
+public class BrooklynLauncherHighAvailabilityTest extends AbstractBrooklynLauncherRebindTest {
 
     private static final Logger log = LoggerFactory.getLogger(BrooklynLauncherHighAvailabilityTest.class);
     
-    private static final Duration TIMEOUT = Duration.THIRTY_SECONDS;
-    
     private BrooklynLauncher primary;
     private BrooklynLauncher secondary;
     private BrooklynLauncher tertiary;
-    private File persistenceDir;
 
-    @BeforeMethod(alwaysRun=true)
-    public void setUp() throws Exception {
-        persistenceDir = Files.createTempDir();
-        Os.deleteOnExitRecursively(persistenceDir);
-    }
-
-    @AfterMethod(alwaysRun=true)
-    public void tearDown() throws Exception {
-        if (primary != null) primary.terminate();
-        primary = null;
-        if (secondary != null) secondary.terminate();
-        secondary = null;
-        if (tertiary != null) tertiary.terminate();
-        tertiary = null;
-        if (persistenceDir != null) RebindTestUtils.deleteMementoDir(persistenceDir);
-        persistenceDir = null;
-    }
-    
     @Test
     public void testStandbyTakesOverWhenPrimaryTerminatedGracefully() throws Exception {
         doTestStandbyTakesOver(true);
@@ -100,39 +68,25 @@ public class BrooklynLauncherHighAvailabilityTest {
     
     protected void doTestStandbyTakesOver(boolean stopGracefully) throws Exception {
         log.info("STARTING standby takeover test");
-        primary = BrooklynLauncher.newInstance();
-        primary.webconsole(false)
-                .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                .highAvailabilityMode(HighAvailabilityMode.AUTO)
-                .persistMode(PersistMode.AUTO)
-                .persistenceDir(persistenceDir)
-                .persistPeriod(Duration.millis(10))
-                .haHeartbeatPeriod(Duration.millis(10))
+        primary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.AUTO)
                 .haHeartbeatTimeout(Duration.millis(1000))
-                .application(EntitySpec.create(TestApplication.class))
                 .start();
         ManagementContext primaryManagementContext = primary.getServerDetails().getManagementContext();
+        TestApplication origApp = primaryManagementContext.getEntityManager().createEntity(EntitySpec.create(TestApplication.class));
         log.info("started mgmt primary "+primaryManagementContext);
         
-        assertOnlyApp(primary.getServerDetails().getManagementContext(), TestApplication.class);
-        primaryManagementContext.getRebindManager().getPersister().waitForWritesCompleted(TIMEOUT);
+        assertOnlyApp(primaryManagementContext, TestApplication.class);
+        primaryManagementContext.getRebindManager().getPersister().waitForWritesCompleted(Asserts.DEFAULT_LONG_TIMEOUT);
         
         // Secondary will come up as standby
-        secondary = BrooklynLauncher.newInstance();
-        secondary.webconsole(false)
-                .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                .highAvailabilityMode(HighAvailabilityMode.AUTO)
-                .persistMode(PersistMode.AUTO)
-                .persistenceDir(persistenceDir)
-                .persistPeriod(Duration.millis(10))
-                .haHeartbeatPeriod(Duration.millis(10))
+        secondary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.AUTO)
                 .haHeartbeatTimeout(Duration.millis(1000))
                 .start();
         ManagementContext secondaryManagementContext = secondary.getServerDetails().getManagementContext();
         log.info("started mgmt secondary "+secondaryManagementContext);
         
-        // TODO can assert it sees the apps read only
-//        assertNoApps(secondary.getServerDetails().getManagementContext());
+        // In standby (rather than hot-standby) it will not read the persisted state of apps
+        assertNoApps(secondary.getServerDetails().getManagementContext());
 
         // Terminate primary; expect secondary to take over
         if (stopGracefully) {
@@ -146,14 +100,7 @@ public class BrooklynLauncherHighAvailabilityTest {
         assertOnlyAppEventually(secondaryManagementContext, TestApplication.class);
         
         // Start tertiary (force up as standby)
-        tertiary = BrooklynLauncher.newInstance();
-        tertiary.webconsole(false)
-                .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                .highAvailabilityMode(HighAvailabilityMode.STANDBY)
-                .persistMode(PersistMode.AUTO)
-                .persistenceDir(persistenceDir)
-                .persistPeriod(Duration.millis(10))
-                .haHeartbeatPeriod(Duration.millis(10))
+        tertiary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.STANDBY)
                 .haHeartbeatTimeout(Duration.millis(1000))
                 .start();
         ManagementContext tertiaryManagementContext = tertiary.getServerDetails().getManagementContext();
@@ -174,27 +121,14 @@ public class BrooklynLauncherHighAvailabilityTest {
     }
     
     public void testHighAvailabilityMasterModeFailsIfAlreadyHasMaster() throws Exception {
-        primary = BrooklynLauncher.newInstance();
-        primary.webconsole(false)
-                .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                .highAvailabilityMode(HighAvailabilityMode.AUTO)
-                .persistMode(PersistMode.AUTO)
-                .persistenceDir(persistenceDir)
-                .persistPeriod(Duration.millis(10))
-                .application(EntitySpec.create(TestApplication.class))
+        primary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.AUTO)
                 .start();
 
         try {
             // Secondary will come up as standby
-            secondary = BrooklynLauncher.newInstance();
-            secondary.webconsole(false)
-                    .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                    .highAvailabilityMode(HighAvailabilityMode.MASTER)
-                    .persistMode(PersistMode.AUTO)
-                    .persistenceDir(persistenceDir)
-                    .persistPeriod(Duration.millis(10))
+            secondary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.MASTER)
                     .start();
-            fail();
+            Asserts.shouldHaveFailedPreviously();
         } catch (IllegalStateException e) {
             // success
         }
@@ -203,15 +137,8 @@ public class BrooklynLauncherHighAvailabilityTest {
     @Test
     public void testHighAvailabilityStandbyModeFailsIfNoExistingMaster() throws Exception {
         try {
-            primary = BrooklynLauncher.newInstance();
-            primary.webconsole(false)
-                    .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                    .highAvailabilityMode(HighAvailabilityMode.STANDBY)
-                    .persistMode(PersistMode.AUTO)
-                    .persistenceDir(persistenceDir)
-                    .persistPeriod(Duration.millis(10))
+            primary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.STANDBY)
                     .ignorePersistenceErrors(false)
-                    .application(EntitySpec.create(TestApplication.class))
                     .start();
             fail();
         } catch (IllegalStateException e) {
@@ -222,15 +149,8 @@ public class BrooklynLauncherHighAvailabilityTest {
     @Test
     public void testHighAvailabilityHotStandbyModeFailsIfNoExistingMaster() throws Exception {
         try {
-            primary = BrooklynLauncher.newInstance();
-            primary.webconsole(false)
-                    .brooklynProperties(LocalManagementContextForTests.setEmptyCatalogAsDefault(BrooklynProperties.Factory.newEmpty()))
-                    .highAvailabilityMode(HighAvailabilityMode.HOT_STANDBY)
-                    .persistMode(PersistMode.AUTO)
-                    .persistenceDir(persistenceDir)
-                    .persistPeriod(Duration.millis(10))
+            primary = newLauncherForTests(PersistMode.AUTO, HighAvailabilityMode.HOT_STANDBY)
                     .ignorePersistenceErrors(false)
-                    .application(EntitySpec.create(TestApplication.class))
                     .start();
             fail();
         } catch (IllegalStateException e) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindAppsTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindAppsTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindAppsTest.java
new file mode 100644
index 0000000..976cfd3
--- /dev/null
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindAppsTest.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher;
+
+import static org.testng.Assert.assertEquals;
+
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.Iterables;
+
+public class BrooklynLauncherRebindAppsTest extends AbstractBrooklynLauncherRebindTest {
+
+    @Test
+    public void testRebindGetsApps() {
+        BrooklynLauncher origLauncher = newLauncherForTests();
+        origLauncher.start();
+        TestApplication origApp = origLauncher.getManagementContext().getEntityManager().createEntity(EntitySpec.create(TestApplication.class));
+        origLauncher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests();
+        newLauncher.start();
+        assertEquals(Iterables.getOnlyElement(newLauncher.getManagementContext().getApplications()).getId(), origApp.getId());
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
new file mode 100644
index 0000000..c52fec4
--- /dev/null
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
@@ -0,0 +1,219 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.apache.brooklyn.api.typereg.ManagedBundle;
+import org.apache.brooklyn.api.typereg.RegisteredType;
+import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
+import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.test.support.TestResourceUnavailableException;
+import org.apache.brooklyn.util.osgi.OsgiTestResources;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Joiner;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
+public class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBrooklynLauncherRebindTest {
+
+    private static final VersionedName COM_EXAMPLE_BUNDLE_ID = new VersionedName(
+            OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_SYMBOLIC_NAME_FULL, 
+            OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_VERSION);
+    
+    private static final Set<VersionedName> COM_EXAMPLE_BUNDLE_CATALOG_IDS = ImmutableSet.of(
+            new VersionedName("com.example.simpleTest", "0.0.0-SNAPSHOT"));
+
+    @BeforeMethod(alwaysRun=true)
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+    }
+    
+    @Override
+    protected boolean useOsgi() {
+        return true;
+    }
+    
+    @Override
+    protected boolean reuseOsgi() {
+        return false;
+    }
+
+    private BrooklynLauncher newLauncherForTests(String catalogInitial) {
+        CatalogInitialization catalogInitialization = new CatalogInitialization(catalogInitial);
+        return super.newLauncherForTests()
+                .catalogInitialization(catalogInitialization);
+    }
+
+    @Test
+    public void testRebindGetsInitialOsgiCatalog() throws Exception {
+        Set<VersionedName> bundleItems = ImmutableSet.of(VersionedName.fromString("one:1.0.0"));
+        String bundleBom = createCatalogYaml(ImmutableList.of(), bundleItems);
+        VersionedName bundleName = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "1.0.0");
+        File bundleFile = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, bundleBom.getBytes(StandardCharsets.UTF_8)), bundleName);
+        File initialBomFile = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFile.toURI()), ImmutableList.of()));
+        
+        BrooklynLauncher launcher = newLauncherForTests(initialBomFile.getAbsolutePath());
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, bundleItems);
+        assertManagedBundle(launcher, bundleName, bundleItems);
+
+        launcher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests(initialBomFile.getAbsolutePath());
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, bundleItems);
+        assertManagedBundle(newLauncher, bundleName, bundleItems);
+    }
+
+    @Test
+    public void testRebindUpgradesBundleWithSameItems() throws Exception {
+        Set<VersionedName> bundleItems = ImmutableSet.of(VersionedName.fromString("one:1.0.0"));
+        String bundleBom = createCatalogYaml(ImmutableList.of(), bundleItems);
+        
+        VersionedName bundleNameV1 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "1.0.0");
+        File bundleFileV1 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, bundleBom.getBytes(StandardCharsets.UTF_8)), bundleNameV1);
+        File initialBomFileV1 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV1.toURI()), ImmutableList.of()));
+
+        VersionedName bundleNameV2 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "2.0.0");
+        File bundleFileV2 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, bundleBom.getBytes(StandardCharsets.UTF_8)), bundleNameV2);
+        File initialBomFileV2 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV2.toURI()), ImmutableList.of()));
+        
+        BrooklynLauncher launcher = newLauncherForTests(initialBomFileV1.getAbsolutePath());
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, bundleItems);
+        assertManagedBundle(launcher, bundleNameV1, bundleItems);
+
+        launcher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests(initialBomFileV2.getAbsolutePath());
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, bundleItems);
+        assertManagedBundle(newLauncher, bundleNameV2, bundleItems);
+    }
+
+    @Test
+    public void testRebindUpgradesBundleWithNewerItems() throws Exception {
+        Set<VersionedName> bundleItemsV1 = ImmutableSet.of(VersionedName.fromString("one:1.0.0"));
+        String bundleBomV1 = createCatalogYaml(ImmutableList.of(), bundleItemsV1);
+        VersionedName bundleNameV1 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "1.0.0");
+        File bundleFileV1 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, bundleBomV1.getBytes(StandardCharsets.UTF_8)), bundleNameV1);
+        File initialBomFileV1 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV1.toURI()), ImmutableList.of()));
+
+        Set<VersionedName> bundleItemsV2 = ImmutableSet.of(VersionedName.fromString("one:2.0.0"));
+        String bundleBomV2 = createCatalogYaml(ImmutableList.of(), bundleItemsV2);
+        VersionedName bundleNameV2 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "2.0.0");
+        File bundleFileV2 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, bundleBomV2.getBytes(StandardCharsets.UTF_8)), bundleNameV2);
+        File initialBomFileV2 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV2.toURI()), ImmutableList.of()));
+        
+        BrooklynLauncher launcher = newLauncherForTests(initialBomFileV1.getAbsolutePath());
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, bundleItemsV1);
+        assertManagedBundle(launcher, bundleNameV1, bundleItemsV1);
+
+        launcher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests(initialBomFileV2.getAbsolutePath());
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, Iterables.concat(bundleItemsV1, bundleItemsV2));
+        assertManagedBundle(newLauncher, bundleNameV1, bundleItemsV1);
+        assertManagedBundle(newLauncher, bundleNameV2, bundleItemsV2);
+    }
+
+    // TODO bundle fails to start: 
+    //     missing requirement [com.example.brooklyn.test.resources.osgi.brooklyn-test-osgi-com-example-entities [147](R 147.0)] 
+    //     osgi.wiring.package; (&(osgi.wiring.package=org.apache.brooklyn.config)(version>=0.12.0)(!(version>=1.0.0)))
+    // Presumably because brooklyn-api is just on the classpath rather than in osgi.
+    // How did such tests work previously?!
+    @Test(groups="Broken")
+    public void testRebindGetsInitialOsgiCatalogWithJava() throws Exception {
+        TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_PATH);
+        
+        String initialBom = Joiner.on("\n").join(
+                "brooklyn.catalog:",
+                "  brooklyn.libraries:",
+                "    - " + OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_URL);
+        File initialBomFile = newTmpFile(initialBom);
+        
+        BrooklynLauncher launcher = newLauncherForTests(initialBomFile.getAbsolutePath());
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, COM_EXAMPLE_BUNDLE_CATALOG_IDS);
+        assertManagedBundle(launcher, COM_EXAMPLE_BUNDLE_ID, COM_EXAMPLE_BUNDLE_CATALOG_IDS);
+
+        launcher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests(initialBomFile.getAbsolutePath());
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, COM_EXAMPLE_BUNDLE_CATALOG_IDS);
+        assertManagedBundle(newLauncher, COM_EXAMPLE_BUNDLE_ID, COM_EXAMPLE_BUNDLE_CATALOG_IDS);
+    }
+    
+    private void assertManagedBundle(BrooklynLauncher launcher, VersionedName bundleId, Set<VersionedName> expectedCatalogItems) {
+        ManagementContextInternal mgmt = (ManagementContextInternal)launcher.getManagementContext();
+        ManagedBundle bundle = mgmt.getOsgiManager().get().getManagedBundle(bundleId);
+        assertNotNull(bundle, bundleId+" not found");
+        
+        Set<VersionedName> actualCatalogItems = new LinkedHashSet<>();
+        Iterable<RegisteredType> types = launcher.getManagementContext().getTypeRegistry().getAll();
+        for (RegisteredType type : types) {
+            if (Objects.equal(bundleId.toOsgiString(), type.getContainingBundle())) {
+                actualCatalogItems.add(type.getVersionedName());
+            }
+        }
+        assertEquals(actualCatalogItems, expectedCatalogItems, "actual="+actualCatalogItems+"; expected="+expectedCatalogItems);
+    }
+    
+    private String createCatalogYaml(Iterable<URI> libraries, Iterable<VersionedName> entities) {
+        StringBuilder result = new StringBuilder();
+        result.append("brooklyn.catalog:\n");
+        if (!Iterables.isEmpty(libraries)) {
+            result.append("  brooklyn.libraries:\n");
+        }
+        for (URI library : libraries) {
+            result.append("    - " + library+"\n");
+        }
+        if (!Iterables.isEmpty(entities)) {
+            result.append("  items:\n");
+        }
+        for (VersionedName entity : entities) {
+            result.append("    - id: " + entity.getSymbolicName()+"\n");
+            result.append("      version: " + entity.getVersionString()+"\n");
+            result.append("      itemType: entity"+"\n");
+            result.append("      item:"+"\n");
+            result.append("        type: " + BasicEntity.class.getName()+"\n");
+        }
+        return result.toString();
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/85991614/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
index bafb641..611096a 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
@@ -18,7 +18,6 @@
  */
 package org.apache.brooklyn.launcher;
 
-import java.io.File;
 import java.util.List;
 import java.util.Set;
 
@@ -29,92 +28,61 @@ import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
-import org.apache.brooklyn.core.mgmt.persist.PersistMode;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.ResourceUtils;
-import org.apache.brooklyn.util.os.Os;
+import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.commons.collections.IteratorUtils;
 import org.testng.Assert;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.io.Files;
 
-public class BrooklynLauncherRebindCatalogTest {
+public class BrooklynLauncherRebindCatalogTest extends AbstractBrooklynLauncherRebindTest {
 
     private static final String TEST_VERSION = "test-version";
     private static final String CATALOG_INITIAL = "classpath://rebind-test-catalog.bom";
     private static final String CATALOG_EMPTY_INITIAL = "classpath://rebind-test-empty-catalog.bom";
     private static final String CATALOG_ADDITIONS = "rebind-test-catalog-additions.bom";
-    private static final Set<String> EXPECTED_DEFAULT_IDS = ImmutableSet.of("one:" + TEST_VERSION, "two:" + TEST_VERSION);
-    private static final Set<String> EXPECTED_ADDED_IDS = ImmutableSet.of("three:" + TEST_VERSION, "four:" + TEST_VERSION);
-
-    private List<BrooklynLauncher> launchers = Lists.newCopyOnWriteArrayList();
-    
-    @AfterMethod(alwaysRun=true)
-    public void tearDown() throws Exception {
-        for (BrooklynLauncher launcher : launchers) {
-            launcher.terminate();
-        }
-        launchers.clear();
-    }
+    private static final Set<VersionedName> EXPECTED_DEFAULT_IDS = ImmutableSet.of(new VersionedName("one", TEST_VERSION), new VersionedName("two", TEST_VERSION));
+    private static final Set<VersionedName> EXPECTED_ADDED_IDS = ImmutableSet.of(new VersionedName("three", TEST_VERSION), new VersionedName("four", TEST_VERSION));
 
-    private BrooklynLauncher newLauncherForTests(String persistenceDir) {
-        return newLauncherForTests(persistenceDir, CATALOG_INITIAL);
-    }
-    
-    private BrooklynLauncher newLauncherForTests(String persistenceDir, String catalogInitial) {
+    private BrooklynLauncher newLauncherForTests(String catalogInitial) {
         CatalogInitialization catalogInitialization = new CatalogInitialization(catalogInitial);
-        BrooklynLauncher launcher = BrooklynLauncher.newInstance()
-                .brooklynProperties(LocalManagementContextForTests.builder(true).buildProperties())
-                .catalogInitialization(catalogInitialization)
-                .persistMode(PersistMode.AUTO)
-                .persistenceDir(persistenceDir)
-                .webconsole(false);
-        launchers.add(launcher);
-        return launcher;
+        return super.newLauncherForTests()
+                .catalogInitialization(catalogInitialization);
     }
 
     @Test
     public void testRebindGetsInitialCatalog() {
-        String persistenceDir = newTempPersistenceContainerName();
-
-        BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher launcher = newLauncherForTests(CATALOG_INITIAL);
         launcher.start();
         assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
 
         launcher.terminate();
 
-        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher newLauncher = newLauncherForTests(CATALOG_INITIAL);
         newLauncher.start();
         assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS);
     }
 
     @Test
     public void testRebindPersistsInitialCatalog() {
-        String persistenceDir = newTempPersistenceContainerName();
-
-        BrooklynLauncher launcher = newLauncherForTests(persistenceDir, CATALOG_INITIAL);
+        BrooklynLauncher launcher = newLauncherForTests(CATALOG_INITIAL);
         launcher.start();
         assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
 
         launcher.terminate();
 
-        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir, CATALOG_EMPTY_INITIAL);
+        BrooklynLauncher newLauncher = newLauncherForTests(CATALOG_EMPTY_INITIAL);
         newLauncher.start();
         assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS);
     }
 
     @Test
     public void testRebindGetsUnionOfInitialAndPersisted() {
-        String persistenceDir = newTempPersistenceContainerName();
-
-        BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher launcher = newLauncherForTests(CATALOG_INITIAL);
         launcher.start();
         assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
 
@@ -124,7 +92,7 @@ public class BrooklynLauncherRebindCatalogTest {
 
         launcher.terminate();
 
-        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher newLauncher = newLauncherForTests(CATALOG_INITIAL);
         newLauncher.start();
         assertCatalogConsistsOfIds(newLauncher, Iterables.concat(EXPECTED_DEFAULT_IDS, EXPECTED_ADDED_IDS));
     }
@@ -136,13 +104,12 @@ public class BrooklynLauncherRebindCatalogTest {
     // persisted state.
     @Test(groups="Broken")
     public void testRemovedInitialItemStillRemovedAfterRebind() {
-        Set<String> EXPECTED_DEFAULT_IDS_WITHOUT_ONE = MutableSet.<String>builder()
+        Set<VersionedName> EXPECTED_DEFAULT_IDS_WITHOUT_ONE = MutableSet.<VersionedName>builder()
                 .addAll(EXPECTED_DEFAULT_IDS)
-                .remove("one:" + TEST_VERSION).build();
+                .remove(new VersionedName("one", TEST_VERSION))
+                .build();
         
-        String persistenceDir = newTempPersistenceContainerName();
-
-        BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher launcher = newLauncherForTests(CATALOG_INITIAL);
         launcher.start();
 
         BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
@@ -151,50 +118,8 @@ public class BrooklynLauncherRebindCatalogTest {
         
         launcher.terminate();
 
-        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
+        BrooklynLauncher newLauncher = newLauncherForTests(CATALOG_INITIAL);
         newLauncher.start();
         assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS_WITHOUT_ONE);
     }
-
-    private void assertCatalogConsistsOfIds(BrooklynLauncher launcher, Iterable<String> ids) {
-        BrooklynTypeRegistry typeRegistry = launcher.getServerDetails().getManagementContext().getTypeRegistry();
-        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
-        assertTypeRegistryConsistsOfIds(typeRegistry.getAll(), ids);
-        assertCatalogConsistsOfIds(catalog.getCatalogItems(), ids);
-    }
-
-    private void assertCatalogConsistsOfIds(Iterable<CatalogItem<Object, Object>> catalogItems, Iterable<String> ids) {
-        Iterable<String> idsFromItems = Iterables.transform(catalogItems, new Function<CatalogItem<?,?>, String>() {
-            @Nullable
-            @Override
-            public String apply(CatalogItem<?, ?> input) {
-                return input.getCatalogItemId();
-            }
-        });
-        Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));
-    }
-
-    private void assertTypeRegistryConsistsOfIds(Iterable<RegisteredType> types, Iterable<String> ids) {
-        Iterable<String> idsFromItems = Iterables.transform(types, new Function<RegisteredType, String>() {
-            @Nullable
-            @Override
-            public String apply(RegisteredType input) {
-                return input.getId();
-            }
-        });
-        Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));
-    }
-
-    protected String newTempPersistenceContainerName() {
-        File persistenceDirF = Files.createTempDir();
-        Os.deleteOnExitRecursively(persistenceDirF);
-        return persistenceDirF.getAbsolutePath();
-    }
-
-    private static <T> boolean compareIterablesWithoutOrderMatters(Iterable<T> a, Iterable<T> b) {
-        List<T> aList = IteratorUtils.toList(a.iterator());
-        List<T> bList = IteratorUtils.toList(b.iterator());
-
-        return aList.containsAll(bList) && bList.containsAll(aList);
-    }
 }


[5/6] brooklyn-server git commit: PR #852: incorporate review comments

Posted by tb...@apache.org.
PR #852: incorporate review comments

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/48d99991
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/48d99991
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/48d99991

Branch: refs/heads/master
Commit: 48d99991654f8add08c4025a66be8e7c97678620
Parents: 8599161
Author: Aled Sage <al...@gmail.com>
Authored: Tue Oct 17 09:38:46 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Oct 17 09:38:46 2017 +0100

----------------------------------------------------------------------
 .../org/apache/brooklyn/api/catalog/BrooklynCatalog.java    | 4 ++--
 .../core/catalog/internal/CatalogInitialization.java        | 2 +-
 .../core/mgmt/rebind/PeriodicDeltaChangeListener.java       | 2 --
 .../launcher/AbstractBrooklynLauncherRebindTest.java        | 6 ------
 .../launcher/BrooklynLauncherRebindCatalogTest.java         | 9 ---------
 launcher/src/test/resources/rebind-test-empty-catalog.bom   | 3 ---
 server-cli/src/main/java/org/apache/brooklyn/cli/Main.java  | 2 +-
 7 files changed, 4 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/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 cb305ef..5f98c03 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
@@ -217,8 +217,8 @@ public interface BrooklynCatalog {
      * just adds without removing the existing content. Note this is very different from 
      * {@link #addItem(CatalogItem)}, which adds to the 'manual' catalog.
      *
-     * @since 0.13.0 (only for legacy backwards compatibility)
-     * @deprecated since 0.13.0; instead use bundles in persisted state!
+     * @since 1.0.0 (only for legacy backwards compatibility)
+     * @deprecated since 1.0.0; instead use bundles in persisted state!
      */
     @Deprecated
     void addCatalogLegacyItemsOnRebind(Iterable<? extends CatalogItem<?,?>> items);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
index e11b64d..58999a2 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
@@ -159,7 +159,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
     }
     
     private boolean hasRunInitialCatalogInitialization() {
-        return hasRunInitialCatalogInitialization;
+        return hasRunFinalInitialization || hasRunInitialCatalogInitialization;
     }
     
     /**

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/PeriodicDeltaChangeListener.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/PeriodicDeltaChangeListener.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/PeriodicDeltaChangeListener.java
index 5f5e50a..a5af458 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/PeriodicDeltaChangeListener.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/PeriodicDeltaChangeListener.java
@@ -44,11 +44,9 @@ import org.apache.brooklyn.api.sensor.Feed;
 import org.apache.brooklyn.api.typereg.ManagedBundle;
 import org.apache.brooklyn.core.BrooklynFeatureEnablement;
 import org.apache.brooklyn.core.entity.EntityInternal;
-import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.mgmt.persist.BrooklynPersistenceUtils;
 import org.apache.brooklyn.core.mgmt.persist.PersistenceActivityMetrics;
 import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
-import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.task.ScheduledTask;
 import org.apache.brooklyn.util.core.task.Tasks;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
index 1406a7f..6b13313 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractBrooklynLauncherRebindTest.java
@@ -137,12 +137,6 @@ public abstract class AbstractBrooklynLauncherRebindTest {
             tmpFiles.add(bf);
         }
         return bf;
-        
-//        ReferenceWithError<OsgiBundleInstallationResult> b = ((ManagementContextInternal)mgmt).getOsgiManager().get().install(
-//                new FileInputStream(bf) );
-//
-//            b.checkNoError();
-
     }
     
     protected void assertCatalogConsistsOfIds(BrooklynLauncher launcher, Iterable<VersionedName> ids) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
index 611096a..1b80887 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
@@ -18,24 +18,15 @@
  */
 package org.apache.brooklyn.launcher;
 
-import java.util.List;
 import java.util.Set;
 
-import javax.annotation.Nullable;
-
 import org.apache.brooklyn.api.catalog.BrooklynCatalog;
-import org.apache.brooklyn.api.catalog.CatalogItem;
-import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
-import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.osgi.VersionedName;
-import org.apache.commons.collections.IteratorUtils;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
-import com.google.common.base.Function;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/launcher/src/test/resources/rebind-test-empty-catalog.bom
----------------------------------------------------------------------
diff --git a/launcher/src/test/resources/rebind-test-empty-catalog.bom b/launcher/src/test/resources/rebind-test-empty-catalog.bom
index 53b914f..d13bd31 100644
--- a/launcher/src/test/resources/rebind-test-empty-catalog.bom
+++ b/launcher/src/test/resources/rebind-test-empty-catalog.bom
@@ -20,6 +20,3 @@ brooklyn.catalog:
   version: "test-version"
   itemType: entity
   items:
-
-  # Do not scan classpath with for classes with a @Catalog annotation
-  - scanJavaAnnotations: false

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/48d99991/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 9d037f6..8d20d59 100644
--- a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -592,7 +592,7 @@ public class Main extends AbstractMain {
         /**
          * method intended for subclassing, to add custom items to the catalog.
          * 
-         * @deprecated since 0.13.0; no longer supported; does nothing - subclasses should not try to extend it!
+         * @deprecated since 1.0.0; no longer supported; does nothing - subclasses should not try to extend it!
          */
         protected final void populateCatalog(BrooklynCatalog catalog) {
             // nothing else added here


[4/6] brooklyn-server git commit: Merge initial catalog and persisted catalog

Posted by tb...@apache.org.
Merge initial catalog and persisted catalog


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/802cbb9b
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/802cbb9b
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/802cbb9b

Branch: refs/heads/master
Commit: 802cbb9b2219976cc87a50d45dd61502b7a09a68
Parents: 7c6f7d4
Author: Aled Sage <al...@gmail.com>
Authored: Mon Oct 2 18:30:10 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Oct 17 09:02:51 2017 +0100

----------------------------------------------------------------------
 .../brooklyn/api/catalog/BrooklynCatalog.java   |  11 +
 .../catalog/internal/BasicBrooklynCatalog.java  |  36 ++
 .../catalog/internal/CatalogInitialization.java | 462 +++++++++++--------
 .../internal/AbstractManagementContext.java     |   8 +-
 .../mgmt/internal/LocalManagementContext.java   |   4 -
 .../core/mgmt/rebind/RebindIteration.java       |  37 +-
 .../ha/HighAvailabilityManagerTestFixture.java  |   3 +-
 .../mgmt/rebind/RebindCatalogEntityTest.java    |   8 +-
 .../core/mgmt/rebind/RebindEntityTest.java      |   2 +
 .../rebind/RebindHistoricCatalogItemTest.java   |  53 +++
 .../core/mgmt/rebind/RebindTestFixture.java     |  11 -
 .../core/mgmt/rebind/RebindTestUtils.java       |   3 +
 .../entity/LocalManagementContextForTests.java  |  12 +
 .../core/mgmt/rebind/catalog-myapp_1.0.0        |  36 ++
 .../brooklyn/launcher/common/BasicLauncher.java | 125 ++---
 .../BrooklynLauncherRebindCatalogTest.java      | 120 ++++-
 .../brooklyn/launcher/BrooklynLauncherTest.java |  10 +-
 .../resources/rebind-test-empty-catalog.bom     |  25 +
 .../rest/testing/BrooklynRestApiTest.java       |   4 +-
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |   3 +-
 .../brooklyn/rest/HaMasterCheckFilterTest.java  |   9 +-
 .../main/java/org/apache/brooklyn/cli/Main.java |  76 +--
 22 files changed, 632 insertions(+), 426 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/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 e03adbf..cb305ef 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
@@ -213,6 +213,17 @@ public interface BrooklynCatalog {
     void addItem(CatalogItem<?,?> item);
 
     /**
+     * adds the given items to the catalog, similar to {@link #reset(Collection)} but where it 
+     * just adds without removing the existing content. Note this is very different from 
+     * {@link #addItem(CatalogItem)}, which adds to the 'manual' catalog.
+     *
+     * @since 0.13.0 (only for legacy backwards compatibility)
+     * @deprecated since 0.13.0; instead use bundles in persisted state!
+     */
+    @Deprecated
+    void addCatalogLegacyItemsOnRebind(Iterable<? extends CatalogItem<?,?>> items);
+
+    /**
      * Creates a catalog item and adds it to the 'manual' catalog,
      * with the corresponding Class definition (loaded by a classloader)
      * registered and available in the classloader.

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/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 4126d5d..fe0bffe 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
@@ -1826,6 +1826,42 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
 
     @Override @Deprecated /** @deprecated see super */
+    public void addCatalogLegacyItemsOnRebind(Iterable<? extends CatalogItem<?,?>> items) {
+        addCatalogLegacyItemsOnRebind(items, true);
+    }
+    
+    private void addCatalogLegacyItemsOnRebind(Iterable<? extends CatalogItem<?,?>> items, boolean failOnLoadError) {
+        specCache.invalidate();
+        
+        log.debug("Adding manual catalog items to "+mgmt+": "+items);
+        checkNotNull(items, "item");
+        for (CatalogItem<?,?> item : items) {
+            CatalogItemDtoAbstract<?,?> cdto;
+            if (item instanceof CatalogItemDtoAbstract) {
+                cdto = (CatalogItemDtoAbstract<?,?>) item;
+            } else if (item instanceof CatalogItemDo && ((CatalogItemDo<?,?>)item).getDto() instanceof CatalogItemDtoAbstract) {
+                cdto = (CatalogItemDtoAbstract<?,?>) ((CatalogItemDo<?,?>)item).getDto();
+            } else {
+                throw new IllegalArgumentException("Expected items of type "+CatalogItemDtoAbstract.class.getSimpleName());
+            }
+            cdto.setManagementContext((ManagementContextInternal) mgmt);
+            
+            try {
+                CatalogUtils.installLibraries(mgmt, item.getLibraries());
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+                if (failOnLoadError) {
+                    Exceptions.propagateAnnotated("Loading bundles for catalog item " + item.getCatalogItemId() + " failed", e);
+                } else {
+                    log.error("Loading bundles for catalog item " + item + " failed: " + e.getMessage(), e);
+                }
+            }
+            catalog.addEntry((CatalogItemDtoAbstract<?,?>)item);
+        }
+        catalog.load(mgmt, null, failOnLoadError);
+    }
+
+    @Override @Deprecated /** @deprecated see super */
     public CatalogItem<?,?> addItem(Class<?> type) {
         //assume forceUpdate for backwards compatibility
         log.debug("Adding manual catalog item to "+mgmt+": "+type);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
index 5a67b1f..e11b64d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
@@ -18,27 +18,39 @@
  */
 package org.apache.brooklyn.core.catalog.internal;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Dictionary;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 import org.apache.brooklyn.api.catalog.BrooklynCatalog;
 import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
 import org.apache.brooklyn.api.mgmt.rebind.RebindExceptionHandler;
 import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind;
 import org.apache.brooklyn.api.typereg.ManagedBundle;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
 import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult;
+import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.objs.BrooklynTypes;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
 import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
+import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -47,19 +59,24 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.FatalRuntimeException;
 import org.apache.brooklyn.util.exceptions.PropagatedRuntimeException;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.time.Duration;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
-import com.google.common.base.Preconditions;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Splitter;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
 @Beta
@@ -82,25 +99,26 @@ public class CatalogInitialization implements ManagementContextInjectable {
      */
 
     private static final Logger log = LoggerFactory.getLogger(CatalogInitialization.class);
-    
+
     private String initialUri;
 
-    private boolean disallowLocal = false;
-    private List<Function<CatalogInitialization, Void>> callbacks = MutableList.of();
-    /** has run an unofficial initialization (i.e. an early load, triggered by an early read of the catalog) */
-    private boolean hasRunUnofficialInitialization = false; 
+    /** has run the initial catalog initialization */
+    private boolean hasRunInitialCatalogInitialization = false; 
+
     /** has run an official initialization, but it is not a permanent one (e.g. during a hot standby mode, or a run failed) */
-    private boolean hasRunTransientOfficialInitialization = false; 
+    private boolean hasRunPersistenceInitialization = false; 
+
     /** has run an official initialization which is permanent (node is master, and the new catalog is now set) */
     private boolean hasRunFinalInitialization = false;
-    /** is running a populate method; used to prevent recursive loops */
-    private boolean isPopulating = false;
-    
+
     private ManagementContextInternal managementContext;
     private boolean isStartingUp = false;
     private boolean failOnStartupErrors = false;
     
-    private Object populatingCatalogMutex = new Object();
+    /** is running a populate method; used to prevent recursive loops */
+    private boolean isPopulatingInitial = false;
+
+    private final Object populatingCatalogMutex = new Object();
     
     public CatalogInitialization() {
         this(null);
@@ -112,7 +130,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
     
     @Override
     public void setManagementContext(ManagementContext managementContext) {
-        Preconditions.checkNotNull(managementContext, "management context");
+        checkNotNull(managementContext, "management context");
         if (this.managementContext!=null && managementContext!=this.managementContext)
             throw new IllegalStateException("Cannot switch management context, from "+this.managementContext+" to "+managementContext);
         this.managementContext = (ManagementContextInternal) managementContext;
@@ -128,88 +146,70 @@ public class CatalogInitialization implements ManagementContextInjectable {
         this.failOnStartupErrors = startupFailOnCatalogErrors;
     }
 
-    public CatalogInitialization addPopulationCallback(Function<CatalogInitialization, Void> callback) {
-        callbacks.add(callback);
-        return this;
-    }
-    
     public ManagementContextInternal getManagementContext() {
-        return Preconditions.checkNotNull(managementContext, "management context has not been injected into "+this);
+        return checkNotNull(managementContext, "management context has not been injected into "+this);
     }
 
     /** Returns true if the canonical initialization has completed, 
      * that is, an initialization which is done when a node is rebinded as master
      * (or an initialization done by the startup routines when not running persistence);
      * see also {@link #hasRunAnyInitialization()}. */
-    public boolean hasRunFinalInitialization() { return hasRunFinalInitialization; }
+    private boolean hasRunFinalInitialization() {
+        return hasRunFinalInitialization;
+    }
     
-    /** Returns true if an official initialization has run,
-     * even if it was a transient run, e.g. so that the launch sequence can tell whether rebind has triggered initialization */
-    public boolean hasRunOfficialInitialization() { return hasRunFinalInitialization || hasRunTransientOfficialInitialization; }
+    private boolean hasRunInitialCatalogInitialization() {
+        return hasRunInitialCatalogInitialization;
+    }
     
-    /** Returns true if the initializer has run at all,
-     * including transient initializations which might be needed before a canonical becoming-master rebind,
-     * for instance because the catalog is being accessed before loading rebind information
-     * (done by {@link #populateUnofficial(BasicBrooklynCatalog)}) */
-    public boolean hasRunAnyInitialization() { return hasRunFinalInitialization || hasRunTransientOfficialInitialization || hasRunUnofficialInitialization; }
+    /**
+     * Returns true if we have added catalog bundles/items from persisted state.
+     */
+    private boolean hasRunPersistenceInitialization() {
+        return hasRunFinalInitialization || hasRunPersistenceInitialization;
+    }
+    
+    /**
+     * Returns true if the initializer has run at all.
+     */
+    @VisibleForTesting
+    @Beta
+    public boolean hasRunAnyInitialization() {
+        return hasRunFinalInitialization || hasRunInitialCatalogInitialization || hasRunPersistenceInitialization;
+    }
 
     /**
-     * This method will almost certainly be changed. It is an interim step, to move the catalog-initialization
-     * decisions and logic out of {@link org.apache.brooklyn.core.mgmt.rebind.RebindIteration} and into 
-     * a single place. We can then make smarter decisions about what to do with the persisted state's catalog.
+     * Populates the initial catalog (i.e. from the initial .bom file).
      * 
-     * @param mode
-     * @param persistedState
-     * @param exceptionHandler
-     * @param rebindLogger
+     * Expected to be called exactly once at startup.
      */
-    @Beta
-    public void populateCatalog(ManagementNodeState mode, PersistedCatalogState persistedState, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
-        // Always installing the bundles from persisted state
-        installBundles(mode, persistedState, exceptionHandler, rebindLogger);
-        
-        // Decide whether to add the persisted catalog items, or to use the "initial items".
-        // Logic copied (unchanged) from RebindIteration.installBundlesAndRebuildCatalog.
-        boolean isEmpty = persistedState.isEmpty();
-        Collection<CatalogItem<?,?>> itemsForResettingCatalog = null;
-        boolean needsInitialItemsLoaded;
-        boolean needsAdditionalItemsLoaded;
-        if (!isEmpty) {
-            rebindLogger.debug("RebindManager clearing local catalog and loading from persisted state");
-            itemsForResettingCatalog = persistedState.getLegacyCatalogItems();
-            needsInitialItemsLoaded = false;
-            // only apply "add" if we haven't yet done so while in MASTER mode
-            needsAdditionalItemsLoaded = !hasRunFinalInitialization();
-        } else {
-            if (hasRunFinalInitialization()) {
-                rebindLogger.debug("RebindManager has already done the final official run, not doing anything (even though persisted state empty)");
-                needsInitialItemsLoaded = false;
-                needsAdditionalItemsLoaded = false;
-            } else {
-                rebindLogger.debug("RebindManager loading initial catalog locally because persisted state empty and the final official run has not yet been performed");
-                needsInitialItemsLoaded = true;
-                needsAdditionalItemsLoaded = true;
-            }
+    public void populateInitialCatalogOnly() {
+        if (log.isDebugEnabled()) {
+            log.debug("Populating only the initial catalog; from "+JavaClassNames.callerNiceClassAndMethod(1));
         }
+        synchronized (populatingCatalogMutex) {
+            if (hasRunInitialCatalogInitialization()) {
+                throw new IllegalStateException("Catalog initialization called to populate only initial, even though it has already run it");
+            }
 
-        // TODO in read-only mode, perhaps do this less frequently than entities etc, maybe only if things change?
-        populateCatalog(mode, needsInitialItemsLoaded, needsAdditionalItemsLoaded, itemsForResettingCatalog);
+            populateInitialCatalogImpl(true);
+            onFinalCatalog();
+        }
     }
-    
-    /** makes or updates the mgmt catalog, based on the settings in this class 
-     * @param nodeState the management node for which this is being read; if master, then we expect this run to be the last one,
-     *   and so subsequent applications should ignore any initialization data (e.g. on a subsequent promotion to master, 
-     *   after a master -> standby -> master cycle)
-     * @param needsInitialItemsLoaded whether the catalog needs the initial items loaded
-     * @param needsAdditionalItemsLoaded whether the catalog needs the additions loaded
-     * @param optionalExplicitItemsForResettingCatalog
-     *   if supplied, the catalog is reset to contain only these items, before calling any other initialization
-     *   for use primarily when rebinding
+
+    /**
+     * Adds the given persisted catalog items.
+     * 
+     * Can be called multiple times, e.g.:
+     * <ul>
+     *   <li>if "hot-standby" then will be called repeatedly, as we rebind to the persisted state
+     *   <li> if being promoted to master then we will be called (and may already have been called for "hot-standby").
+     * </ul>
      */
-    public void populateCatalog(ManagementNodeState nodeState, boolean needsInitialItemsLoaded, boolean needsAdditionsLoaded, Collection<CatalogItem<?, ?>> optionalExplicitItemsForResettingCatalog) {
+    public void populateInitialAndPersistedCatalog(ManagementNodeState mode, PersistedCatalogState persistedState, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
         if (log.isDebugEnabled()) {
-            String message = "Populating catalog for "+nodeState+", needsInitial="+needsInitialItemsLoaded+", needsAdditional="+needsAdditionsLoaded+", explicitItems="+(optionalExplicitItemsForResettingCatalog==null ? "null" : optionalExplicitItemsForResettingCatalog.size())+"; from "+JavaClassNames.callerNiceClassAndMethod(1);
-            if (!ManagementNodeState.isHotProxy(nodeState)) {
+            String message = "Add persisted catalog for "+mode+", persistedBundles="+persistedState.getBundles().size()+", legacyItems="+persistedState.getLegacyCatalogItems().size()+"; from "+JavaClassNames.callerNiceClassAndMethod(1);
+            if (!ManagementNodeState.isHotProxy(mode)) {
                 log.debug(message);
             } else {
                 // in hot modes, make this message trace so we don't get too much output then
@@ -217,75 +217,184 @@ public class CatalogInitialization implements ManagementContextInjectable {
             }
         }
         synchronized (populatingCatalogMutex) {
-            try {
-                if (hasRunFinalInitialization() && (needsInitialItemsLoaded || needsAdditionsLoaded)) {
-                    // if we have already run "final" then we should only ever be used to reset the catalog, 
-                    // not to initialize or add; e.g. we are being given a fixed list on a subsequent master rebind after the initial master rebind 
-                    log.warn("Catalog initialization called to populate initial, even though it has already run the final official initialization");
-                }
-                isPopulating = true;
-                BasicBrooklynCatalog catalog = (BasicBrooklynCatalog) managementContext.getCatalog();
-                if (!catalog.getCatalog().isLoaded()) {
-                    catalog.load();
-                } else {
-                    if (needsInitialItemsLoaded && hasRunAnyInitialization()) {
-                        // an indication that something caused it to load early; not severe, but unusual
-                        if (hasRunTransientOfficialInitialization) {
-                            log.debug("Catalog initialization now populating, but has noted a previous official run which was not final (probalby loaded while in a standby mode, or a previous run failed); overwriting any items installed earlier");
-                        } else {
-                            log.warn("Catalog initialization now populating, but has noted a previous unofficial run (it may have been an early web request); overwriting any items installed earlier");
-                        }
-                        catalog.reset(ImmutableList.<CatalogItem<?,?>>of());
+            if (hasRunFinalInitialization()) {
+                log.warn("Catalog initialization called to add persisted catalog, even though it has already run the final 'master' initialization; mode="+mode+" (perhaps previously demoted from master?)");      
+                hasRunFinalInitialization = false;
+            }
+            if (hasRunPersistenceInitialization()) {
+                // Multiple calls; will need to reset (only way to clear out the previous persisted state's catalog)
+                if (log.isDebugEnabled()) {
+                    String message = "Catalog initialization repeated call to add persisted catalog, resetting catalog (including initial) to start from clean slate; mode="+mode;
+                    if (!ManagementNodeState.isHotProxy(mode)) {
+                        log.debug(message);
+                    } else {
+                        // in hot modes, make this message trace so we don't get too much output then
+                        log.trace(message);
                     }
                 }
+            } else if (hasRunInitialCatalogInitialization()) {
+                throw new IllegalStateException("Catalog initialization already run for initial catalog by mechanism other than populating persisted state; mode="+mode);      
+            }
+            
+            populateInitialCatalogImpl(true);
+            addPersistedCatalogImpl(persistedState, exceptionHandler, rebindLogger);
+            
+            if (mode == ManagementNodeState.MASTER) {
+                // TODO ideally this would remain false until it has *persisted* the changed catalog;
+                // if there is a subsequent startup failure the forced additions will not be persisted,
+                // but nor will they be loaded on a subsequent run.
+                // callers will have to restart a brooklyn, or reach into this class to change this field,
+                // or (recommended) manually adjust the catalog.
+                // TODO also, if a node comes up in standby, the addition might not take effector for a while
+                //
+                // however since these options are mainly for use on the very first brooklyn run, it's not such a big deal; 
+                // once up and running the typical way to add items is via the REST API
+                onFinalCatalog();
+            }
+        }
+    }
 
-                populateCatalogImpl(catalog, needsInitialItemsLoaded, needsAdditionsLoaded, optionalExplicitItemsForResettingCatalog);
-                if (nodeState == ManagementNodeState.MASTER) {
-                    // TODO ideally this would remain false until it has *persisted* the changed catalog;
-                    // if there is a subsequent startup failure the forced additions will not be persisted,
-                    // but nor will they be loaded on a subsequent run.
-                    // callers will have to restart a brooklyn, or reach into this class to change this field,
-                    // or (recommended) manually adjust the catalog.
-                    // TODO also, if a node comes up in standby, the addition might not take effector for a while
-                    //
-                    // however since these options are mainly for use on the very first brooklyn run, it's not such a big deal; 
-                    // once up and running the typical way to add items is via the REST API
-                    hasRunFinalInitialization = true;
-                }
-            } catch (Throwable e) {
-                log.warn("Error populating catalog (rethrowing): "+e, e);
-                throw Exceptions.propagate(e);
-            } finally {
-                if (!hasRunFinalInitialization) {
-                    hasRunTransientOfficialInitialization = true;
-                }
-                isPopulating = false;
+    /**
+     * Populates the initial catalog, but not via an official code-path.
+     * 
+     * Expected to be called only during tests, where the test has not gone through the same 
+     * management-context lifecycle as is done in BasicLauncher.
+     * 
+     * Subsequent calls will fail to things like {@link #populateInitialCatalog()} or 
+     * {@link #populateInitialAndPersistedCatalog(ManagementNodeState, PersistedCatalogState, RebindExceptionHandler, RebindLogger)}.
+     */
+    @VisibleForTesting
+    @Beta
+    public void unofficialPopulateInitialCatalog() {
+        if (log.isDebugEnabled()) {
+            log.debug("Unofficially populate initial catalog; should be used only by tests! Called from "+JavaClassNames.callerNiceClassAndMethod(1));
+        }
+        synchronized (populatingCatalogMutex) {
+            if (hasRunInitialCatalogInitialization()) {
+                return;
             }
+
+            populateInitialCatalogImpl(true);
         }
     }
 
-    private void populateCatalogImpl(BasicBrooklynCatalog catalog, boolean needsInitialItemsLoaded, boolean needsAdditionsLoaded, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
-        if (optionalItemsForResettingCatalog!=null) {
-            catalog.reset(optionalItemsForResettingCatalog);
+    public void handleException(Throwable throwable, Object details) {
+        if (throwable instanceof InterruptedException)
+            throw new RuntimeInterruptedException((InterruptedException) throwable);
+        if (throwable instanceof RuntimeInterruptedException)
+            throw (RuntimeInterruptedException) throwable;
+
+        if (details instanceof CatalogItem) {
+            if (((CatalogItem<?,?>)details).getCatalogItemId() != null) {
+                details = ((CatalogItem<?,?>)details).getCatalogItemId();
+            }
         }
+        PropagatedRuntimeException wrap = new PropagatedRuntimeException("Error loading catalog item "+details, throwable);
+        log.warn(Exceptions.collapseText(wrap));
+        log.debug("Trace for: "+wrap, wrap);
+
+        getManagementContext().errors().add(wrap);
         
-        if (needsInitialItemsLoaded) {
-            populateInitial(catalog);
+        if (isStartingUp && failOnStartupErrors) {
+            throw new FatalRuntimeException("Unable to load catalog item "+details, wrap);
+        }
+    }
+    
+    private void confirmCatalog() {
+        // Force load of catalog (so web console is up to date)
+        Stopwatch time = Stopwatch.createStarted();
+        Iterable<RegisteredType> all = getManagementContext().getTypeRegistry().getAll();
+        int errors = 0;
+        for (RegisteredType rt: all) {
+            if (RegisteredTypes.isTemplate(rt)) {
+                // skip validation of templates, they might contain instructions,
+                // and additionally they might contain multiple items in which case
+                // the validation below won't work anyway (you need to go via a deployment plan)
+            } else {
+                if (rt.getKind()==RegisteredTypeKind.UNRESOLVED) {
+                    errors++;
+                    handleException(new UserFacingException("Unresolved type in catalog"), rt);
+                }
+            }
         }
 
-        if (needsAdditionsLoaded) {
-            populateViaCallbacks(catalog);
+        // and force resolution of legacy items
+        BrooklynCatalog catalog = getManagementContext().getCatalog();
+        Iterable<CatalogItem<Object, Object>> items = catalog.getCatalogItemsLegacy();
+        for (CatalogItem<Object, Object> item: items) {
+            try {
+                if (item.getCatalogItemType()==CatalogItemType.TEMPLATE) {
+                    // skip validation of templates, they might contain instructions,
+                    // and additionally they might contain multiple items in which case
+                    // the validation below won't work anyway (you need to go via a deployment plan)
+                } else {
+                    AbstractBrooklynObjectSpec<?, ?> spec = catalog.peekSpec(item);
+                    if (spec instanceof EntitySpec) {
+                        BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType());
+                    }
+                    log.debug("Catalog loaded spec "+spec+" for item "+item);
+                }
+            } catch (Throwable throwable) {
+                handleException(throwable, item);
+            }
         }
+        
+        log.debug("Catalog (size "+Iterables.size(all)+", of which "+Iterables.size(items)+" legacy) confirmed in "+Duration.of(time)+(errors>0 ? ", errors found ("+errors+")" : ""));
+        // nothing else added here
     }
 
-    protected void populateInitial(BasicBrooklynCatalog catalog) {
-        if (disallowLocal) {
-            if (!hasRunFinalInitialization()) {
-                log.debug("CLI initial catalog not being read when local catalog load mode is disallowed.");
-            }
+    private void populateInitialCatalogImpl(boolean reset) {
+        assert Thread.holdsLock(populatingCatalogMutex);
+        
+        if (isPopulatingInitial) {
+            // Avoid recursively loops, where getCatalog() calls unofficialPopulateInitialCatalog(), but populateInitialCatalogImpl() also calls getCatalog()
             return;
         }
+        
+        isPopulatingInitial = true;
+        try {
+            BasicBrooklynCatalog catalog = (BasicBrooklynCatalog) managementContext.getCatalog();
+            if (!catalog.getCatalog().isLoaded()) {
+                catalog.load();
+            } else {
+                if (reset) {
+                    catalog.reset(ImmutableList.<CatalogItem<?,?>>of());
+                }
+            }
+
+            populateViaInitialBomImpl(catalog);
 
+        } catch (Throwable e) {
+            log.warn("Error populating catalog (rethrowing): "+e, e);
+            throw Exceptions.propagate(e);
+        } finally {
+            isPopulatingInitial = false;
+            hasRunInitialCatalogInitialization = true;
+        }
+    }
+
+    private void addPersistedCatalogImpl(PersistedCatalogState persistedState, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
+        assert Thread.holdsLock(populatingCatalogMutex);
+
+        try {
+            // Always installing the bundles from persisted state
+            installBundles(persistedState.getBundles(), exceptionHandler, rebindLogger);
+            
+            BrooklynCatalog catalog = managementContext.getCatalog();
+            catalog.addCatalogLegacyItemsOnRebind(persistedState.getLegacyCatalogItems());
+        } finally {
+            hasRunPersistenceInitialization = true;
+        }
+    }
+    
+    private void onFinalCatalog() {
+        assert Thread.holdsLock(populatingCatalogMutex);
+        
+        hasRunFinalInitialization = true;
+        confirmCatalog();
+    }
+
+    private void populateViaInitialBomImpl(BasicBrooklynCatalog catalog) {
 //        B1) look for --catalog-initial, if so read it, then go to C1
 //        B2) look for BrooklynServerConfig.BROOKLYN_CATALOG_URL, if so, read it, supporting YAML, then go to C1
 //        B3) look for ~/.brooklyn/catalog.bom, if exists, read it then go to C1
@@ -341,70 +450,18 @@ public class CatalogInitialization implements ManagementContextInjectable {
         }
     }
 
-    protected void populateViaCallbacks(BasicBrooklynCatalog catalog) {
-        for (Function<CatalogInitialization, Void> callback: callbacks)
-            callback.apply(this);
-    }
-
-    /** Creates the catalog based on parameters set here, if not yet loaded,
-     * but ignoring persisted state and warning if persistence is on and we are starting up
-     * (because the official persistence is preferred and the catalog will be subsequently replaced);
-     * for use when the catalog is accessed before persistence is completed. 
-     * <p>
-     * This method is primarily used during testing, which in many cases does not enforce the full startup order
-     * and which wants a local catalog in any case. It may also be invoked if a client requests the catalog
-     * while the server is starting up. */
-    public void populateUnofficial(BasicBrooklynCatalog catalog) {
-        synchronized (populatingCatalogMutex) {
-            // check isPopulating in case this method gets called from inside another populate call
-            if (hasRunAnyInitialization() || isPopulating) return;
-            log.debug("Populating catalog unofficially ("+catalog+")");
-            isPopulating = true;
-            try {
-                if (isStartingUp) {
-                    log.warn("Catalog access requested when not yet initialized; populating best effort rather than through recommended pathway. Catalog data may be replaced subsequently.");
-                }
-                populateCatalogImpl(catalog, true, true, null);
-            } finally {
-                hasRunUnofficialInitialization = true;
-                isPopulating = false;
-            }
-        }
-    }
-
-    public void handleException(Throwable throwable, Object details) {
-        if (throwable instanceof InterruptedException)
-            throw new RuntimeInterruptedException((InterruptedException) throwable);
-        if (throwable instanceof RuntimeInterruptedException)
-            throw (RuntimeInterruptedException) throwable;
-
-        if (details instanceof CatalogItem) {
-            if (((CatalogItem<?,?>)details).getCatalogItemId() != null) {
-                details = ((CatalogItem<?,?>)details).getCatalogItemId();
-            }
-        }
-        PropagatedRuntimeException wrap = new PropagatedRuntimeException("Error loading catalog item "+details, throwable);
-        log.warn(Exceptions.collapseText(wrap));
-        log.debug("Trace for: "+wrap, wrap);
-
-        getManagementContext().errors().add(wrap);
-        
-        if (isStartingUp && failOnStartupErrors) {
-            throw new FatalRuntimeException("Unable to load catalog item "+details, wrap);
-        }
-    }
-    
-    private void installBundles(ManagementNodeState mode, PersistedCatalogState persistedState, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
+    private void installBundles(Map<VersionedName, InstallableManagedBundle> bundles, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
         List<OsgiBundleInstallationResult> installs = MutableList.of();
 
         // Install the bundles
-        for (String bundleId : persistedState.getBundleIds()) {
+        for (Map.Entry<VersionedName, InstallableManagedBundle> entry : bundles.entrySet()) {
+            VersionedName bundleId = entry.getKey();
+            InstallableManagedBundle installableBundle = entry.getValue();
             rebindLogger.debug("RebindManager installing bundle {}", bundleId);
-            InstallableManagedBundle installableBundle = persistedState.getInstallableManagedBundle(bundleId);
             try (InputStream in = installableBundle.getInputStream()) {
                 installs.add(installBundle(installableBundle.getManagedBundle(), in));
             } catch (Exception e) {
-                exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, bundleId, installableBundle.getManagedBundle().getSymbolicName(), e);
+                exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, bundleId.toString(), installableBundle.getManagedBundle().getSymbolicName(), e);
             }
         }
         
@@ -461,11 +518,11 @@ public class CatalogInitialization implements ManagementContextInjectable {
     /** install the bundles into brooklyn and osgi, but do not start nor validate;
      * caller (rebind) will do that manually, doing each step across all bundles before proceeding 
      * to prevent reference errors */
-    public OsgiBundleInstallationResult installBundle(ManagedBundle bundle, InputStream zipInput) {
+    private OsgiBundleInstallationResult installBundle(ManagedBundle bundle, InputStream zipInput) {
         return getManagementContext().getOsgiManager().get().installDeferredStart(bundle, zipInput, false).get();
     }
     
-    public void startBundle(OsgiBundleInstallationResult br) throws BundleException {
+    private void startBundle(OsgiBundleInstallationResult br) throws BundleException {
         if (br.getDeferredStart()!=null) {
             br.getDeferredStart().run();
         }
@@ -481,26 +538,27 @@ public class CatalogInitialization implements ManagementContextInjectable {
         public InputStream getInputStream() throws IOException;
     }
     
-    public interface PersistedCatalogState {
-
-        /**
-         * Whether the persisted state is entirely empty.
-         */
-        public boolean isEmpty();
-
-        /**
-         * The persisted catalog items (from the {@code /catalog} sub-directory of the persisted state).
-         */
-        public Collection<CatalogItem<?,?>> getLegacyCatalogItems();
+    public static class PersistedCatalogState {
+        private final Map<VersionedName, InstallableManagedBundle> bundles;
+        private final Collection<CatalogItem<?, ?>> legacyCatalogItems;
         
+        public PersistedCatalogState(Map<VersionedName, InstallableManagedBundle> bundles, Collection<CatalogItem<?, ?>> legacyCatalogItems) {
+            this.bundles = checkNotNull(bundles, "bundles");
+            this.legacyCatalogItems = checkNotNull(legacyCatalogItems, "legacyCatalogItems");
+        }
+
         /**
          * The persisted bundles (from the {@code /bundles} sub-directory of the persisted state).
          */
-        public Set<String> getBundleIds();
+        public Map<VersionedName, InstallableManagedBundle> getBundles() {
+            return bundles;
+        }
         
         /**
-         * Loads the details of a particular bundle, so it can be installed.
+         * The persisted catalog items (from the {@code /catalog} sub-directory of the persisted state).
          */
-        public InstallableManagedBundle getInstallableManagedBundle(String id);
+        public Collection<CatalogItem<?,?>> getLegacyCatalogItems() {
+            return legacyCatalogItems;
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
index 27645bc..3cdd944 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
@@ -421,12 +421,6 @@ public abstract class AbstractManagementContext implements ManagementContextInte
 
     @Override
     public BrooklynCatalog getCatalog() {
-        if (!getCatalogInitialization().hasRunAnyInitialization()) {
-            // catalog init is needed; normally this will be done from start sequence,
-            // but if accessed early -- and in tests -- we will load it here
-            getCatalogInitialization().setManagementContext(this);
-            getCatalogInitialization().populateUnofficial(catalog);
-        }
         return catalog;
     }
     
@@ -505,7 +499,7 @@ public abstract class AbstractManagementContext implements ManagementContextInte
         return uri;
     }
     
-    private Object catalogInitMutex = new Object();
+    private final Object catalogInitMutex = new Object();
     @Override
     public CatalogInitialization getCatalogInitialization() {
         synchronized (catalogInitMutex) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
index 4836d70..9076845 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
@@ -162,10 +162,6 @@ public class LocalManagementContext extends AbstractManagementContext {
         this(builder, null);
     }
     
-    public LocalManagementContext(BrooklynProperties brooklynProperties, Map<String, Object> brooklynAdditionalProperties) {
-        this(Builder.fromProperties(brooklynProperties), brooklynAdditionalProperties);
-    }
-    
     public LocalManagementContext(Builder builder, Map<String, Object> brooklynAdditionalProperties) {
         super(builder.build());
         

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
index 326ebc0..20c20e3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
@@ -109,6 +109,7 @@ import org.apache.brooklyn.util.core.flags.FlagUtils;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.Reflections;
+import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.brooklyn.util.time.Time;
@@ -350,42 +351,14 @@ public abstract class RebindIteration {
             }
         }
         
-        class PersistedCatalogStateImpl implements CatalogInitialization.PersistedCatalogState {
-            private final boolean isEmpty;
-            private final Collection<CatalogItem<?, ?>> legacyCatalogItems;
-            private final Map<String, ? extends InstallableManagedBundle> bundles;
-            
-            PersistedCatalogStateImpl(boolean isEmpty, Collection<CatalogItem<?, ?>> legacyCatalogItems, Map<String, ? extends InstallableManagedBundle> bundles) {
-                this.isEmpty = isEmpty;
-                this.legacyCatalogItems = legacyCatalogItems;
-                this.bundles = bundles;
-            }
-            
-            @Override public boolean isEmpty() {
-                return isEmpty;
-            }
-
-            @Override public Collection<CatalogItem<?, ?>> getLegacyCatalogItems() {
-                return legacyCatalogItems;
-            }
-
-            @Override public Set<String> getBundleIds() {
-                return bundles.keySet();
-            }
-
-            @Override public InstallableManagedBundle getInstallableManagedBundle(String id) {
-                return bundles.get(id);
-            }
-        }
-        
-        Map<String, InstallableManagedBundleImpl> bundles = new LinkedHashMap<>();
+        Map<VersionedName, InstallableManagedBundle> bundles = new LinkedHashMap<>();
         Collection<CatalogItem<?,?>> legacyCatalogItems = new ArrayList<>();
         
         // Find the bundles
         if (rebindManager.persistBundlesEnabled) {
             for (ManagedBundleMemento bundleMemento : mementoManifest.getBundles().values()) {
                 ManagedBundle managedBundle = instantiator.newManagedBundle(bundleMemento);
-                bundles.put(bundleMemento.getId(), new InstallableManagedBundleImpl(bundleMemento, managedBundle));
+                bundles.put(managedBundle.getVersionedName(), new InstallableManagedBundleImpl(bundleMemento, managedBundle));
             }
         } else {
             logRebindingDebug("Not rebinding bundles; feature disabled: {}", mementoManifest.getBundleIds());
@@ -433,10 +406,10 @@ public abstract class RebindIteration {
 
 
         // Delegates to CatalogInitialization; see notes there.
-        CatalogInitialization.PersistedCatalogState persistedCatalogState = new PersistedCatalogStateImpl(isEmpty, legacyCatalogItems, bundles);
+        CatalogInitialization.PersistedCatalogState persistedCatalogState = new CatalogInitialization.PersistedCatalogState(bundles, legacyCatalogItems);
         
         CatalogInitialization catInit = managementContext.getCatalogInitialization();
-        catInit.populateCatalog(mode, persistedCatalogState, exceptionHandler, rebindLogger);
+        catInit.populateInitialAndPersistedCatalog(mode, persistedCatalogState, exceptionHandler, rebindLogger);
     }
 
     protected void instantiateLocationsAndEntities() {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerTestFixture.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerTestFixture.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerTestFixture.java
index c4d8a3d..ff02c1c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerTestFixture.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerTestFixture.java
@@ -116,7 +116,8 @@ public abstract class HighAvailabilityManagerTestFixture {
     protected ManagementContextInternal newLocalManagementContext() {
         BrooklynProperties brooklynProperties = BrooklynProperties.Factory.newEmpty();
         brooklynProperties.put(BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS, ImmutableList.of(stateListener));
-        return LocalManagementContextForTests.newInstance(brooklynProperties);
+        ManagementContextInternal result = LocalManagementContextForTests.newInstance(brooklynProperties);
+        return result;
     }
 
     protected abstract PersistenceObjectStore newPersistenceObjectStore();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
index 6809bac..b53d92c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
@@ -116,9 +116,9 @@ public class RebindCatalogEntityTest extends RebindTestFixture<StartableApplicat
     
     @Test(invocationCount=100, groups="Integration")
     public void testRestoresAppFromCatalogClassloaderManyTimes() throws Exception {
-	    // Need to fix package name and rebuild brooklyn-AppInCatalog.jar
-    	//  or better add it as a new test-bundles subproject
-    	testRestoresAppFromCatalogClassloader();
+        // Need to fix package name and rebuild brooklyn-AppInCatalog.jar
+        //  or better add it as a new test-bundles subproject
+        testRestoresAppFromCatalogClassloader();
     }
     
     // TODO Not using RebindTestUtils.rebind(mementoDir, getClass().getClassLoader());
@@ -150,4 +150,4 @@ public class RebindCatalogEntityTest extends RebindTestFixture<StartableApplicat
         return (StartableApplication) newApps.get(0);
     }
 
-}
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
index 8c2e7b7..422ba4c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
@@ -61,6 +61,7 @@ import org.apache.brooklyn.core.entity.trait.Resizable;
 import org.apache.brooklyn.core.entity.trait.Startable;
 import org.apache.brooklyn.core.location.LocationConfigTest.MyLocation;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.sensor.BasicAttributeSensor;
 import org.apache.brooklyn.core.sensor.BasicSensorEvent;
 import org.apache.brooklyn.core.sensor.DependentConfiguration;
@@ -648,6 +649,7 @@ public class RebindEntityTest extends RebindTestFixtureWithApp {
 
         RebindTestUtils.waitForPersisted(origManagementContext);
         newManagementContext = RebindTestUtils.newPersistingManagementContextUnstarted(mementoDir, classLoader);
+
         List<Application> newApps = newManagementContext.getRebindManager().rebind(classLoader, null, ManagementNodeState.MASTER);
         newManagementContext.getRebindManager().startPersistence();
         

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricCatalogItemTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricCatalogItemTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricCatalogItemTest.java
new file mode 100644
index 0000000..0d1b004
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindHistoricCatalogItemTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.rebind;
+
+import static org.testng.Assert.assertEquals;
+
+import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind;
+import org.apache.brooklyn.api.typereg.RegisteredType;
+import org.testng.annotations.Test;
+
+public class RebindHistoricCatalogItemTest extends AbstractRebindHistoricTest {
+    
+    /**
+     * Purpose of test is for rebinding to historic state created before we auto-wrapped .bom 
+     * catalog items in bundles.
+     * 
+     * The persisted state contains a catalog item (which is not wrapped as a bundle).
+     * It was created in brooklyn 0.11.0 via {@code br catalog add app.bom}, using:
+     * <pre>
+     *   brooklyn.catalog:
+     *     id: catalog-myapp
+     *     version: 1.0.0
+     *     itemType: entity
+     *     item:
+     *       type: org.apache.brooklyn.entity.stock.BasicApplication
+     * </pre>
+     */
+    @Test
+    public void testLegacyCatalogItem() throws Exception {
+        addMemento(BrooklynObjectType.CATALOG_ITEM, "catalog", "myapp_1.0.0");
+        rebind();
+        
+        RegisteredType type = mgmt().getTypeRegistry().get("myapp", "1.0.0");
+        assertEquals(type.getKind(), RegisteredTypeKind.SPEC);
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestFixture.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestFixture.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestFixture.java
index c7d97a9..bad753d 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestFixture.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestFixture.java
@@ -18,16 +18,11 @@
  */
 package org.apache.brooklyn.core.mgmt.rebind;
 
-import static org.testng.Assert.assertEquals;
-
 import java.io.File;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.Callable;
 
-import org.apache.brooklyn.api.catalog.BrooklynCatalog;
-import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
@@ -35,9 +30,7 @@ import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
 import org.apache.brooklyn.api.mgmt.rebind.RebindExceptionHandler;
 import org.apache.brooklyn.api.mgmt.rebind.RebindManager;
 import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoManifest;
-import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
 import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.core.entity.EntityFunctions;
 import org.apache.brooklyn.core.entity.EntityPredicates;
 import org.apache.brooklyn.core.entity.StartableApplication;
 import org.apache.brooklyn.core.entity.trait.Startable;
@@ -56,10 +49,6 @@ import org.slf4j.LoggerFactory;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 
-import com.google.common.collect.FluentIterable;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
-
 public abstract class RebindTestFixture<T extends StartableApplication> {
 
     private static final Logger LOG = LoggerFactory.getLogger(RebindTestFixture.class);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java
index ba0ece2..911db07 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java
@@ -284,10 +284,12 @@ public class RebindTestUtils {
 
         public LocalManagementContext buildStarted() {
             LocalManagementContext unstarted = buildUnstarted();
+            
             // Follows BasicLauncher logic for initialising persistence.
             // TODO It should really be encapsulated in a common entry point
             if (persistMode == PersistMode.DISABLED) {
                 unstarted.generateManagementPlaneId();
+                unstarted.getCatalogInitialization().populateInitialCatalogOnly();
             } else if (haMode == HighAvailabilityMode.DISABLED) {
                 unstarted.getRebindManager().rebind(classLoader, null, ManagementNodeState.MASTER);
                 unstarted.getRebindManager().startPersistence();
@@ -478,6 +480,7 @@ public class RebindTestUtils {
             // Would that affect any tests?
             newManagementContext = new LocalManagementContextForTests(BrooklynProperties.Factory.newDefault());
         }
+        
         if (!hasPersister) {
             if (objectStore == null) {
                 objectStore = new FileBasedObjectStore(checkNotNull(mementoDir, "mementoDir and objectStore must not both be null"));

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/java/org/apache/brooklyn/core/test/entity/LocalManagementContextForTests.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/test/entity/LocalManagementContextForTests.java b/core/src/test/java/org/apache/brooklyn/core/test/entity/LocalManagementContextForTests.java
index 9996294..4634bc5 100644
--- a/core/src/test/java/org/apache/brooklyn/core/test/entity/LocalManagementContextForTests.java
+++ b/core/src/test/java/org/apache/brooklyn/core/test/entity/LocalManagementContextForTests.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.core.test.entity;
 
 import java.util.Map;
 
+import org.apache.brooklyn.api.catalog.BrooklynCatalog;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.internal.BrooklynProperties;
 import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
@@ -209,4 +210,15 @@ public class LocalManagementContextForTests extends LocalManagementContext {
         return builder(true).enableOsgiReusable().build();
     }
 
+    @Override
+    public BrooklynCatalog getCatalog() {
+        if (!getCatalogInitialization().hasRunAnyInitialization()) {
+            // Before catalog init, the catalog will be empty.
+            // Normally the BasicLauncher (either the classic BrooklynLauncher or the OsgiLauncher)
+            // will have ensured the catalog initialization is called. But for tests, the lifecycle
+            // for the management context is unfortunately different.
+            getCatalogInitialization().unofficialPopulateInitialCatalog();
+        }
+        return catalog;
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/catalog-myapp_1.0.0
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/catalog-myapp_1.0.0 b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/catalog-myapp_1.0.0
new file mode 100644
index 0000000..e948184
--- /dev/null
+++ b/core/src/test/resources/org/apache/brooklyn/core/mgmt/rebind/catalog-myapp_1.0.0
@@ -0,0 +1,36 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<catalogItem>
+  <brooklynVersion>0.12.0-20170901.1331</brooklynVersion>
+  <type>org.apache.brooklyn.core:org.apache.brooklyn.core.catalog.internal.CatalogEntityItemDto</type>
+  <id>myapp:1.0.0</id>
+  <catalogItemId>myapp:1.0.0</catalogItemId>
+  <searchPath class="ImmutableList"/>
+  <registeredTypeName>myapp</registeredTypeName>
+  <version>1.0.0</version>
+  <planYaml>services:
+- type: org.apache.brooklyn.entity.stock.BasicApplication</planYaml>
+  <libraries class="ImmutableList" reference="../searchPath"/>
+  <catalogItemType>ENTITY</catalogItemType>
+  <catalogItemJavaType>org.apache.brooklyn.api:org.apache.brooklyn.api.entity.Entity</catalogItemJavaType>
+  <specType>org.apache.brooklyn.api:org.apache.brooklyn.api.entity.EntitySpec</specType>
+  <deprecated>false</deprecated>
+  <disabled>false</disabled>
+</catalogItem>

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/launcher-common/src/main/java/org/apache/brooklyn/launcher/common/BasicLauncher.java
----------------------------------------------------------------------
diff --git a/launcher-common/src/main/java/org/apache/brooklyn/launcher/common/BasicLauncher.java b/launcher-common/src/main/java/org/apache/brooklyn/launcher/common/BasicLauncher.java
index ec82d45..19b6e20 100644
--- a/launcher-common/src/main/java/org/apache/brooklyn/launcher/common/BasicLauncher.java
+++ b/launcher-common/src/main/java/org/apache/brooklyn/launcher/common/BasicLauncher.java
@@ -253,12 +253,12 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
     }
 
     public T persistMode(PersistMode persistMode) {
-        this.persistMode = persistMode;
+        this.persistMode = checkNotNull(persistMode, "persistMode");
         return self();
     }
 
     public T highAvailabilityMode(HighAvailabilityMode highAvailabilityMode) {
-        this.highAvailabilityMode = highAvailabilityMode;
+        this.highAvailabilityMode = checkNotNull(highAvailabilityMode, "highAvailabilityMode");
         return self();
     }
 
@@ -389,6 +389,8 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
         if (started) throw new IllegalStateException("Cannot start() or launch() multiple times");
         started = true;
 
+        checkConfiguration();
+        
         initManagementContext();
 
         CatalogInitialization catInit = ((ManagementContextInternal)managementContext).getCatalogInitialization();
@@ -407,6 +409,23 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
         return self();
     }
 
+    private void checkConfiguration() {
+        if (highAvailabilityMode != HighAvailabilityMode.DISABLED) {
+            if (persistMode == PersistMode.DISABLED) {
+                if (highAvailabilityMode == HighAvailabilityMode.AUTO) {
+                    highAvailabilityMode = HighAvailabilityMode.DISABLED;
+                } else {
+                    throw new FatalConfigurationRuntimeException("Cannot specify highAvailability when persistence is disabled");
+                }
+            } else if (persistMode == PersistMode.CLEAN && 
+                    (highAvailabilityMode == HighAvailabilityMode.STANDBY 
+                            || highAvailabilityMode == HighAvailabilityMode.HOT_STANDBY
+                            || highAvailabilityMode == HighAvailabilityMode.HOT_BACKUP)) {
+                throw new FatalConfigurationRuntimeException("Cannot specify highAvailability "+highAvailabilityMode+" when persistence is CLEAN");
+            }
+        }
+    }
+
     /**
      * Starts the web server (with web console) and Brooklyn applications, as per the specifications configured. 
      * @return An object containing details of the web server and the management context.
@@ -415,8 +434,12 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
         if (startedPartTwo) throw new IllegalStateException("Cannot start() or launch() multiple times");
         startedPartTwo = true;
         
-        handlePersistence();
-        populateCatalog(catalogInitialization);
+        if (persistMode != PersistMode.DISABLED) {
+            handlePersistence();
+        } else {
+            ((LocalManagementContext)managementContext).generateManagementPlaneId();
+            populateInitialCatalogNoPersistence(catalogInitialization);
+        }
         markCatalogStarted(catalogInitialization);
         addLocations();
         markStartupComplete();
@@ -471,30 +494,21 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
     protected void startingUp() {
     }
 
-    private void markCatalogStarted(CatalogInitialization catInit) {
-        catInit.setStartingUp(false);
-    }
-
-    protected void populateCatalog(CatalogInitialization catInit) {
+    protected void populateInitialCatalogNoPersistence(CatalogInitialization catInit) {
+        assert persistMode == PersistMode.DISABLED : "persistMode="+persistMode;
+        
         try {
-            // run cat init now if it hasn't yet been run; 
-            // will also run if there was an ignored error in catalog above, allowing it to fail startup here if requested
-            if (catInit!=null && !catInit.hasRunOfficialInitialization()) {
-                if (persistMode==PersistMode.DISABLED) {
-                    LOG.debug("Loading catalog as part of launch sequence (it was not loaded as part of any rebind sequence)");
-                    catInit.populateCatalog(ManagementNodeState.MASTER, true, true, null);
-                } else {
-                    // should have loaded during rebind
-                    ManagementNodeState state = managementContext.getHighAvailabilityManager().getNodeState();
-                    LOG.warn("Loading catalog for "+state+" as part of launch sequence (it was not loaded as part of the rebind sequence)");
-                    catInit.populateCatalog(state, true, true, null);
-                }
-            }
+            LOG.debug("Initializing initial catalog as part of launch sequence (prior to any persistence rebind)");
+            catInit.populateInitialCatalogOnly();
         } catch (Exception e) {
             handleSubsystemStartupError(ignoreCatalogErrors, "initial catalog", e);
         }
     }
 
+    private void markCatalogStarted(CatalogInitialization catInit) {
+        catInit.setStartingUp(false);
+    }
+
     protected void handlePersistence() {
         try {
             initPersistence();
@@ -551,45 +565,38 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
     }
 
     protected void initPersistence() {
-        // Prepare the rebind directory, and initialise the RebindManager as required
+        assert persistMode != PersistMode.DISABLED : "persistMode="+persistMode+"; haMode="+highAvailabilityMode;
+
+        // Prepare the rebind directory, and initialise the RebindManager
         final PersistenceObjectStore objectStore;
-        if (persistMode == PersistMode.DISABLED) {
-            LOG.info("Persistence disabled");
-            objectStore = null;
-            ((LocalManagementContext)managementContext).generateManagementPlaneId();
-        } else {
-            try {
-                if (persistenceLocation == null) {
-                    persistenceLocation = brooklynProperties.getConfig(BrooklynServerConfig.PERSISTENCE_LOCATION_SPEC);
-                }
-                persistenceDir = BrooklynServerPaths.newMainPersistencePathResolver(brooklynProperties).location(persistenceLocation).dir(persistenceDir).resolve();
-                objectStore = BrooklynPersistenceUtils.newPersistenceObjectStore(managementContext, persistenceLocation, persistenceDir, 
-                    persistMode, highAvailabilityMode);
-                    
-                RebindManager rebindManager = managementContext.getRebindManager();
-                
-                BrooklynMementoPersisterToObjectStore persister = new BrooklynMementoPersisterToObjectStore(
-                    objectStore, managementContext);
-                PersistenceExceptionHandler persistenceExceptionHandler = PersistenceExceptionHandlerImpl.builder().build();
-                ((RebindManagerImpl) rebindManager).setPeriodicPersistPeriod(persistPeriod);
-                rebindManager.setPersister(persister, persistenceExceptionHandler);
-            } catch (FatalConfigurationRuntimeException e) {
-                throw e;
-            } catch (Exception e) {
-                Exceptions.propagateIfFatal(e);
-                LOG.debug("Error initializing persistence subsystem (rethrowing): "+e, e);
-                throw new FatalRuntimeException("Error initializing persistence subsystem: "+
-                    Exceptions.collapseText(e), e);
+        try {
+            if (persistenceLocation == null) {
+                persistenceLocation = brooklynProperties.getConfig(BrooklynServerConfig.PERSISTENCE_LOCATION_SPEC);
             }
+            persistenceDir = BrooklynServerPaths.newMainPersistencePathResolver(brooklynProperties).location(persistenceLocation).dir(persistenceDir).resolve();
+            objectStore = BrooklynPersistenceUtils.newPersistenceObjectStore(managementContext, persistenceLocation, persistenceDir, 
+                persistMode, highAvailabilityMode);
+                
+            RebindManager rebindManager = managementContext.getRebindManager();
+            
+            BrooklynMementoPersisterToObjectStore persister = new BrooklynMementoPersisterToObjectStore(
+                objectStore, managementContext);
+            PersistenceExceptionHandler persistenceExceptionHandler = PersistenceExceptionHandlerImpl.builder().build();
+            ((RebindManagerImpl) rebindManager).setPeriodicPersistPeriod(persistPeriod);
+            rebindManager.setPersister(persister, persistenceExceptionHandler);
+        } catch (FatalConfigurationRuntimeException e) {
+            throw e;
+        } catch (Exception e) {
+            Exceptions.propagateIfFatal(e);
+            LOG.debug("Error initializing persistence subsystem (rethrowing): "+e, e);
+            throw new FatalRuntimeException("Error initializing persistence subsystem: "+
+                Exceptions.collapseText(e), e);
         }
         
         // Initialise the HA manager as required
         if (highAvailabilityMode == HighAvailabilityMode.DISABLED) {
             LOG.info("High availability disabled");
         } else {
-            if (objectStore==null)
-                throw new FatalConfigurationRuntimeException("Cannot run in HA mode when no persistence configured.");
-
             HighAvailabilityManager haManager = managementContext.getHighAvailabilityManager();
             ManagementPlaneSyncRecordPersister persister =
                 new ManagementPlaneSyncRecordPersisterToObjectStore(managementContext,
@@ -602,20 +609,20 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
     }
     
     protected void startPersistence() {
+        assert persistMode != PersistMode.DISABLED : "persistMode="+persistMode+"; haMode="+highAvailabilityMode;
+        
         // Now start the HA Manager and the Rebind manager, as required
         if (highAvailabilityMode == HighAvailabilityMode.DISABLED) {
             HighAvailabilityManager haManager = managementContext.getHighAvailabilityManager();
             haManager.disabled();
 
-            if (persistMode != PersistMode.DISABLED) {
-                startPersistenceWithoutHA();
-            }
+            startPersistenceWithoutHA();
             
         } else {
             // Let the HA manager decide when objectstore.prepare and rebindmgr.rebind need to be called 
             // (based on whether other nodes in plane are already running).
             
-            HighAvailabilityMode startMode=null;
+            HighAvailabilityMode startMode;
             switch (highAvailabilityMode) {
                 case AUTO:
                 case MASTER:
@@ -626,9 +633,9 @@ public class BasicLauncher<T extends BasicLauncher<T>> {
                     break;
                 case DISABLED:
                     throw new IllegalStateException("Unexpected code-branch for high availability mode "+highAvailabilityMode);
+                default:
+                    throw new IllegalStateException("Unexpected high availability mode "+highAvailabilityMode);
             }
-            if (startMode==null)
-                throw new IllegalStateException("Unexpected high availability mode "+highAvailabilityMode);
             
             LOG.debug("Management node (with HA) starting");
             HighAvailabilityManager haManager = managementContext.getHighAvailabilityManager();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
index 1bf94cd..bafb641 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogTest.java
@@ -20,9 +20,20 @@ package org.apache.brooklyn.launcher;
 
 import java.io.File;
 import java.util.List;
+import java.util.Set;
 
 import javax.annotation.Nullable;
 
+import org.apache.brooklyn.api.catalog.BrooklynCatalog;
+import org.apache.brooklyn.api.catalog.CatalogItem;
+import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
+import org.apache.brooklyn.api.typereg.RegisteredType;
+import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
+import org.apache.brooklyn.core.mgmt.persist.PersistMode;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.os.Os;
 import org.apache.commons.collections.IteratorUtils;
 import org.testng.Assert;
 import org.testng.annotations.AfterMethod;
@@ -34,21 +45,14 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.io.Files;
 
-import org.apache.brooklyn.api.catalog.BrooklynCatalog;
-import org.apache.brooklyn.api.catalog.CatalogItem;
-import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
-import org.apache.brooklyn.core.mgmt.persist.PersistMode;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
-import org.apache.brooklyn.util.core.ResourceUtils;
-import org.apache.brooklyn.util.os.Os;
-
 public class BrooklynLauncherRebindCatalogTest {
 
     private static final String TEST_VERSION = "test-version";
     private static final String CATALOG_INITIAL = "classpath://rebind-test-catalog.bom";
+    private static final String CATALOG_EMPTY_INITIAL = "classpath://rebind-test-empty-catalog.bom";
     private static final String CATALOG_ADDITIONS = "rebind-test-catalog-additions.bom";
-    private static final Iterable<String> EXPECTED_DEFAULT_IDS = ImmutableSet.of("one:" + TEST_VERSION, "two:" + TEST_VERSION);
-    private static final Iterable<String> EXPECTED_ADDED_IDS = ImmutableSet.of("three:" + TEST_VERSION, "four:" + TEST_VERSION);
+    private static final Set<String> EXPECTED_DEFAULT_IDS = ImmutableSet.of("one:" + TEST_VERSION, "two:" + TEST_VERSION);
+    private static final Set<String> EXPECTED_ADDED_IDS = ImmutableSet.of("three:" + TEST_VERSION, "four:" + TEST_VERSION);
 
     private List<BrooklynLauncher> launchers = Lists.newCopyOnWriteArrayList();
     
@@ -59,9 +63,13 @@ public class BrooklynLauncherRebindCatalogTest {
         }
         launchers.clear();
     }
-    
+
     private BrooklynLauncher newLauncherForTests(String persistenceDir) {
-        CatalogInitialization catalogInitialization = new CatalogInitialization(CATALOG_INITIAL);
+        return newLauncherForTests(persistenceDir, CATALOG_INITIAL);
+    }
+    
+    private BrooklynLauncher newLauncherForTests(String persistenceDir, String catalogInitial) {
+        CatalogInitialization catalogInitialization = new CatalogInitialization(catalogInitial);
         BrooklynLauncher launcher = BrooklynLauncher.newInstance()
                 .brooklynProperties(LocalManagementContextForTests.builder(true).buildProperties())
                 .catalogInitialization(catalogInitialization)
@@ -73,37 +81,105 @@ public class BrooklynLauncherRebindCatalogTest {
     }
 
     @Test
-    public void testRebindDoesNotEffectCatalog() {
+    public void testRebindGetsInitialCatalog() {
         String persistenceDir = newTempPersistenceContainerName();
 
         BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
         launcher.start();
-        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
+        assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
 
-        assertCatalogConsistsOfIds(catalog.getCatalogItems(), EXPECTED_DEFAULT_IDS);
+        launcher.terminate();
 
-        catalog.deleteCatalogItem("one", TEST_VERSION);
-        catalog.deleteCatalogItem("two", TEST_VERSION);
+        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS);
+    }
+
+    @Test
+    public void testRebindPersistsInitialCatalog() {
+        String persistenceDir = newTempPersistenceContainerName();
 
-        Assert.assertEquals(Iterables.size(catalog.getCatalogItems()), 0);
+        BrooklynLauncher launcher = newLauncherForTests(persistenceDir, CATALOG_INITIAL);
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
+
+        launcher.terminate();
 
+        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir, CATALOG_EMPTY_INITIAL);
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS);
+    }
+
+    @Test
+    public void testRebindGetsUnionOfInitialAndPersisted() {
+        String persistenceDir = newTempPersistenceContainerName();
+
+        BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
+        launcher.start();
+        assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS);
+
+        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
         catalog.addItems(new ResourceUtils(this).getResourceAsString(CATALOG_ADDITIONS));
+        assertCatalogConsistsOfIds(launcher, Iterables.concat(EXPECTED_DEFAULT_IDS, EXPECTED_ADDED_IDS));
+
+        launcher.terminate();
+
+        BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
+        newLauncher.start();
+        assertCatalogConsistsOfIds(newLauncher, Iterables.concat(EXPECTED_DEFAULT_IDS, EXPECTED_ADDED_IDS));
+    }
 
-        assertCatalogConsistsOfIds(catalog.getCatalogItems(), EXPECTED_ADDED_IDS);
+    // In CatalogInitialization, we take the union of the initial catalog and the persisted state catalog.
+    // Therefore removals from the original catalog do not take effect.
+    // That is acceptable - better than previously where, after upgrading Brooklyn, one had to run
+    // `br catalog add `${BROOKLYN_HOME}/catalog/catalog.bom` to add the new catalog items to the existing
+    // persisted state.
+    @Test(groups="Broken")
+    public void testRemovedInitialItemStillRemovedAfterRebind() {
+        Set<String> EXPECTED_DEFAULT_IDS_WITHOUT_ONE = MutableSet.<String>builder()
+                .addAll(EXPECTED_DEFAULT_IDS)
+                .remove("one:" + TEST_VERSION).build();
+        
+        String persistenceDir = newTempPersistenceContainerName();
 
+        BrooklynLauncher launcher = newLauncherForTests(persistenceDir);
+        launcher.start();
+
+        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
+        catalog.deleteCatalogItem("one", TEST_VERSION);
+        assertCatalogConsistsOfIds(launcher, EXPECTED_DEFAULT_IDS_WITHOUT_ONE);
+        
         launcher.terminate();
 
         BrooklynLauncher newLauncher = newLauncherForTests(persistenceDir);
         newLauncher.start();
-        assertCatalogConsistsOfIds(newLauncher.getServerDetails().getManagementContext().getCatalog().getCatalogItems(), EXPECTED_ADDED_IDS);
+        assertCatalogConsistsOfIds(newLauncher, EXPECTED_DEFAULT_IDS_WITHOUT_ONE);
+    }
+
+    private void assertCatalogConsistsOfIds(BrooklynLauncher launcher, Iterable<String> ids) {
+        BrooklynTypeRegistry typeRegistry = launcher.getServerDetails().getManagementContext().getTypeRegistry();
+        BrooklynCatalog catalog = launcher.getServerDetails().getManagementContext().getCatalog();
+        assertTypeRegistryConsistsOfIds(typeRegistry.getAll(), ids);
+        assertCatalogConsistsOfIds(catalog.getCatalogItems(), ids);
     }
 
     private void assertCatalogConsistsOfIds(Iterable<CatalogItem<Object, Object>> catalogItems, Iterable<String> ids) {
         Iterable<String> idsFromItems = Iterables.transform(catalogItems, new Function<CatalogItem<?,?>, String>() {
             @Nullable
             @Override
-            public String apply(CatalogItem<?, ?> catalogItem) {
-                return catalogItem.getCatalogItemId();
+            public String apply(CatalogItem<?, ?> input) {
+                return input.getCatalogItemId();
+            }
+        });
+        Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));
+    }
+
+    private void assertTypeRegistryConsistsOfIds(Iterable<RegisteredType> types, Iterable<String> ids) {
+        Iterable<String> idsFromItems = Iterables.transform(types, new Function<RegisteredType, String>() {
+            @Nullable
+            @Override
+            public String apply(RegisteredType input) {
+                return input.getId();
             }
         });
         Assert.assertTrue(compareIterablesWithoutOrderMatters(ids, idsFromItems), String.format("Expected %s, found %s", ids, idsFromItems));

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherTest.java
index c8d4352..09c08e9 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherTest.java
@@ -35,6 +35,7 @@ import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
 import org.apache.brooklyn.core.internal.BrooklynProperties;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.mgmt.persist.PersistMode;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
 import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.core.test.entity.TestApplication;
@@ -272,12 +273,11 @@ public class BrooklynLauncherTest {
     @Test  // takes a bit of time because starts webapp, but also tests rest api so useful
     public void testErrorsCaughtByApiAndRestApiWorks() throws Exception {
         launcher = newLauncherForTests(true)
-                .catalogInitialization(new CatalogInitialization(null).addPopulationCallback(new Function<CatalogInitialization, Void>() {
-                    @Override
-                    public Void apply(CatalogInitialization input) {
+                .catalogInitialization(new CatalogInitialization(null) {
+                    @Override public void populateInitialCatalogOnly() {
                         throw new RuntimeException("deliberate-exception-for-testing");
-                    }
-                }))
+                    }})
+                .persistMode(PersistMode.DISABLED)
                 .installSecurityFilter(false)
                 .start();
         // 'deliberate-exception' error above should be thrown, then caught in this calling thread

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/launcher/src/test/resources/rebind-test-empty-catalog.bom
----------------------------------------------------------------------
diff --git a/launcher/src/test/resources/rebind-test-empty-catalog.bom b/launcher/src/test/resources/rebind-test-empty-catalog.bom
new file mode 100644
index 0000000..53b914f
--- /dev/null
+++ b/launcher/src/test/resources/rebind-test-empty-catalog.bom
@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+brooklyn.catalog:
+  version: "test-version"
+  itemType: entity
+  items:
+
+  # Do not scan classpath with for classes with a @Catalog annotation
+  - scanJavaAnnotations: false

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
index b5136e6..2ae0183 100644
--- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
+++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
@@ -140,7 +140,9 @@ public abstract class BrooklynRestApiTest {
     protected synchronized ManagementContext getManagementContext() {
         if (manager==null) {
             if (useLocalScannedCatalog()) {
-                manager = new LocalManagementContext();
+                manager = LocalManagementContextForTests.builder(true)
+                        .enableOsgiReusable()
+                        .build();
                 forceUseOfDefaultCatalogWithJavaClassPath();
             } else {
                 manager = new LocalManagementContextForTests();


[2/6] brooklyn-server git commit: Delete RebindCatalogEntityTest

Posted by tb...@apache.org.
Delete RebindCatalogEntityTest

Was testing use of catalog class loader, not OSGi, with catalog item
requiring custom class loader being added programmatically via 
catalog.addItem() and then fiddling with catalog on rebind to add it 
back in!

No user should be doing that; the test is not something we want to
support.

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/4ef74fac
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/4ef74fac
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/4ef74fac

Branch: refs/heads/master
Commit: 4ef74facfecefe4a265dc9dda81a8746ceb5f66d
Parents: 802cbb9
Author: Aled Sage <al...@gmail.com>
Authored: Wed Oct 4 10:36:19 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Oct 17 09:02:51 2017 +0100

----------------------------------------------------------------------
 .../mgmt/rebind/RebindCatalogEntityTest.java    | 153 -------------------
 1 file changed, 153 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4ef74fac/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
deleted file mode 100644
index b53d92c..0000000
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindCatalogEntityTest.java
+++ /dev/null
@@ -1,153 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.brooklyn.core.mgmt.rebind;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNotSame;
-
-import java.io.Closeable;
-import java.net.URL;
-import java.util.List;
-
-import org.apache.brooklyn.api.entity.Application;
-import org.apache.brooklyn.api.entity.EntitySpec;
-import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
-import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
-import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.entity.AbstractApplication;
-import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.core.entity.EntityInternal;
-import org.apache.brooklyn.core.entity.StartableApplication;
-import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
-import org.apache.brooklyn.core.sensor.Sensors;
-import org.apache.brooklyn.test.support.TestResourceUnavailableException;
-import org.apache.brooklyn.util.core.javalang.UrlClassLoader;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-import com.google.common.base.Function;
-
-public class RebindCatalogEntityTest extends RebindTestFixture<StartableApplication> {
-
-    @SuppressWarnings("unused")
-    private static final Logger LOG = LoggerFactory.getLogger(RebindCatalogEntityTest.class);
-
-    /*
-     * Code contained in brooklyn-AppInCatalog.jar is:
-     * 
-     * package brooklyn.entity.rebind;
-     * public class AppInCatalog extends AbstractApplication {
-     *     public static final ConfigKey<String> MY_CONF = ConfigKeys.newStringConfigKey("myconf");
-     *     public static final AttributeSensor<String> MY_SENSOR = Sensors.newStringSensor("mysensor");
-     * }
-     */
-
-    private static final String JAR_PATH = "/org/apache/brooklyn/core/test/rebind/sample-app-in-catalog/brooklyn-AppInCatalog.jar";
-    private static final String APP_CLASSNAME = "org.apache.brooklyn.core.test.rebind.sample_app_in_catalog.AppInCatalog";
-
-    private URL url;
-
-    @Override
-    protected boolean useEmptyCatalog() {
-        return true;
-    }
-
-    @Override
-    protected StartableApplication createApp() {
-        // do nothing here
-        return null;
-    }
-    
-    @BeforeMethod(alwaysRun=true)
-    @Override
-    public void setUp() throws Exception {
-        TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), JAR_PATH);
-        url = getClass().getResource(JAR_PATH);
-        assertNotNull(url, "Could not find on classpath: "+JAR_PATH);
-        super.setUp();
-    }
-
-    // TODO Failed in jenkins (once on 20141104, with invocationCount=100): mysensor was null post-rebind.
-    //
-    // Note: to test before/after behaviour (i.e. that we're really fixing what we think we are) then comment out the body of:
-    //       AbstractMemento.injectTypeClass(Class)
-    //
-    // NB: this behaviour is generally deprecated in favour of OSGi now.
-    @SuppressWarnings("resource")
-    @Test 
-    public void testRestoresAppFromCatalogClassloader() throws Exception {
-        @SuppressWarnings("unchecked")
-        Class<? extends AbstractApplication> appClazz = (Class<? extends AbstractApplication>) new UrlClassLoader(url).loadClass(APP_CLASSNAME);
-        origManagementContext.getCatalog().addItem(appClazz);
-        
-        EntitySpec<StartableApplication> appSpec = EntitySpec.create(StartableApplication.class, appClazz)
-                .configure("myconf", "myconfval");
-        origApp = origManagementContext.getEntityManager().createEntity(appSpec);
-        ((EntityInternal)origApp).sensors().set(Sensors.newStringSensor("mysensor"), "mysensorval");
-        
-        newApp = rebindWithAppClass();
-        Entities.dumpInfo(newApp);
-        assertNotSame(newApp, origApp);
-        assertEquals(newApp.getId(), origApp.getId());
-        assertEquals(newApp.getClass().getName(), APP_CLASSNAME);
-        assertEquals(newApp.getEntityType().getName(), APP_CLASSNAME);
-        assertEquals(newApp.getAttribute(Sensors.newStringSensor("mysensor")), "mysensorval");
-        assertEquals(newApp.getConfig(ConfigKeys.newStringConfigKey("myconf")), "myconfval");
-    }
-    
-    @Test(invocationCount=100, groups="Integration")
-    public void testRestoresAppFromCatalogClassloaderManyTimes() throws Exception {
-        // Need to fix package name and rebuild brooklyn-AppInCatalog.jar
-        //  or better add it as a new test-bundles subproject
-        testRestoresAppFromCatalogClassloader();
-    }
-    
-    // TODO Not using RebindTestUtils.rebind(mementoDir, getClass().getClassLoader());
-    //      because that won't have right catalog classpath.
-    //      How to reuse that code cleanly?
-    protected StartableApplication rebindWithAppClass() throws Exception {
-        RebindTestUtils.stopPersistence(origApp);
-        LocalManagementContext newManagementContext = RebindTestUtils.newPersistingManagementContextUnstarted(mementoDir, classLoader);
-
-        UrlClassLoader ucl = new UrlClassLoader(url);
-        @SuppressWarnings("unchecked")
-        final Class<? extends AbstractApplication> appClazz = (Class<? extends AbstractApplication>) ucl.loadClass(APP_CLASSNAME);
-        // ucl.close is only introduced in java 1.7
-        if (ucl instanceof Closeable) ((Closeable)ucl).close();
-
-        newManagementContext.getCatalogInitialization().addPopulationCallback(new Function<CatalogInitialization, Void>() {
-            @Override
-            public Void apply(CatalogInitialization input) {
-                newManagementContext.getCatalog().addItem(appClazz);
-                return null;
-            }
-        });
-        
-        ClassLoader classLoader = newManagementContext.getCatalog().getRootClassLoader();
-        
-        classLoader.loadClass(appClazz.getName());
-        List<Application> newApps = newManagementContext.getRebindManager().rebind(classLoader, null, ManagementNodeState.MASTER);
-        newManagementContext.getRebindManager().startPersistence();
-        return (StartableApplication) newApps.get(0);
-    }
-
-}
\ No newline at end of file


[6/6] brooklyn-server git commit: This closes #852

Posted by tb...@apache.org.
This closes #852


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/efa4af66
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/efa4af66
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/efa4af66

Branch: refs/heads/master
Commit: efa4af664e5de3a412d955458d380b67719728f7
Parents: 7c6f7d4 48d9999
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Authored: Tue Oct 17 10:19:16 2017 +0100
Committer: Thomas Bouron <th...@cloudsoftcorp.com>
Committed: Tue Oct 17 10:19:16 2017 +0100

----------------------------------------------------------------------
 .../brooklyn/api/catalog/BrooklynCatalog.java   |  11 +
 .../catalog/internal/BasicBrooklynCatalog.java  |  36 ++
 .../catalog/internal/CatalogInitialization.java | 462 +++++++++++--------
 .../internal/AbstractManagementContext.java     |   8 +-
 .../mgmt/internal/LocalManagementContext.java   |   4 -
 .../rebind/PeriodicDeltaChangeListener.java     |   2 -
 .../core/mgmt/rebind/RebindIteration.java       |  37 +-
 .../ha/HighAvailabilityManagerTestFixture.java  |   3 +-
 .../mgmt/rebind/RebindCatalogEntityTest.java    | 153 ------
 .../core/mgmt/rebind/RebindEntityTest.java      |   2 +
 .../rebind/RebindHistoricCatalogItemTest.java   |  53 +++
 .../core/mgmt/rebind/RebindTestFixture.java     |  11 -
 .../core/mgmt/rebind/RebindTestUtils.java       |   3 +
 .../entity/LocalManagementContextForTests.java  |  12 +
 .../core/mgmt/rebind/catalog-myapp_1.0.0        |  36 ++
 .../brooklyn/launcher/common/BasicLauncher.java | 125 ++---
 launcher/pom.xml                                |   7 +
 .../AbstractBrooklynLauncherRebindTest.java     | 177 +++++++
 .../BrooklynLauncherHighAvailabilityTest.java   | 118 +----
 .../BrooklynLauncherRebindAppsTest.java         |  42 ++
 .../BrooklynLauncherRebindCatalogOsgiTest.java  | 219 +++++++++
 .../BrooklynLauncherRebindCatalogTest.java      | 140 +++---
 .../brooklyn/launcher/BrooklynLauncherTest.java |  10 +-
 .../resources/rebind-test-empty-catalog.bom     |  22 +
 .../rest/testing/BrooklynRestApiTest.java       |   4 +-
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |   3 +-
 .../brooklyn/rest/HaMasterCheckFilterTest.java  |   9 +-
 .../main/java/org/apache/brooklyn/cli/Main.java |  76 +--
 28 files changed, 1057 insertions(+), 728 deletions(-)
----------------------------------------------------------------------



[3/6] brooklyn-server git commit: Merge initial catalog and persisted catalog

Posted by tb...@apache.org.
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index a003f79..b7c6b2e 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -37,6 +37,7 @@ import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
 import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.rest.filter.CorsImplSupplierFilter;
 import org.apache.brooklyn.rest.filter.CsrfTokenFilter;
 import org.apache.brooklyn.rest.filter.EntitlementContextFilter;
@@ -167,7 +168,7 @@ public class BrooklynRestApiLauncher {
 
     public Server start() {
         if (this.mgmt == null) {
-            mgmt = new LocalManagementContext();
+            mgmt = new LocalManagementContextForTests();
         }
         BrooklynCampPlatformLauncherAbstract platform = new BrooklynCampPlatformLauncherNoServer()
                 .useManagementContext(mgmt)

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
index 9d345ce..d03abb5 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
@@ -130,13 +130,8 @@ public class HaMasterCheckFilterTest extends BrooklynRestApiLauncherTestFixture
                 .persistPeriodMillis(1)
                 .forLive(false)
                 .emptyCatalog(true)
-                .buildUnstarted();
-
-        if (mode == HighAvailabilityMode.DISABLED) {
-            mgmt.getHighAvailabilityManager().disabled();
-        } else {
-            mgmt.getHighAvailabilityManager().start(mode);
-        }
+                .haMode(mode)
+                .buildStarted();
 
         new BrooklynCampPlatformLauncherNoServer()
             .useManagementContext(mgmt)

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/802cbb9b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 2ede25a..9d037f6 100644
--- a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -32,17 +32,12 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
 
 import org.apache.brooklyn.api.catalog.BrooklynCatalog;
-import org.apache.brooklyn.api.catalog.CatalogItem;
-import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
-import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
-import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind;
-import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.cli.CloudExplorer.BlobstoreGetBlobCommand;
 import org.apache.brooklyn.cli.CloudExplorer.BlobstoreListContainerCommand;
 import org.apache.brooklyn.cli.CloudExplorer.BlobstoreListContainersCommand;
@@ -62,8 +57,6 @@ import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
 import org.apache.brooklyn.core.mgmt.persist.BrooklynPersistenceUtils;
 import org.apache.brooklyn.core.mgmt.persist.PersistMode;
 import org.apache.brooklyn.core.mgmt.rebind.transformer.CompoundTransformer;
-import org.apache.brooklyn.core.objs.BrooklynTypes;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.launcher.BrooklynLauncher;
 import org.apache.brooklyn.launcher.BrooklynServerDetails;
@@ -87,11 +80,8 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
 import com.google.common.base.Objects.ToStringHelper;
-import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 
 import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyShell;
@@ -111,7 +101,6 @@ import io.airlift.airline.Option;
  * <li> providing an overridden {@link LaunchCommand} via {@link #cliLaunchCommand()} if desired
  * <li> providing any other CLI customisations by overriding {@link #cliBuilder()}
  *      (typically calling the parent and then customizing the builder)
- * <li> populating a custom catalog using {@link LaunchCommand#populateCatalog(BrooklynCatalog)}
  */
 public class Main extends AbstractMain {
 
@@ -410,20 +399,6 @@ public class Main extends AbstractMain {
                 launcher = createLauncher();
 
                 CatalogInitialization catInit = new CatalogInitialization(catalogInitial);
-                catInit.addPopulationCallback(new Function<CatalogInitialization,Void>() {
-                    @Override
-                    public Void apply(CatalogInitialization catInit) {
-                        try {
-                            populateCatalog(catInit.getManagementContext().getCatalog());
-                        } catch (Throwable e) {
-                            catInit.handleException(e, "in main class populate catalog override");
-                        }
-                        
-                        // Force load of catalog (so web console is up to date)
-                        confirmCatalog(catInit);
-                        return null;
-                    }
-                });
                 catInit.setFailOnStartupErrors(startupFailOnCatalogErrors);
                 launcher.catalogInitialization(catInit);
                 
@@ -614,54 +589,15 @@ public class Main extends AbstractMain {
             return launcher;
         }
 
-        /** method intended for subclassing, to add custom items to the catalog */
-        protected void populateCatalog(BrooklynCatalog catalog) {
+        /**
+         * method intended for subclassing, to add custom items to the catalog.
+         * 
+         * @deprecated since 0.13.0; no longer supported; does nothing - subclasses should not try to extend it!
+         */
+        protected final void populateCatalog(BrooklynCatalog catalog) {
             // nothing else added here
         }
 
-        protected void confirmCatalog(CatalogInitialization catInit) {
-            // Force load of catalog (so web console is up to date)
-            Stopwatch time = Stopwatch.createStarted();
-            Iterable<RegisteredType> all = catInit.getManagementContext().getTypeRegistry().getAll();
-            int errors = 0;
-            for (RegisteredType rt: all) {
-                if (RegisteredTypes.isTemplate(rt)) {
-                    // skip validation of templates, they might contain instructions,
-                    // and additionally they might contain multiple items in which case
-                    // the validation below won't work anyway (you need to go via a deployment plan)
-                } else {
-                    if (rt.getKind()==RegisteredTypeKind.UNRESOLVED) {
-                        errors++;
-                        catInit.handleException(new UserFacingException("Unresolved type in catalog"), rt);
-                    }
-                }
-            }
-
-            // and force resolution of legacy items
-            BrooklynCatalog catalog = catInit.getManagementContext().getCatalog();
-            Iterable<CatalogItem<Object, Object>> items = catalog.getCatalogItemsLegacy();
-            for (CatalogItem<Object, Object> item: items) {
-                try {
-                    if (item.getCatalogItemType()==CatalogItemType.TEMPLATE) {
-                        // skip validation of templates, they might contain instructions,
-                        // and additionally they might contain multiple items in which case
-                        // the validation below won't work anyway (you need to go via a deployment plan)
-                    } else {
-                        AbstractBrooklynObjectSpec<?, ?> spec = catalog.peekSpec(item);
-                        if (spec instanceof EntitySpec) {
-                            BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType());
-                        }
-                        log.debug("Catalog loaded spec "+spec+" for item "+item);
-                    }
-                } catch (Throwable throwable) {
-                    catInit.handleException(throwable, item);
-                }
-            }
-            
-            log.debug("Catalog (size "+Iterables.size(all)+", of which "+Iterables.size(items)+" legacy) confirmed in "+Duration.of(time)+(errors>0 ? ", errors found ("+errors+")" : ""));
-            // nothing else added here
-        }
-        
         /** convenience for subclasses to specify that an app should run,
          * throwing the right (caught) error if another app has already been specified */
         protected void setAppToLaunch(String className) {