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.s...@gmail.com> Authored: Thu Jun 5 21:56:28 2014 +0200 Committer: Aled Sage <aled.s...@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()); }
