brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rich...@apache.org
Subject [13/26] git commit: Rebind: simplify LookupContext
Date Tue, 10 Jun 2014 02:25:34 GMT
Rebind: simplify LookupContext

- Previously we needed to pass the expected type; now we construct
  a “memento manifest” that tells us the ids and types of entity,
  location, etc.
- Code was logging lots of incorrect warnings such as:
  “Entity with id MGc3cuT8 does not match type class io.cloudsoft.Foo”


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

Branch: refs/heads/master
Commit: d9570eecbbbca92f266d0157abf99768d7ceedaa
Parents: f25d28a
Author: Aled Sage <aled.sage@gmail.com>
Authored: Thu Jun 5 21:56:28 2014 +0200
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Fri Jun 6 16:31:36 2014 +0200

----------------------------------------------------------------------
 .../mementos/BrooklynMementoPersister.java      |  8 +--
 .../entity/rebind/RebindManagerImpl.java        | 16 ++---
 .../BrooklynMementoPersisterInMemory.java       | 75 +++++++++++++-------
 .../rebind/persister/XmlMementoSerializer.java  | 29 +++-----
 .../persister/XmlMementoSerializerTest.java     | 32 +++------
 5 files changed, 74 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d9570eec/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java b/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
index e4bcc36..1b49eda 100644
--- a/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
+++ b/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
@@ -22,10 +22,10 @@ import com.google.common.annotations.VisibleForTesting;
 public interface BrooklynMementoPersister {
 
     public static interface LookupContext {
-        Entity lookupEntity(Class<?> type, String id);
-        Location lookupLocation(Class<?> type, String id);
-        Policy lookupPolicy(Class<?> type, String id);
-        Enricher lookupEnricher(Class<?> type, String id);
+        Entity lookupEntity(String id);
+        Location lookupLocation(String id);
+        Policy lookupPolicy(String id);
+        Enricher lookupEnricher(String id);
     }
 
     BrooklynMementoManifest loadMementoManifest(RebindExceptionHandler exceptionHandler)
throws IOException;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d9570eec/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 f7a871c..ae38083 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -237,39 +237,31 @@ public class RebindManagerImpl implements RebindManager {
             final RebindContextImpl rebindContext = new RebindContextImpl(classLoader);
     
             LookupContext realLookupContext = new LookupContext() {
-                @Override public Entity lookupEntity(Class<?> type, String id) {
+                @Override public Entity lookupEntity(String id) {
                     Entity result = rebindContext.getEntity(id);
                     if (result == null) {
                         result = exceptionHandler.onDanglingEntityRef(id);
-                    } else if (type != null && !type.isInstance(result)) {
-                        LOG.warn("Entity with id "+id+" does not match type "+type+"; returning
"+result);
                     }
                     return result;
                 }
-                @Override public Location lookupLocation(Class<?> type, String id)
{
+                @Override public Location lookupLocation(String id) {
                     Location result = rebindContext.getLocation(id);
                     if (result == null) {
                         result = exceptionHandler.onDanglingLocationRef(id);
-                    } else if (type != null && !type.isInstance(result)) {
-                        LOG.warn("Location with id "+id+" does not match type "+type+"; returning
"+result);
                     }
                     return result;
                 }
-                @Override public Policy lookupPolicy(Class<?> type, String id) {
+                @Override public Policy lookupPolicy(String id) {
                     Policy result = rebindContext.getPolicy(id);
                     if (result == null) {
                         result = exceptionHandler.onDanglingPolicyRef(id);
-                    } else if (type != null && !type.isInstance(result)) {
-                        LOG.warn("Policy with id "+id+" does not match type "+type+"; returning
"+result);
                     }
                     return result;
                 }
-                @Override public Enricher lookupEnricher(Class<?> type, String id)
{
+                @Override public Enricher lookupEnricher(String id) {
                     Enricher result = rebindContext.getEnricher(id);
                     if (result == null) {
                         result = exceptionHandler.onDanglingEnricherRef(id);
-                    } else if (type != null && !type.isInstance(result)) {
-                        LOG.warn("Enricher with id "+id+" does not match type "+type+"; returning
"+result);
                     }
                     return result;
                 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d9570eec/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemory.java
b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemory.java
index f1fda50..7c1f4e6 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemory.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemory.java
@@ -17,15 +17,19 @@ import brooklyn.entity.rebind.RebindExceptionHandler;
 import brooklyn.entity.rebind.RebindExceptionHandlerImpl;
 import brooklyn.entity.rebind.RebindManager;
 import brooklyn.location.Location;
-import brooklyn.location.basic.LocationInternal;
 import brooklyn.mementos.BrooklynMemento;
+import brooklyn.mementos.BrooklynMementoManifest;
 import brooklyn.policy.Enricher;
 import brooklyn.policy.Policy;
 import brooklyn.util.collections.MutableList;
+import brooklyn.util.collections.MutableMap;
+import brooklyn.util.exceptions.Exceptions;
+import brooklyn.util.javalang.Reflections;
 import brooklyn.util.os.Os;
 import brooklyn.util.time.Duration;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
 import com.google.common.base.Throwables;
 import com.google.common.io.Files;
 
@@ -76,44 +80,61 @@ public class BrooklynMementoPersisterInMemory extends AbstractBrooklynMementoPer
         try {
             File tempDir = Files.createTempDir();
             try {
-                // TODO See RebindManagerImpl.rebind for dummyLookupContext; remove duplication
+                // TODO Duplicate code for LookupContext in RebindManager
                 BrooklynMementoPersisterToMultiFile persister = new BrooklynMementoPersisterToMultiFile(tempDir
, classLoader);
+                RebindExceptionHandler exceptionHandler = new RebindExceptionHandlerImpl(RebindManager.RebindFailureMode.FAIL_AT_END,
RebindManager.RebindFailureMode.FAIL_AT_END);
                 persister.checkpoint(memento);
+                final BrooklynMementoManifest manifest = persister.loadMementoManifest(exceptionHandler);
                 LookupContext dummyLookupContext = new LookupContext() {
-                    @Override public Entity lookupEntity(Class<?> type, String id)
{
-                        List<Class<?>> types = MutableList.<Class<?>>of(Entity.class,
EntityInternal.class, EntityProxy.class);
-                        if (type != null) types.add(type);
-                        return (Entity) newDummy(types);
+                    @Override public Entity lookupEntity(String id) {
+                        List<Class<?>> types = MutableList.<Class<?>>builder()
+                                .add(Entity.class, EntityInternal.class, EntityProxy.class)
+                                .add(loadClass(manifest.getEntityIdToType().get(id)))
+                                .build();
+                        return (Entity) java.lang.reflect.Proxy.newProxyInstance(
+                                classLoader,
+                                types.toArray(new Class<?>[types.size()]),
+                                new InvocationHandler() {
+                                    @Override public Object invoke(Object proxy, Method m,
Object[] args) throws Throwable {
+                                        return m.invoke(this, args);
+                                    }
+                                });
+                    }
+                    @Override public Location lookupLocation(String id) {
+                        Class<?> clazz = loadClass(manifest.getLocationIdToType().get(id));
+                        return (Location) invokeConstructor(clazz, new Object[0], new Object[]
{MutableMap.of()});
                     }
-                    @Override public Location lookupLocation(Class<?> type, String
id) {
-                        List<Class<?>> types = MutableList.<Class<?>>of(Location.class,
LocationInternal.class);
-                        if (type != null) types.add(type);
-                        return (Location) newDummy(types);
+                    @Override public Policy lookupPolicy(String id) {
+                        Class<?> clazz = loadClass(manifest.getPolicyIdToType().get(id));
+                        return (Policy) invokeConstructor(clazz, new Object[0], new Object[]
{MutableMap.of()});
                     }
-                    @Override public Policy lookupPolicy(Class<?> type, String id)
{
-                        List<Class<?>> types = MutableList.<Class<?>>of(Policy.class);
-                        if (type != null) types.add(type);
-                        return (Policy) newDummy(types);
+                    @Override public Enricher lookupEnricher(String id) {
+                        Class<?> clazz = loadClass(manifest.getEnricherIdToType().get(id));
+                        return (Enricher) invokeConstructor(clazz, new Object[0], new Object[]
{MutableMap.of()});
                     }
-                    @Override public Enricher lookupEnricher(Class<?> type, String
id) {
-                        List<Class<?>> types = MutableList.<Class<?>>of(Enricher.class);
-                        if (type != null) types.add(type);
-                        return (Enricher) newDummy(types);
+                    private Class<?> loadClass(String name) {
+                        try {
+                            return classLoader.loadClass(name);
+                        } catch (ClassNotFoundException e) {
+                            throw Exceptions.propagate(e);
+                        }
                     }
-                    private Object newDummy(List<Class<?>> types) {
-                        return java.lang.reflect.Proxy.newProxyInstance(
-                            classLoader,
-                            types.toArray(new Class<?>[types.size()]),
-                            new InvocationHandler() {
-                                @Override public Object invoke(Object proxy, Method m, Object[]
args) throws Throwable {
-                                    return m.invoke(this, args);
+                    private <T> T invokeConstructor(Class<T> clazz, Object[]...
possibleArgs) {
+                        for (Object[] args : possibleArgs) {
+                            try {
+                                Optional<T> v = Reflections.invokeConstructorWithArgs(clazz,
args, true);
+                                if (v.isPresent()) {
+                                    return v.get();
                                 }
-                            });
+                            } catch (Exception e) {
+                                throw Exceptions.propagate(e);
+                            }
+                        }
+                        throw new IllegalStateException("Cannot instantiate instance of type
"+clazz+"; expected constructor signature not found");
                     }
                 };
 
                 // Not actually reconstituting, because need to use a real lookupContext
to reconstitute all the entities
-                RebindExceptionHandler exceptionHandler = new RebindExceptionHandlerImpl(RebindManager.RebindFailureMode.FAIL_AT_END,
RebindManager.RebindFailureMode.FAIL_AT_END);
                 persister.loadMemento(dummyLookupContext, exceptionHandler);
             } finally {
                 Os.tryDeleteDirectory(tempDir.getAbsolutePath());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d9570eec/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java
b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java
index 02da444..ab33e8b 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java
@@ -152,21 +152,12 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
     public abstract class IdentifiableConverter<IT extends Identifiable> implements
SingleValueConverter {
         private final Class<IT> clazz;
         
-        /*
-         * Ugly hack so we know what type to deserialize the string as. Remember the last
call to canConvert!
-         * This is needed for RebindManager's two-phase approach, where in the first phase
we create a 
-         * dynamic proxy to represent the Entity/Location (can't return null as ImmutableList
etc won't accept
-         * null values).
-         */
-        private Class<?> toClazz;
-        
         IdentifiableConverter(Class<IT> clazz) {
             this.clazz = clazz;
         }
         @Override
         public boolean canConvert(@SuppressWarnings("rawtypes") Class type) {
             boolean result = clazz.isAssignableFrom(type);
-            toClazz = (result) ? type : null;
             return result;
         }
 
@@ -180,11 +171,11 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
                 LOG.warn("Cannot unmarshall from persisted xml {} {}; no lookup context supplied!",
clazz.getSimpleName(), str);
                 return null;
             } else {
-                return lookup(toClazz, str);
+                return lookup(str);
             }
         }
         
-        protected abstract IT lookup(Class<?> type, String id);
+        protected abstract IT lookup(String id);
     }
 
     public class LocationConverter extends IdentifiableConverter<Location> {
@@ -192,8 +183,8 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
             super(Location.class);
         }
         @Override
-        protected Location lookup(Class<?> type, String id) {
-            return lookupContext.lookupLocation(type, id);
+        protected Location lookup(String id) {
+            return lookupContext.lookupLocation(id);
         }
     }
 
@@ -202,8 +193,8 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
             super(Policy.class);
         }
         @Override
-        protected Policy lookup(Class<?> type, String id) {
-            return lookupContext.lookupPolicy(type, id);
+        protected Policy lookup(String id) {
+            return lookupContext.lookupPolicy(id);
         }
     }
 
@@ -212,8 +203,8 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
             super(Enricher.class);
         }
         @Override
-        protected Enricher lookup(Class<?> type, String id) {
-            return lookupContext.lookupEnricher(type, id);
+        protected Enricher lookup(String id) {
+            return lookupContext.lookupEnricher(id);
         }
     }
     
@@ -222,8 +213,8 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T>
implements Memento
             super(Entity.class);
         }
         @Override
-        protected Entity lookup(Class<?> type, String id) {
-            return lookupContext.lookupEntity(type, id);
+        protected Entity lookup(String id) {
+            return lookupContext.lookupEntity(id);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d9570eec/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
b/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
index 903fe4e..c995ddf 100644
--- a/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
@@ -210,43 +210,27 @@ public class XmlMementoSerializerTest {
             this.policies = ImmutableMap.copyOf(policies);
             this.enrichers = ImmutableMap.copyOf(enrichers);
         }
-        @Override public Entity lookupEntity(Class<?> type, String id) {
+        @Override public Entity lookupEntity(String id) {
             if (entities.containsKey(id)) {
-                Entity result = entities.get(id);
-                if (type != null && !type.isInstance(result)) {
-                    throw new IllegalStateException("Entity with id "+id+" does not match
type "+type+"; got "+result);
-                }
-                return result;
+                return entities.get(id);
             }
             throw new NoSuchElementException("no entity with id "+id+"; contenders are "+entities.keySet());
         }
-        @Override public Location lookupLocation(Class<?> type, String id) {
+        @Override public Location lookupLocation(String id) {
             if (locations.containsKey(id)) {
-                Location result = locations.get(id);
-                if (type != null && !type.isInstance(result)) {
-                    throw new IllegalStateException("Location with id "+id+" does not match
type "+type+"; got "+result);
-                }
-                return result;
+                return locations.get(id);
             }
             throw new NoSuchElementException("no location with id "+id+"; contenders are
"+locations.keySet());
         }
-        @Override public Policy lookupPolicy(Class<?> type, String id) {
+        @Override public Policy lookupPolicy(String id) {
             if (policies.containsKey(id)) {
-                Policy result = policies.get(id);
-                if (type != null && !type.isInstance(result)) {
-                    throw new IllegalStateException("Policy with id "+id+" does not match
type "+type+"; got "+result);
-                }
-                return result;
+                return policies.get(id);
             }
             throw new NoSuchElementException("no policy with id "+id+"; contenders are "+policies.keySet());
         }
-        @Override public Enricher lookupEnricher(Class<?> type, String id) {
+        @Override public Enricher lookupEnricher(String id) {
             if (enrichers.containsKey(id)) {
-                Enricher result = enrichers.get(id);
-                if (type != null && !type.isInstance(result)) {
-                    throw new IllegalStateException("Enricher with id "+id+" does not match
type "+type+"; got "+result);
-                }
-                return result;
+                return enrichers.get(id);
             }
             throw new NoSuchElementException("no enricher with id "+id+"; contenders are
"+enrichers.keySet());
         }


Mime
View raw message