You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/11/13 23:23:50 UTC

[17/18] incubator-brooklyn git commit: Catalog versioning - address review comments; OSGi rebinding

Catalog versioning - address review comments; OSGi rebinding

When the type for an item being deserialized can't be find try iterating through all available catalog items as a last resort.


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

Branch: refs/heads/master
Commit: cf3a9eb0ed97fef0562f023465e9a5b7edbf601a
Parents: 8bd76ad
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Thu Nov 13 21:44:15 2014 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Thu Nov 13 21:44:15 2014 +0200

----------------------------------------------------------------------
 .../internal/CatalogBundleConverter.java        |  4 ++
 .../catalog/internal/CatalogItemComparator.java |  4 ++
 .../internal/CatalogItemDtoAbstract.java        |  6 +-
 .../AbstractBrooklynObjectRebindSupport.java    |  5 +-
 .../entity/rebind/RebindManagerImpl.java        | 58 +++++++++++++++-----
 .../BrooklynMementoPersisterToObjectStore.java  |  5 +-
 .../CatalogItemLibrariesConverter.java          |  4 ++
 .../internal/BrooklynFeatureEnablement.java     |  9 +++
 8 files changed, 73 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java
index fe77d02..5ef4a96 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java
@@ -28,6 +28,10 @@ import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import com.thoughtworks.xstream.mapper.Mapper;
 
 
+/**
+ *  Convert old-style catalog.xml file formats to the latest version.
+ *  The code is needed only during transition to the new version, can be removed after a while.
+ */
 @Deprecated
 public class CatalogBundleConverter implements Converter {
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
index a202b92..d8a2b2e 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java
@@ -23,6 +23,10 @@ import java.util.Comparator;
 import brooklyn.catalog.CatalogItem;
 import brooklyn.util.text.NaturalOrderComparator;
 
+/**
+ * When using the comparator to sort - first using symbolicName
+ * and if equal puts larger versions first, snapshots at the back.
+ */
 public class CatalogItemComparator implements Comparator<CatalogItem<?, ?>> {
     private static final String SNAPSHOT = "SNAPSHOT";
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
index 9b434cc..fde647b 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
@@ -111,9 +111,9 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO
 
     @Override
     public String getVersion() {
-        //xstream doesn't call constructors
-        //the object is used directly (instead of a memento) when
-        //deserializing directly from catalog.xml
+        // The property is set to NO_VERSION when the object is initialized so it's not supposed to be null ever.
+        // But xstream doesn't call constructors when reading from the catalog.xml file which results in null value
+        // for the version property. That's why we have to fix it in the getter.
         if (version != null) {
             return version;
         } else {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/entity/rebind/AbstractBrooklynObjectRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/AbstractBrooklynObjectRebindSupport.java b/core/src/main/java/brooklyn/entity/rebind/AbstractBrooklynObjectRebindSupport.java
index 9cfaf97..edd70c3 100644
--- a/core/src/main/java/brooklyn/entity/rebind/AbstractBrooklynObjectRebindSupport.java
+++ b/core/src/main/java/brooklyn/entity/rebind/AbstractBrooklynObjectRebindSupport.java
@@ -48,10 +48,7 @@ public abstract class AbstractBrooklynObjectRebindSupport<T extends Memento> imp
         if (LOG.isTraceEnabled()) LOG.trace("Reconstructing: {}", memento.toVerboseString());
 
         instance.setDisplayName(memento.getDisplayName());
-        //check if not already forced at entity creation time
-        if (instance.getCatalogItemId() == null) {
-            instance.setCatalogItemId(memento.getCatalogItemId());
-        }
+        //catalogItemId already set when creating the object
         addConfig(rebindContext, memento);
         addTags(rebindContext, memento);
         addCustoms(rebindContext, memento);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index 07c60a5..5ad0f7d 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -66,7 +66,6 @@ import brooklyn.location.basic.LocationInternal;
 import brooklyn.management.ExecutionContext;
 import brooklyn.management.Task;
 import brooklyn.management.classloading.BrooklynClassLoadingContext;
-import brooklyn.management.classloading.JavaBrooklynClassLoadingContext;
 import brooklyn.management.ha.HighAvailabilityManagerImpl;
 import brooklyn.management.ha.ManagementNodeState;
 import brooklyn.management.internal.EntityManagerInternal;
@@ -94,6 +93,7 @@ import brooklyn.util.collections.MutableMap;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.RuntimeInterruptedException;
 import brooklyn.util.flags.FlagUtils;
+import brooklyn.util.guava.Maybe;
 import brooklyn.util.javalang.Reflections;
 import brooklyn.util.task.BasicExecutionContext;
 import brooklyn.util.task.ScheduledTask;
@@ -660,7 +660,7 @@ public class RebindManagerImpl implements RebindManager {
             for (Map.Entry<String, EntityMementoManifest> entry : mementoManifest.getEntityIdToManifest().entrySet()) {
                 String entityId = entry.getKey();
                 EntityMementoManifest entityManifest = entry.getValue();
-                String catalogItemId = findCatalogItemId(mementoManifest.getEntityIdToManifest(), entityManifest);
+                String catalogItemId = findCatalogItemId(classLoader, mementoManifest.getEntityIdToManifest(), entityManifest);
                 
                 if (LOG.isTraceEnabled()) LOG.trace("RebindManager instantiating entity {}", entityId);
                 
@@ -955,7 +955,7 @@ public class RebindManagerImpl implements RebindManager {
         }
     }
 
-    private String findCatalogItemId(Map<String, EntityMementoManifest> entityIdToManifest, EntityMementoManifest entityManifest) {
+    private String findCatalogItemId(ClassLoader cl, Map<String, EntityMementoManifest> entityIdToManifest, EntityMementoManifest entityManifest) {
         if (entityManifest.getCatalogItemId() != null) {
             return entityManifest.getCatalogItemId();
         }
@@ -998,20 +998,33 @@ public class RebindManagerImpl implements RebindManager {
                     ptr = null;
                 }
             }
+
+            //As a last resort go through all catalog items trying to load the type and use the first that succeeds.
+            //But first check if can be loaded from the default classpath
+            try {
+                cl.loadClass(entityManifest.getType());
+                return null;
+            } catch (ClassNotFoundException e) {
+            }
+
+            for (CatalogItem<?, ?> item : catalog.getCatalogItems()) {
+                BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(managementContext, item);
+                boolean canLoadClass = loader.tryLoadClass(entityManifest.getType()).isPresent();
+                if (canLoadClass) {
+                    return item.getId();
+                }
+            }
         }
         return null;
     }
 
     private BrooklynClassLoadingContext getLoadingContextFromCatalogItemId(String catalogItemId, ClassLoader classLoader, RebindContext rebindContext) {
-        if (catalogItemId != null) {
-            CatalogItem<?, ?> catalogItem = rebindContext.getCatalogItem(catalogItemId);
-            if (catalogItem != null) {
-                return CatalogUtils.newClassLoadingContext(managementContext, catalogItem);
-            } else {
-                throw new IllegalStateException("Failed to load catalog item " + catalogItemId + " required for rebinding.");
-            }
+        Preconditions.checkNotNull(catalogItemId, "catalogItemId required (should not be null)");
+        CatalogItem<?, ?> catalogItem = rebindContext.getCatalogItem(catalogItemId);
+        if (catalogItem != null) {
+            return CatalogUtils.newClassLoadingContext(managementContext, catalogItem);
         } else {
-            return JavaBrooklynClassLoadingContext.create(managementContext, classLoader);
+            throw new IllegalStateException("Failed to load catalog item " + catalogItemId + " required for rebinding.");
         }
     }
 
@@ -1132,16 +1145,33 @@ public class RebindManagerImpl implements RebindManager {
         @SuppressWarnings("unchecked")
         private <T extends BrooklynObject> Class<? extends T> load(Class<T> bType, String jType, String catalogItemId, String contextSuchAsId) {
             checkNotNull(jType, "Type of %s (%s) must not be null", contextSuchAsId, bType.getSimpleName());
-            if (catalogItemId==null) {
+            if (catalogItemId != null) {
+                BrooklynClassLoadingContext loader = getLoadingContextFromCatalogItemId(catalogItemId, classLoader, rebindContext);
+                return loader.loadClass(jType, bType);
+            } else {
                 // we have previously used reflections; not sure if that's needed?
                 try {
                     return (Class<T>)reflections.loadClass(jType);
                 } catch (Exception e) {
                     LOG.warn("Unable to load "+jType+" using reflections; will try standard context");
                 }
+
+                if (BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) {
+                    //Try loading from whichever catalog bundle succeeds.
+                    BrooklynCatalog catalog = managementContext.getCatalog();
+                    for (CatalogItem<?, ?> item : catalog.getCatalogItems()) {
+                        BrooklynClassLoadingContext catalogLoader = CatalogUtils.newClassLoadingContext(managementContext, item);
+                        Maybe<Class<?>> catalogClass = catalogLoader.tryLoadClass(jType);
+                        if (catalogClass.isPresent()) {
+                            return (Class<? extends T>) catalogClass.get();
+                        }
+                    }
+                    throw new IllegalStateException("No catalogItemId specified and can't load class from either classpath of catalog items");
+                } else {
+                    throw new IllegalStateException("No catalogItemId specified and can't load class from classpath");
+                }
+
             }
-            BrooklynClassLoadingContext loader = getLoadingContextFromCatalogItemId(catalogItemId, classLoader, rebindContext);
-            return loader.loadClass(jType, bType);
         }
 
         /**

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
index 2c1e323..fdaca78 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
@@ -172,7 +172,10 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
         if (item==null || item.getCatalogItemId()==null) {
             return null;
         }
-        CatalogItem<?, ?> catalogItem = lookupContext.lookupCatalogItem(item.getCatalogItemId());
+        CatalogItem<?, ?> catalogItem = CatalogUtils.getCatalogItemOptionalVersion(lookupContext.lookupManagementContext(), item.getCatalogItemId());
+        if (catalogItem == null) {
+            throw new IllegalStateException("Catalog item " + item.getCatalogItemId() + " not found. Can't deserialize object " + objectId + " of type " + type);
+        }
         return ClassLoaderFromBrooklynClassLoadingContext.of(CatalogUtils.newClassLoadingContext(lookupContext.lookupManagementContext(), catalogItem));
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java b/core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java
index ea0399b..91b99d6 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java
@@ -31,6 +31,10 @@ import com.thoughtworks.xstream.converters.UnmarshallingContext;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 
+/**
+ *  Convert old-style rebind file formats to the latest version.
+ *  The code is needed only during transition to the new version, can be removed after a while.
+ */
 @Deprecated
 public class CatalogItemLibrariesConverter implements Converter {
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cf3a9eb0/core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java b/core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java
index 88c5f6d..0ce1b54 100644
--- a/core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java
+++ b/core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java
@@ -73,6 +73,15 @@ public class BrooklynFeatureEnablement {
      */
     public static final String FEATURE_RENAME_THREADS = "brooklyn.executionManager.renameThreads";
 
+    /**
+     * When rebinding to store created from a previous version the catalogItemId properties will be missing which
+     * results in errors when OSGi bundles are used. When enabled the code tries to infer the catalogItemId from
+     * <ul>
+     *   <li> parent entities
+     *   <li> catalog items matching the type that needs to be deserialized
+     *   <li> iterating through all catalog items and checking if they can provide the needed type
+     * </ul>
+     */
     public static final String FEATURE_INFER_CATALOG_ITEM_ON_REBIND = "brooklyn.backwardCompatibility.feature.inferCatalogItemOnRebind";
     
     private static final Map<String, Boolean> FEATURE_ENABLEMENTS = Maps.newLinkedHashMap();