Repository: brooklyn-server
Updated Branches:
  refs/heads/master e2c0247b6 -> 213a4b332


Move catalog init code to CatalogInitialization

Move code from `RebindIteration` so it delegates everything to the 
CatalogInitialization.

Just refactors (no change in behaviour).

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

Branch: refs/heads/master
Commit: d268fe36d404935411d854c3c99f8084c49ff0d4
Parents: 0adf3fd
Author: Aled Sage <[email protected]>
Authored: Wed Sep 27 15:02:33 2017 +0100
Committer: Aled Sage <[email protected]>
Committed: Mon Oct 2 13:50:33 2017 +0100

----------------------------------------------------------------------
 .../catalog/internal/CatalogInitialization.java | 192 +++++++++++++++++--
 .../core/mgmt/rebind/RebindIteration.java       | 177 +++++++----------
 .../mgmt/rebind/RebindCatalogEntityTest.java    |   2 +-
 3 files changed, 256 insertions(+), 115 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d268fe36/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 282695e..5a67b1f 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
@@ -19,16 +19,29 @@
 package org.apache.brooklyn.core.catalog.internal;
 
 import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
 import java.util.Collection;
 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.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.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.internal.ManagementContextInternal;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
+import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.FatalRuntimeException;
@@ -36,14 +49,18 @@ import 
org.apache.brooklyn.util.exceptions.PropagatedRuntimeException;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
 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.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.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 
 @Beta
 public class CatalogInitialization implements ManagementContextInjectable {
@@ -70,17 +87,16 @@ public class CatalogInitialization implements 
ManagementContextInjectable {
 
     private boolean disallowLocal = false;
     private List<Function<CatalogInitialization, Void>> callbacks = 
MutableList.of();
-    private boolean 
-        /** has run an unofficial initialization (i.e. an early load, 
triggered by an early read of the catalog) */
-        hasRunUnofficialInitialization = false, 
-        /** has run an official initialization, but it is not a permanent one 
(e.g. during a hot standby mode, or a run failed) */
-        hasRunTransientOfficialInitialization = false, 
-        /** has run an official initialization which is permanent (node is 
master, and the new catalog is now set) */
-        hasRunFinalInitialization = false;
+    /** has run an unofficial initialization (i.e. an early load, triggered by 
an early read of the catalog) */
+    private boolean hasRunUnofficialInitialization = 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; 
+    /** 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 ManagementContext managementContext;
+    private ManagementContextInternal managementContext;
     private boolean isStartingUp = false;
     private boolean failOnStartupErrors = false;
     
@@ -99,7 +115,7 @@ public class CatalogInitialization implements 
ManagementContextInjectable {
         Preconditions.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 = managementContext;
+        this.managementContext = (ManagementContextInternal) managementContext;
     }
     
     /** Called by the framework to set true while starting up, and false 
afterwards,
@@ -117,7 +133,7 @@ public class CatalogInitialization implements 
ManagementContextInjectable {
         return this;
     }
     
-    public ManagementContext getManagementContext() {
+    public ManagementContextInternal getManagementContext() {
         return Preconditions.checkNotNull(managementContext, "management 
context has not been injected into "+this);
     }
 
@@ -126,15 +142,60 @@ public class CatalogInitialization implements 
ManagementContextInjectable {
      * (or an initialization done by the startup routines when not running 
persistence);
      * see also {@link #hasRunAnyInitialization()}. */
     public 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; }
+    
     /** 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; }
 
+    /**
+     * 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.
+     * 
+     * @param mode
+     * @param persistedState
+     * @param exceptionHandler
+     * @param rebindLogger
+     */
+    @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;
+            }
+        }
+
+        // TODO in read-only mode, perhaps do this less frequently than 
entities etc, maybe only if things change?
+        populateCatalog(mode, needsInitialItemsLoaded, 
needsAdditionalItemsLoaded, itemsForResettingCatalog);
+    }
+    
     /** 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, 
@@ -326,11 +387,120 @@ public class CatalogInitialization implements 
ManagementContextInjectable {
         log.warn(Exceptions.collapseText(wrap));
         log.debug("Trace for: "+wrap, wrap);
 
-        ((ManagementContextInternal)getManagementContext()).errors().add(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) {
+        List<OsgiBundleInstallationResult> installs = MutableList.of();
+
+        // Install the bundles
+        for (String bundleId : persistedState.getBundleIds()) {
+            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);
+            }
+        }
+        
+        // Start the bundles (now that we've installed them all)
+        Set<RegisteredType> installedTypes = MutableSet.of();
+        for (OsgiBundleInstallationResult br: installs) {
+            try {
+                startBundle(br);
+                Iterables.addAll(installedTypes, 
managementContext.getTypeRegistry().getMatching(
+                    
RegisteredTypePredicates.containingBundle(br.getVersionedName())));
+            } catch (Exception e) {
+                
exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, 
br.getMetadata().getId(), br.getMetadata().getSymbolicName(), e);
+            }
+        }
+        
+        // Validate that they all started successfully
+        if (!installedTypes.isEmpty()) {
+            validateAllTypes(installedTypes, exceptionHandler);
+        }
+    }
+    
+    private void validateAllTypes(Set<RegisteredType> installedTypes, 
RebindExceptionHandler exceptionHandler) {
+        Stopwatch sw = Stopwatch.createStarted();
+        log.debug("Getting catalog to validate all types");
+        final BrooklynCatalog catalog = this.managementContext.getCatalog();
+        log.debug("Got catalog in {} now validate", sw.toString());
+        sw.reset(); sw.start();
+        Map<RegisteredType, Collection<Throwable>> validationErrors = 
catalog.validateTypes( installedTypes );
+        log.debug("Validation done in {}", sw.toString());
+        if (!validationErrors.isEmpty()) {
+            Map<VersionedName, Map<RegisteredType,Collection<Throwable>>> 
errorsByBundle = MutableMap.of();
+            for (RegisteredType t: validationErrors.keySet()) {
+                VersionedName vn = 
VersionedName.fromString(t.getContainingBundle());
+                Map<RegisteredType, Collection<Throwable>> errorsInBundle = 
errorsByBundle.get(vn);
+                if (errorsInBundle==null) {
+                    errorsInBundle = MutableMap.of();
+                    errorsByBundle.put(vn, errorsInBundle);
+                }
+                errorsInBundle.put(t, validationErrors.get(t));
+            }
+            for (VersionedName vn: errorsByBundle.keySet()) {
+                Map<RegisteredType, Collection<Throwable>> errorsInBundle = 
errorsByBundle.get(vn);
+                ManagedBundle b = 
managementContext.getOsgiManager().get().getManagedBundle(vn);
+                String id = b!=null ? b.getId() : /* just in case it was 
uninstalled concurrently somehow */ vn.toString();
+                
exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE,
+                    id,
+                    vn.getSymbolicName(),
+                    Exceptions.create("Failed to install "+vn+", types 
"+errorsInBundle.keySet()+" gave errors",
+                        Iterables.concat(errorsInBundle.values())));
+            }
+        }
+    }
+
+    /** 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) {
+        return 
getManagementContext().getOsgiManager().get().installDeferredStart(bundle, 
zipInput, false).get();
+    }
+    
+    public void startBundle(OsgiBundleInstallationResult br) throws 
BundleException {
+        if (br.getDeferredStart()!=null) {
+            br.getDeferredStart().run();
+        }
+    }
+
+    public interface RebindLogger {
+        void debug(String message, Object... args);
+    }
+    
+    public interface InstallableManagedBundle {
+        public ManagedBundle getManagedBundle();
+        /** The caller is responsible for closing the returned stream. */
+        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();
+        
+        /**
+         * The persisted bundles (from the {@code /bundles} sub-directory of 
the persisted state).
+         */
+        public Set<String> getBundleIds();
+        
+        /**
+         * Loads the details of a particular bundle, so it can be installed.
+         */
+        public InstallableManagedBundle getInstallableManagedBundle(String id);
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d268fe36/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 1645784..326ebc0 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
@@ -25,8 +25,10 @@ import static 
org.apache.brooklyn.core.catalog.internal.CatalogUtils.newClassLoa
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -70,6 +72,7 @@ import org.apache.brooklyn.core.BrooklynFeatureEnablement;
 import org.apache.brooklyn.core.BrooklynLogging;
 import org.apache.brooklyn.core.BrooklynLogging.LoggingLevel;
 import org.apache.brooklyn.core.catalog.internal.CatalogInitialization;
+import 
org.apache.brooklyn.core.catalog.internal.CatalogInitialization.InstallableManagedBundle;
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
 import org.apache.brooklyn.core.enricher.AbstractEnricher;
 import org.apache.brooklyn.core.entity.AbstractApplication;
@@ -80,7 +83,6 @@ import org.apache.brooklyn.core.location.AbstractLocation;
 import org.apache.brooklyn.core.location.internal.LocationInternal;
 import 
org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContextSequential;
 import 
org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
-import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult;
 import org.apache.brooklyn.core.mgmt.internal.BrooklynObjectManagementMode;
 import org.apache.brooklyn.core.mgmt.internal.BrooklynObjectManagerInternal;
 import org.apache.brooklyn.core.mgmt.internal.EntityManagerInternal;
@@ -100,16 +102,13 @@ import 
org.apache.brooklyn.core.objs.proxy.InternalPolicyFactory;
 import org.apache.brooklyn.core.policy.AbstractPolicy;
 import org.apache.brooklyn.core.typereg.BasicManagedBundle;
 import org.apache.brooklyn.core.typereg.RegisteredTypeNaming;
-import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.ClassLoaderUtils;
 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;
@@ -120,7 +119,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -323,62 +321,94 @@ public abstract class RebindIteration {
     }
 
     protected void installBundlesAndRebuildCatalog() {
-        
-        // Build catalog early so we can load other things
+        // Build catalog early so we can load other things.
+        // Reads the persisted catalog contents, and passes it all to 
CatalogInitialization, which decides what to do with it.
         checkEnteringPhase(2);
         
-        // Install bundles
-        if (rebindManager.persistBundlesEnabled) {
-            List<OsgiBundleInstallationResult> installs = MutableList.of();
-            logRebindingDebug("RebindManager installing bundles: {}", 
mementoManifest.getBundleIds());
-            for (ManagedBundleMemento bundleM : 
mementoManifest.getBundles().values()) {
-                logRebindingDebug("RebindManager installing bundle {}", 
bundleM.getId());
-                try (InputStream in = bundleM.getJarContent().openStream()) {
-                    
installs.add(rebindContext.installBundle(instantiator.newManagedBundle(bundleM),
 in));
-                } catch (Exception e) {
-                    
exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, 
bundleM.getId(), bundleM.getSymbolicName(), e);
-                }
+        CatalogInitialization.RebindLogger rebindLogger = new 
CatalogInitialization.RebindLogger() {
+            @Override
+            public void debug(String message, Object... args) {
+                logRebindingDebug(message, args);
             }
-            // Start them all after we've installed them
-            Set<RegisteredType> installedTypes = MutableSet.of();
-            for (OsgiBundleInstallationResult br: installs) {
-                try {
-                    rebindContext.startBundle(br);
-                    Iterables.addAll(installedTypes, 
managementContext.getTypeRegistry().getMatching(
-                        
RegisteredTypePredicates.containingBundle(br.getVersionedName())));
-                } catch (Exception e) {
-                    
exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, 
br.getMetadata().getId(), br.getMetadata().getSymbolicName(), e);
-                }
+        };
+
+        class InstallableManagedBundleImpl implements 
CatalogInitialization.InstallableManagedBundle {
+            private final ManagedBundleMemento memento;
+            private final ManagedBundle managedBundle;
+            
+            InstallableManagedBundleImpl(ManagedBundleMemento memento, 
ManagedBundle managedBundle) {
+                this.memento = memento;
+                this.managedBundle = managedBundle;
+            }
+
+            @Override public ManagedBundle getManagedBundle() {
+                return managedBundle;
+            }
+
+            @Override public InputStream getInputStream() throws IOException {
+                return memento.getJarContent().openStream();
             }
-            if (!installedTypes.isEmpty()) {
-                validateAllTypes(installedTypes);
+        }
+        
+        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<>();
+        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));
+            }
         } else {
             logRebindingDebug("Not rebinding bundles; feature disabled: {}", 
mementoManifest.getBundleIds());
         }
         
-        // Do legacy items
+        // Construct the legacy catalog items; don't add them to the catalog 
here, 
+        // but instead pass them to catalogInitialization.populateCatalog.
         
-        // Instantiate catalog items
         if (rebindManager.persistCatalogItemsEnabled) {
+            // Instantiate catalog items
             logRebindingDebug("RebindManager instantiating catalog items: {}", 
mementoManifest.getCatalogItemIds());
             for (CatalogItemMemento catalogItemMemento : 
mementoManifest.getCatalogItemMementos().values()) {
                 logRebindingDebug("RebindManager instantiating catalog item 
{}", catalogItemMemento);
                 try {
                     CatalogItem<?, ?> catalogItem = 
instantiator.newCatalogItem(catalogItemMemento);
                     
rebindContext.registerCatalogItem(catalogItemMemento.getId(), catalogItem);
+                    legacyCatalogItems.add(catalogItem);
                 } catch (Exception e) {
                     
exceptionHandler.onCreateFailed(BrooklynObjectType.CATALOG_ITEM, 
catalogItemMemento.getId(), catalogItemMemento.getType(), e);
                 }
             }
-        } else {
-            logRebindingDebug("Not rebinding catalog; feature disabled: {}", 
mementoManifest.getCatalogItemIds());
-        }
-
-        // Reconstruct catalog entries
-        if (rebindManager.persistCatalogItemsEnabled) {
+            
+            // Reconstruct catalog entries
             logRebindingDebug("RebindManager reconstructing catalog items");
             for (CatalogItemMemento catalogItemMemento : 
mementoManifest.getCatalogItemMementos().values()) {
                 CatalogItem<?, ?> item = 
rebindContext.getCatalogItem(catalogItemMemento.getId());
@@ -396,76 +426,17 @@ public abstract class RebindIteration {
                     }
                 }
             }
-        }
 
-        // See notes in CatalogInitialization
-        
-        CatalogInitialization catInit = 
managementContext.getCatalogInitialization();
-        Collection<CatalogItem<?,?>> itemsForResettingCatalog = null;
-        boolean needsInitialItemsLoaded, needsAdditionalItemsLoaded;
-        if (rebindManager.persistCatalogItemsEnabled) {
-            if (!isEmpty) {
-                logRebindingDebug("RebindManager clearing local catalog and 
loading from persisted state");
-                itemsForResettingCatalog = rebindContext.getCatalogItems();
-                needsInitialItemsLoaded = false;
-                // only apply "add" if we haven't yet done so while in MASTER 
mode
-                needsAdditionalItemsLoaded = 
!catInit.hasRunFinalInitialization();
-            } else {
-                if (catInit.hasRunFinalInitialization()) {
-                    logRebindingDebug("RebindManager has already done the 
final official run, not doing anything (even though persisted state empty)");
-                    needsInitialItemsLoaded = false;
-                    needsAdditionalItemsLoaded = false;
-                } else {
-                    logRebindingDebug("RebindManager loading initial catalog 
locally because persisted state empty and the final official run has not yet 
been performed");
-                    needsInitialItemsLoaded = true;
-                    needsAdditionalItemsLoaded = true;
-                }
-            }
         } else {
-            if (catInit.hasRunFinalInitialization()) {
-                logRebindingDebug("RebindManager skipping catalog init because 
it has already run (catalog persistence disabled)");
-                needsInitialItemsLoaded = false;
-                needsAdditionalItemsLoaded = false;
-            } else {
-                logRebindingDebug("RebindManager will initialize catalog 
locally because catalog persistence is disabled and the final official run has 
not yet been performed");
-                needsInitialItemsLoaded = true;
-                needsAdditionalItemsLoaded = true;
-            }
+            logRebindingDebug("Not rebinding catalog; feature disabled: {}", 
mementoManifest.getCatalogItemIds());
         }
 
-        // TODO in read-only mode, perhaps do this less frequently than 
entities etc, maybe only if things change?
-        catInit.populateCatalog(mode, needsInitialItemsLoaded, 
needsAdditionalItemsLoaded, itemsForResettingCatalog);
-    }
 
-    private void validateAllTypes(Set<RegisteredType> installedTypes) {
-        Stopwatch sw = Stopwatch.createStarted();
-        LOG.debug("Getting catalog to validate all types");
-        final BrooklynCatalog catalog = this.managementContext.getCatalog();
-        LOG.debug("Got catalog in {} now validate", sw.toString());
-        sw.reset(); sw.start();
-        Map<RegisteredType, Collection<Throwable>> validationErrors = 
catalog.validateTypes( installedTypes );
-        LOG.debug("Validation done in {}", sw.toString());
-        if (!validationErrors.isEmpty()) {
-            Map<VersionedName, Map<RegisteredType,Collection<Throwable>>> 
errorsByBundle = MutableMap.of();
-            for (RegisteredType t: validationErrors.keySet()) {
-                VersionedName vn = 
VersionedName.fromString(t.getContainingBundle());
-                Map<RegisteredType, Collection<Throwable>> errorsInBundle = 
errorsByBundle.get(vn);
-                if (errorsInBundle==null) {
-                    errorsInBundle = MutableMap.of();
-                    errorsByBundle.put(vn, errorsInBundle);
-                }
-                errorsInBundle.put(t, validationErrors.get(t));
-            }
-            for (VersionedName vn: errorsByBundle.keySet()) {
-                Map<RegisteredType, Collection<Throwable>> errorsInBundle = 
errorsByBundle.get(vn);
-                ManagedBundle b = 
managementContext.getOsgiManager().get().getManagedBundle(vn);
-                
exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE,
-                    b!=null ? b.getId() : /* just in case it was uninstalled 
concurrently somehow */ vn.toString(),
-                    vn.getSymbolicName(),
-                    Exceptions.create("Failed to install "+vn+", types 
"+errorsInBundle.keySet()+" gave errors",
-                        Iterables.concat(errorsInBundle.values())));
-            }
-        }
+        // Delegates to CatalogInitialization; see notes there.
+        CatalogInitialization.PersistedCatalogState persistedCatalogState = 
new PersistedCatalogStateImpl(isEmpty, legacyCatalogItems, bundles);
+        
+        CatalogInitialization catInit = 
managementContext.getCatalogInitialization();
+        catInit.populateCatalog(mode, persistedCatalogState, exceptionHandler, 
rebindLogger);
     }
 
     protected void instantiateLocationsAndEntities() {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d268fe36/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 ac53283..6809bac 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
@@ -137,7 +137,7 @@ public class RebindCatalogEntityTest extends 
RebindTestFixture<StartableApplicat
         
newManagementContext.getCatalogInitialization().addPopulationCallback(new 
Function<CatalogInitialization, Void>() {
             @Override
             public Void apply(CatalogInitialization input) {
-                input.getManagementContext().getCatalog().addItem(appClazz);
+                newManagementContext.getCatalog().addItem(appClazz);
                 return null;
             }
         });

Reply via email to