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/07/03 11:46:53 UTC

[1/6] git commit: Group notified on member unmanaged

Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master abf21bdc4 -> f28d9cc6b


Group notified on member unmanaged

- and member notified on group unmanaged.
- done automatically, without need for registering listeners.
- will be generalised to “relationships” at some point.


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

Branch: refs/heads/master
Commit: e2207db602db4ac9e9d656d765526812d2b9ebe8
Parents: 40c69e5
Author: Aled Sage <al...@gmail.com>
Authored: Mon Jun 9 06:42:19 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Jun 26 14:17:23 2014 +0100

----------------------------------------------------------------------
 api/src/main/java/brooklyn/entity/Entity.java   |  15 ++-
 .../brooklyn/entity/basic/AbstractEntity.java   |   9 +-
 .../entity/basic/AbstractGroupImpl.java         |   1 +
 .../management/internal/LocalEntityManager.java |  11 ++
 .../java/brooklyn/entity/group/GroupTest.java   | 105 ++++++++++++++++++
 .../brooklyn/entity/rebind/RebindGroupTest.java | 106 +++++++++++++++++++
 6 files changed, 243 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/api/src/main/java/brooklyn/entity/Entity.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/entity/Entity.java b/api/src/main/java/brooklyn/entity/Entity.java
index 3e1a77f..aec44a4 100644
--- a/api/src/main/java/brooklyn/entity/Entity.java
+++ b/api/src/main/java/brooklyn/entity/Entity.java
@@ -135,11 +135,24 @@ public interface Entity extends Identifiable {
     Collection<Group> getGroups();
 
     /**
-     * Add this entity as a member of the given {@link Group}.
+     * Add this entity as a member of the given {@link Group}. Called by framework.
+     * <p>
+     * Users should call {@link Group#addMember(Entity)} instead; this method will then 
+     * automatically be called. However, the reverse is not true (calling this method will 
+     * not tell the group; this behaviour may change in a future release!)
      */
     void addGroup(Group group);
 
     /**
+     * Removes this entity as a member of the given {@link Group}. Called by framework.
+     * <p>
+     * Users should call {@link Group#removeMember(Entity)} instead; this method will then 
+     * automatically be called. However, the reverse is not true (calling this method will 
+     * not tell the group; this behaviour may change in a future release!)
+     */
+    void removeGroup(Group group);
+
+    /**
      * Return all the {@link Location}s this entity is deployed to.
      */
     Collection<Location> getLocations();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
index bf05f86..633bb49 100644
--- a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
+++ b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
@@ -641,9 +641,6 @@ public abstract class AbstractEntity implements EntityLocal, EntityInternal {
         return changed;
     }
 
-    /**
-     * Adds this as a member of the given group, registers with application if necessary
-     */
     @Override
     public void addGroup(Group e) {
         groups.add(e);
@@ -651,6 +648,12 @@ public abstract class AbstractEntity implements EntityLocal, EntityInternal {
     }
 
     @Override
+    public void removeGroup(Group e) {
+        groups.remove(e);
+        getApplication();
+    }
+
+    @Override
     public Entity getParent() {
         return parent.get();
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/core/src/main/java/brooklyn/entity/basic/AbstractGroupImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/AbstractGroupImpl.java b/core/src/main/java/brooklyn/entity/basic/AbstractGroupImpl.java
index 78ab0ba..ed9ec9e 100644
--- a/core/src/main/java/brooklyn/entity/basic/AbstractGroupImpl.java
+++ b/core/src/main/java/brooklyn/entity/basic/AbstractGroupImpl.java
@@ -115,6 +115,7 @@ public abstract class AbstractGroupImpl extends AbstractEntity implements Abstra
     @Override
     public boolean removeMember(final Entity member) {
         synchronized (members) {
+            member.removeGroup((Group)getProxyIfAvailable());
             boolean changed = (member != null && members.remove(member));
             if (changed) {
                 log.debug("Group {} lost member {}", this, member);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
index c806645..5df0841 100644
--- a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
+++ b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
@@ -15,6 +15,7 @@ import org.slf4j.LoggerFactory;
 
 import brooklyn.entity.Application;
 import brooklyn.entity.Entity;
+import brooklyn.entity.Group;
 import brooklyn.entity.basic.AbstractEntity;
 import brooklyn.entity.basic.EntityInternal;
 import brooklyn.entity.basic.EntityPredicates;
@@ -382,8 +383,18 @@ public class LocalEntityManager implements EntityManagerInternal {
      */
     private synchronized boolean unmanageNonRecursive(Entity e) {
         Entity proxyE = toProxyEntityIfAvailable(e);
+        Collection<Group> groups = e.getGroups();
         
         e.clearParent();
+        for (Group group : groups) {
+            group.removeMember(e);
+        }
+        if (e instanceof Group) {
+            Collection<Entity> members = ((Group)e).getMembers();
+            for (Entity member : members) {
+                member.removeGroup((Group)e);
+            }
+        }
         if (e instanceof Application) {
             applications.remove(proxyE);
             applicationIds.remove(e.getId());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/core/src/test/java/brooklyn/entity/group/GroupTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/group/GroupTest.java b/core/src/test/java/brooklyn/entity/group/GroupTest.java
new file mode 100644
index 0000000..3f876b6
--- /dev/null
+++ b/core/src/test/java/brooklyn/entity/group/GroupTest.java
@@ -0,0 +1,105 @@
+package brooklyn.entity.group;
+
+import static org.testng.Assert.assertEquals;
+
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Entity;
+import brooklyn.entity.basic.ApplicationBuilder;
+import brooklyn.entity.basic.BasicGroup;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.SimulatedLocation;
+import brooklyn.test.Asserts;
+import brooklyn.test.entity.TestApplication;
+import brooklyn.test.entity.TestEntity;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class GroupTest {
+
+    private static final int TIMEOUT_MS = 2000;
+
+    private TestApplication app;
+    private BasicGroup group;
+    private TestEntity entity1;
+    private TestEntity entity2;
+    
+    SimulatedLocation loc;
+
+
+    @BeforeMethod
+    public void setUp() {
+        app = ApplicationBuilder.newManagedApp(TestApplication.class);
+        loc = app.getManagementContext().getLocationManager().createLocation(LocationSpec.create(SimulatedLocation.class));
+        group = app.createAndManageChild(EntitySpec.create(BasicGroup.class));
+        entity1 = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        entity2 = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+    }
+
+    @AfterMethod(alwaysRun = true)
+    public void tearDown(){
+        if (app != null) Entities.destroyAll(app.getManagementContext());
+    }
+
+    @Test
+    public void testAddRemoveMembers() throws Exception {
+        group.addMember(entity1);
+        assertGroupMembers(entity1);
+        
+        group.addMember(entity2);
+        assertGroupMembers(entity1, entity2);
+        
+        group.removeMember(entity2);
+        assertGroupMembers(entity1);
+        
+        group.removeMember(entity1);
+        assertGroupMembers(new Entity[0]);
+    }
+    
+    @Test
+    public void testEntityGetGroups() throws Exception {
+        group.addMember(entity1);
+        Asserts.assertEqualsIgnoringOrder(entity1.getGroups(), ImmutableSet.of(group));
+        
+        group.removeMember(entity1);
+        Asserts.assertEqualsIgnoringOrder(entity1.getGroups(), ImmutableSet.of());
+   }
+    
+    @Test
+    public void testUnmanagedMemberAutomaticallyRemoved() throws Exception {
+        group.addMember(entity1);
+        Entities.unmanage(entity1);
+        assertGroupMembers(new Entity[0]);
+    }
+    
+    @Test
+    public void testUnmanagedGroupAutomaticallyRemovedMembers() throws Exception {
+        group.addMember(entity1);
+        Entities.unmanage(group);
+        Asserts.assertEqualsIgnoringOrder(entity1.getGroups(), ImmutableSet.of());
+    }
+    
+    @Test
+    public void testAddingUnmanagedMemberDoesNotFailBadly() throws Exception {
+        Entities.unmanage(entity1);
+        group.addMember(entity1);
+        Entities.unmanage(group);
+    }
+    
+    @Test
+    public void testAddingUnmanagedGroupDoesNotFailBadly() throws Exception {
+        Entities.unmanage(group);
+        entity1.addGroup(group);
+        Entities.unmanage(entity1);
+    }
+    
+    private void assertGroupMembers(Entity... expectedMembers) {
+        Asserts.assertEqualsIgnoringOrder(group.getMembers(), ImmutableList.copyOf(expectedMembers));
+        assertEquals(group.getAttribute(BasicGroup.GROUP_SIZE), (Integer)expectedMembers.length);
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e2207db6/core/src/test/java/brooklyn/entity/rebind/RebindGroupTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/RebindGroupTest.java b/core/src/test/java/brooklyn/entity/rebind/RebindGroupTest.java
new file mode 100644
index 0000000..e907ae4
--- /dev/null
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindGroupTest.java
@@ -0,0 +1,106 @@
+package brooklyn.entity.rebind;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.List;
+
+import org.testng.annotations.Test;
+
+import brooklyn.entity.Entity;
+import brooklyn.entity.basic.AbstractGroup;
+import brooklyn.entity.basic.AbstractGroupImpl;
+import brooklyn.entity.basic.BasicGroup;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.entity.proxying.ImplementedBy;
+import brooklyn.test.Asserts;
+import brooklyn.test.entity.TestEntity;
+
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+
+public class RebindGroupTest extends RebindTestFixtureWithApp {
+
+    @Test
+    public void testRestoresGroupAndMembers() throws Exception {
+        BasicGroup origGroup = origApp.createAndManageChild(EntitySpec.create(BasicGroup.class));
+        TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class));
+        origGroup.addMember(origEntity);
+        
+        newApp = rebind();
+        BasicGroup newGroup = (BasicGroup) Iterables.find(origApp.getChildren(), Predicates.instanceOf(BasicGroup.class));
+        TestEntity newEntity = (TestEntity) Iterables.find(origApp.getChildren(), Predicates.instanceOf(TestEntity.class));
+        Asserts.assertEqualsIgnoringOrder(newGroup.getMembers(), ImmutableSet.of(newEntity));
+        assertEquals(newGroup.getAttribute(BasicGroup.GROUP_SIZE), (Integer)1);
+        assertEquals(newGroup.getAttribute(BasicGroup.GROUP_MEMBERS), ImmutableSet.of(newEntity));
+    }
+    
+    // FIXME Fails because attribute AbstractGroup.GROUP_MEMBERS is an ImmutableSet which cannot have null values.
+    // However, deserializing the origEntity was a dangling reference which was returned as null.
+    // Therefore deserializing the group fails.
+    @Test(enabled=false)
+    public void testRestoresGroupWithUnmanagedMember() throws Exception {
+        BasicGroup origGroup = origApp.createAndManageChild(EntitySpec.create(BasicGroup.class));
+        TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class));
+        Entities.unmanage(origEntity);
+        origGroup.addMember(origEntity);
+        
+        newApp = rebind();
+        BasicGroup newGroup = (BasicGroup) Iterables.find(origApp.getChildren(), Predicates.instanceOf(BasicGroup.class));
+        Asserts.assertEqualsIgnoringOrder(newGroup.getMembers(), ImmutableSet.of());
+        assertEquals(newGroup.getAttribute(BasicGroup.GROUP_SIZE), (Integer)0);
+        assertEquals(newGroup.getAttribute(BasicGroup.GROUP_MEMBERS), ImmutableSet.of());
+    }
+    
+    // Don't want rebind to call addMember, because some sub-classes override this; calling it kicks off that
+    // behaviour again which is (often?) not what is wanted.
+    // FIXME fails because BasicEntityRebindSupport.addMembers calls group.addMember
+    @Test(enabled=false)
+    public void testAddMemberNotCalledOnRebind() throws Exception {
+        GroupRecordingCalls origGroup = origApp.createAndManageChild(EntitySpec.create(GroupRecordingCalls.class));
+        TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class));
+        origGroup.addMember(origEntity);
+        origGroup.clearCalls();
+        
+        newApp = rebind();
+        GroupRecordingCalls newGroup = (GroupRecordingCalls) Iterables.find(newApp.getChildren(), Predicates.instanceOf(GroupRecordingCalls.class));
+        TestEntity newEntity = (TestEntity) Iterables.find(origApp.getChildren(), Predicates.instanceOf(TestEntity.class));
+        assertEquals(newGroup.getCalls(), ImmutableList.of());
+        Asserts.assertEqualsIgnoringOrder(newGroup.getMembers(), ImmutableSet.of(newEntity));
+    }
+    
+    @ImplementedBy(GroupRecordingCallsImpl.class)
+    public static interface GroupRecordingCalls extends AbstractGroup {
+        List<String> getCalls();
+        void clearCalls();
+    }
+    
+    public static class GroupRecordingCallsImpl extends AbstractGroupImpl implements GroupRecordingCalls {
+        private final List<String> calls = Lists.newCopyOnWriteArrayList();
+        
+        @Override
+        public List<String> getCalls() {
+            return calls;
+        }
+        
+        @Override
+        public void clearCalls() {
+            calls.clear();
+        }
+        
+        @Override
+        public boolean addMember(Entity member) {
+            calls.add("addMember");
+            return super.addMember(member);
+        }
+        
+        @Override
+        public boolean removeMember(Entity member) {
+            calls.add("removeMember");
+            return super.removeMember(member);
+        }
+    }
+}


[6/6] git commit: This closes #24.

Posted by he...@apache.org.
This closes #24.


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

Branch: refs/heads/master
Commit: f28d9cc6b9fd21fe3e05dd04553f16d562bf419c
Parents: abf21bd 1a4aa4b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Jul 3 10:46:32 2014 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Jul 3 10:46:32 2014 +0100

----------------------------------------------------------------------
 api/src/main/java/brooklyn/entity/Entity.java   |  15 ++-
 .../brooklyn/entity/basic/AbstractEntity.java   |   9 +-
 .../entity/basic/AbstractGroupImpl.java         |   1 +
 .../brooklyn/entity/basic/DynamicGroup.java     |   5 +
 .../brooklyn/entity/effector/Effectors.java     |   8 +-
 .../entity/group/QuarantineGroupImpl.java       |  15 ++-
 .../management/internal/LocalEntityManager.java |  11 ++
 .../util/xstream/ImmutableListConverter.java    |  14 +--
 .../util/xstream/ImmutableMapConverter.java     |  38 +++++++
 .../util/xstream/ImmutableSetConverter.java     |  36 +++++++
 .../brooklyn/util/xstream/XmlSerializer.java    |   2 +
 .../brooklyn/entity/basic/DynamicGroupTest.java |  20 +++-
 .../java/brooklyn/entity/group/GroupTest.java   | 105 ++++++++++++++++++
 .../brooklyn/entity/rebind/RebindGroupTest.java | 106 +++++++++++++++++++
 .../persister/XmlMementoSerializerTest.java     |  70 ++++++++++--
 15 files changed, 426 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/f28d9cc6/api/src/main/java/brooklyn/entity/Entity.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/f28d9cc6/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/f28d9cc6/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java
----------------------------------------------------------------------


[5/6] git commit: Deserialize: discard dangling ref in ImmutableSet

Posted by he...@apache.org.
Deserialize: discard dangling ref in ImmutableSet

- If didn’t then it would fail to deserialise, which would cause 
  entire file to fail to deserialise.
  The dangling ref will have been logged plus handled by the 
  RebindExceptionHandler, so mostly harmless ignoring it here.


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

Branch: refs/heads/master
Commit: e518690e53810fd460c47b57088a2a7ebba9f628
Parents: 258bd7d
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jun 24 20:41:13 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Jun 26 15:48:43 2014 +0100

----------------------------------------------------------------------
 .../util/xstream/ImmutableListConverter.java    | 14 ++--
 .../util/xstream/ImmutableMapConverter.java     | 38 +++++++++++
 .../util/xstream/ImmutableSetConverter.java     | 36 ++++++++++
 .../brooklyn/util/xstream/XmlSerializer.java    |  2 +
 .../persister/XmlMementoSerializerTest.java     | 70 +++++++++++++++++---
 5 files changed, 144 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e518690e/core/src/main/java/brooklyn/util/xstream/ImmutableListConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/xstream/ImmutableListConverter.java b/core/src/main/java/brooklyn/util/xstream/ImmutableListConverter.java
index fdd7ee8..e0e42ae 100644
--- a/core/src/main/java/brooklyn/util/xstream/ImmutableListConverter.java
+++ b/core/src/main/java/brooklyn/util/xstream/ImmutableListConverter.java
@@ -1,9 +1,11 @@
 package brooklyn.util.xstream;
 
-import java.util.ArrayList;
 import java.util.Collection;
 
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.thoughtworks.xstream.converters.UnmarshallingContext;
 import com.thoughtworks.xstream.converters.collections.CollectionConverter;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
@@ -23,12 +25,12 @@ public class ImmutableListConverter extends CollectionConverter {
     // marshalling is the same
     // so is unmarshalling the entries
 
-    // only difference is creating the overarching collection, which we do after the fact
-    // (optimizing format on disk as opposed to in-memory)
+    // only differences are creating the overarching collection, which we do after the fact
+    // (optimizing format on disk as opposed to in-memory), and we discard null values 
+    // to avoid failing entirely.
     public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
-        Collection collection = new ArrayList();
+        Collection<?> collection = Lists.newArrayList();
         populateCollection(reader, context, collection);
-        return ImmutableList.copyOf(collection);
+        return ImmutableList.copyOf(Iterables.filter(collection, Predicates.notNull()));
     }
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e518690e/core/src/main/java/brooklyn/util/xstream/ImmutableMapConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/xstream/ImmutableMapConverter.java b/core/src/main/java/brooklyn/util/xstream/ImmutableMapConverter.java
new file mode 100644
index 0000000..27cb049
--- /dev/null
+++ b/core/src/main/java/brooklyn/util/xstream/ImmutableMapConverter.java
@@ -0,0 +1,38 @@
+package brooklyn.util.xstream;
+
+import java.util.Map;
+import java.util.Map.Entry;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.io.HierarchicalStreamReader;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+public class ImmutableMapConverter extends MapConverter {
+
+    public ImmutableMapConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(@SuppressWarnings("rawtypes") Class type) {
+        return ImmutableMap.class.isAssignableFrom(type);
+    }
+
+    // marshalling is the same
+    // so is unmarshalling the entries
+
+    // only differences are creating the overarching collection, which we do after the fact
+    // (optimizing format on disk as opposed to in-memory), and we discard null key/values 
+    // to avoid failing entirely.
+    public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
+        Map<?, ?> map = Maps.newLinkedHashMap();
+        populateMap(reader, context, map);
+        return ImmutableMap.copyOf(Maps.filterEntries(map, new Predicate<Map.Entry<?,?>>() {
+                @Override public boolean apply(Entry<?, ?> input) {
+                    return input != null && input.getKey() != null && input.getValue() != null;
+                }}));
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e518690e/core/src/main/java/brooklyn/util/xstream/ImmutableSetConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/xstream/ImmutableSetConverter.java b/core/src/main/java/brooklyn/util/xstream/ImmutableSetConverter.java
new file mode 100644
index 0000000..757c643
--- /dev/null
+++ b/core/src/main/java/brooklyn/util/xstream/ImmutableSetConverter.java
@@ -0,0 +1,36 @@
+package brooklyn.util.xstream;
+
+import java.util.Collection;
+
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.converters.collections.CollectionConverter;
+import com.thoughtworks.xstream.io.HierarchicalStreamReader;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+public class ImmutableSetConverter extends CollectionConverter {
+
+    public ImmutableSetConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(@SuppressWarnings("rawtypes") Class type) {
+        return ImmutableSet.class.isAssignableFrom(type);
+    }
+
+    // marshalling is the same
+    // so is unmarshalling the entries
+
+    // only differences are creating the overarching collection, which we do after the fact
+    // (optimizing format on disk as opposed to in-memory), and we discard null values 
+    // to avoid failing entirely.
+    public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
+        Collection<?> collection = Lists.newArrayList();
+        populateCollection(reader, context, collection);
+        return ImmutableSet.copyOf(Iterables.filter(collection, Predicates.notNull()));
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e518690e/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java b/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java
index 1553ff4..334dc7e 100644
--- a/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java
+++ b/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java
@@ -46,6 +46,8 @@ public class XmlSerializer<T> {
         
         xstream.aliasType("ImmutableList", ImmutableList.class);
         xstream.registerConverter(new ImmutableListConverter(xstream.getMapper()));
+        xstream.registerConverter(new ImmutableSetConverter(xstream.getMapper()));
+        xstream.registerConverter(new ImmutableMapConverter(xstream.getMapper()));
 
         xstream.registerConverter(new EnumCaseForgivingConverter());
         xstream.registerConverter(new Inet4AddressConverter());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e518690e/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 22ccec4..c8fcfd6 100644
--- a/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java
@@ -18,6 +18,7 @@ import org.testng.annotations.Test;
 import brooklyn.entity.Entity;
 import brooklyn.entity.basic.ApplicationBuilder;
 import brooklyn.entity.basic.Entities;
+import brooklyn.entity.proxying.EntitySpec;
 import brooklyn.location.Location;
 import brooklyn.location.LocationSpec;
 import brooklyn.management.ManagementContext;
@@ -25,6 +26,7 @@ import brooklyn.mementos.BrooklynMementoPersister.LookupContext;
 import brooklyn.policy.Enricher;
 import brooklyn.policy.Policy;
 import brooklyn.test.entity.TestApplication;
+import brooklyn.test.entity.TestEntity;
 import brooklyn.util.collections.MutableList;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.collections.MutableSet;
@@ -109,7 +111,7 @@ public class XmlMementoSerializerTest {
         final TestApplication app = ApplicationBuilder.newManagedApp(TestApplication.class);
         ManagementContext managementContext = app.getManagementContext();
         try {
-            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of()));
+            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of(), true));
             assertSerializeAndDeserialize(app);
         } finally {
             Entities.destroyAll(managementContext);
@@ -123,7 +125,7 @@ public class XmlMementoSerializerTest {
         try {
             @SuppressWarnings("deprecation")
             final Location loc = managementContext.getLocationManager().createLocation(LocationSpec.create(brooklyn.location.basic.SimulatedLocation.class));
-            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.<Entity>of(), ImmutableList.of(loc), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of()));
+            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.<Entity>of(), ImmutableList.of(loc), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of(), true));
             assertSerializeAndDeserialize(loc);
         } finally {
             Entities.destroyAll(managementContext);
@@ -131,12 +133,38 @@ public class XmlMementoSerializerTest {
     }
 
     @Test
+    public void testImmutableCollectionsWithDanglingEntityRef() throws Exception {
+        // If there's a dangling entity in an ImmutableList etc, then discard it entirely.
+        // If we try to insert null then it fails, breaking the deserialization of that entire file.
+        
+        final TestApplication app = ApplicationBuilder.newManagedApp(TestApplication.class);
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        ManagementContext managementContext = app.getManagementContext();
+        try {
+            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of(), false));
+            
+            List<?> resultList = serializeAndDeserialize(ImmutableList.of(app, entity));
+            assertEquals(resultList, ImmutableList.of(app));
+            
+            Set<?> resultSet = serializeAndDeserialize(ImmutableSet.of(app, entity));
+            assertEquals(resultSet, ImmutableSet.of(app));
+            
+            Map<?, ?> resultMap = serializeAndDeserialize(ImmutableMap.of(app, "appval", "appkey", app, entity, "entityval", "entityKey", entity));
+            assertEquals(resultMap, ImmutableMap.of(app, "appval", "appkey", app));
+
+        } finally {
+            Entities.destroyAll(managementContext);
+        }
+    }
+
+
+    @Test
     public void testFieldReffingEntity() throws Exception {
         final TestApplication app = ApplicationBuilder.newManagedApp(TestApplication.class);
         ReffingEntity reffer = new ReffingEntity(app);
         ManagementContext managementContext = app.getManagementContext();
         try {
-            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of()));
+            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of(), true));
             ReffingEntity reffer2 = assertSerializeAndDeserialize(reffer);
             assertEquals(reffer2.entity, app);
         } finally {
@@ -150,7 +178,7 @@ public class XmlMementoSerializerTest {
         ReffingEntity reffer = new ReffingEntity((Object)app);
         ManagementContext managementContext = app.getManagementContext();
         try {
-            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of()));
+            serializer.setLookupContext(new LookupContextImpl(managementContext, ImmutableList.of(app), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), ImmutableList.<Enricher>of(), true));
             ReffingEntity reffer2 = assertSerializeAndDeserialize(reffer);
             assertEquals(reffer2.obj, app);
         } finally {
@@ -186,15 +214,23 @@ public class XmlMementoSerializerTest {
         return (T) deserialized;
     }
 
+    @SuppressWarnings("unchecked")
+    private <T> T serializeAndDeserialize(T obj) throws Exception {
+        String serializedForm = serializer.toString(obj);
+        System.out.println("serializedForm="+serializedForm);
+        return (T) serializer.fromString(serializedForm);
+    }
+
     static class LookupContextImpl implements LookupContext {
         private final ManagementContext mgmt;
         private final Map<String, Entity> entities;
         private final Map<String, Location> locations;
         private final Map<String, Policy> policies;
         private final Map<String, Enricher> enrichers;
+        private final boolean failOnDangling;
 
         LookupContextImpl(ManagementContext mgmt, Iterable<? extends Entity> entities, Iterable<? extends Location> locations,
-                Iterable<? extends Policy> policies, Iterable<? extends Enricher> enrichers) {
+                Iterable<? extends Policy> policies, Iterable<? extends Enricher> enrichers, boolean failOnDangling) {
             this.mgmt = mgmt;
             this.entities = Maps.newLinkedHashMap();
             this.locations = Maps.newLinkedHashMap();
@@ -204,14 +240,16 @@ public class XmlMementoSerializerTest {
             for (Location location : locations) this.locations.put(location.getId(), location);
             for (Policy policy : policies) this.policies.put(policy.getId(), policy);
             for (Enricher enricher : enrichers) this.enrichers.put(enricher.getId(), enricher);
+            this.failOnDangling = failOnDangling;
         }
         LookupContextImpl(ManagementContext mgmt, Map<String,? extends Entity> entities, Map<String,? extends Location> locations,
-                Map<String,? extends Policy> policies, Map<String,? extends Enricher> enrichers) {
+                Map<String,? extends Policy> policies, Map<String,? extends Enricher> enrichers, boolean failOnDangling) {
             this.mgmt = mgmt;
             this.entities = ImmutableMap.copyOf(entities);
             this.locations = ImmutableMap.copyOf(locations);
             this.policies = ImmutableMap.copyOf(policies);
             this.enrichers = ImmutableMap.copyOf(enrichers);
+            this.failOnDangling = failOnDangling;
         }
         @Override public ManagementContext lookupManagementContext() {
             return mgmt;
@@ -220,25 +258,37 @@ public class XmlMementoSerializerTest {
             if (entities.containsKey(id)) {
                 return entities.get(id);
             }
-            throw new NoSuchElementException("no entity with id "+id+"; contenders are "+entities.keySet());
+            if (failOnDangling) {
+                throw new NoSuchElementException("no entity with id "+id+"; contenders are "+entities.keySet());
+            }
+            return null;
         }
         @Override public Location lookupLocation(String id) {
             if (locations.containsKey(id)) {
                 return locations.get(id);
             }
-            throw new NoSuchElementException("no location with id "+id+"; contenders are "+locations.keySet());
+            if (failOnDangling) {
+                throw new NoSuchElementException("no location with id "+id+"; contenders are "+locations.keySet());
+            }
+            return null;
         }
         @Override public Policy lookupPolicy(String id) {
             if (policies.containsKey(id)) {
                 return policies.get(id);
             }
-            throw new NoSuchElementException("no policy with id "+id+"; contenders are "+policies.keySet());
+            if (failOnDangling) {
+                throw new NoSuchElementException("no policy with id "+id+"; contenders are "+policies.keySet());
+            }
+            return null;
         }
         @Override public Enricher lookupEnricher(String id) {
             if (enrichers.containsKey(id)) {
                 return enrichers.get(id);
             }
-            throw new NoSuchElementException("no enricher with id "+id+"; contenders are "+enrichers.keySet());
+            if (failOnDangling) {
+                throw new NoSuchElementException("no enricher with id "+id+"; contenders are "+enrichers.keySet());
+            }
+            return null;
         }
     };
 }


[3/6] git commit: Fix logging on Effectors.invoke

Posted by he...@apache.org.
Fix logging on Effectors.invoke


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

Branch: refs/heads/master
Commit: f620b977ba6f69c9ab72a7035345e268fc273b15
Parents: e518690
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jun 24 22:27:35 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Jun 26 15:48:43 2014 +0100

----------------------------------------------------------------------
 core/src/main/java/brooklyn/entity/effector/Effectors.java | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/f620b977/core/src/main/java/brooklyn/entity/effector/Effectors.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/effector/Effectors.java b/core/src/main/java/brooklyn/entity/effector/Effectors.java
index 46ae767..4389a47 100644
--- a/core/src/main/java/brooklyn/entity/effector/Effectors.java
+++ b/core/src/main/java/brooklyn/entity/effector/Effectors.java
@@ -117,17 +117,17 @@ public class Effectors {
                 " on entity " + entity+" "+
                 (eff2==eff ? "" : " (actually "+eff2+"/"+
                         (eff2 instanceof EffectorWithBody<?> ? ((EffectorWithBody<?>)eff2).getBody() : "bodyless")+")"));
-        if (eff2!=null) {
-            if (eff2!=eff) {
+        if (eff2 != null) {
+            if (eff2 != eff) {
                 if (eff2 instanceof EffectorWithBody) {
                     log.debug("Replacing invocation of {} on {} with {} which is the impl defined at that entity", new Object[] { eff, entity, eff2 });
                     return ((EffectorWithBody<T>)eff2).getBody().newTask(entity, eff2, ConfigBag.newInstance().putAll(parameters));
                 } else {
                     log.warn("Effector {} defined on {} has no body; invoking caller-supplied {} instead", new Object[] { eff2, entity, eff });
                 }
-            } else {
-                log.debug("Effector {} does not exist on {}; attempting to invoke anyway", new Object[] { eff, entity });
             }
+        } else {
+            log.debug("Effector {} does not exist on {}; attempting to invoke anyway", new Object[] { eff, entity });
         }
         
         if (eff instanceof EffectorWithBody) {


[2/6] git commit: Tidy QuarantineGroup.expungeMembers

Posted by he...@apache.org.
Tidy QuarantineGroup.expungeMembers


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

Branch: refs/heads/master
Commit: 1a4aa4b306d91637ff04b2e41f7d47ba1d755d37
Parents: f620b97
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jun 24 22:27:52 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Jun 26 15:48:43 2014 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/group/QuarantineGroupImpl.java   | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1a4aa4b3/core/src/main/java/brooklyn/entity/group/QuarantineGroupImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/group/QuarantineGroupImpl.java b/core/src/main/java/brooklyn/entity/group/QuarantineGroupImpl.java
index 028f06a..33d2e04 100644
--- a/core/src/main/java/brooklyn/entity/group/QuarantineGroupImpl.java
+++ b/core/src/main/java/brooklyn/entity/group/QuarantineGroupImpl.java
@@ -29,6 +29,7 @@ public class QuarantineGroupImpl extends AbstractGroupImpl implements Quarantine
     @Override
     public void expungeMembers(boolean stopFirst) {
         Set<Entity> members = ImmutableSet.copyOf(getMembers());
+        RuntimeException exception = null;
         if (stopFirst) {
             Map<Entity, Task<?>> tasks = Maps.newLinkedHashMap();
             for (Entity member : members) {
@@ -38,14 +39,20 @@ public class QuarantineGroupImpl extends AbstractGroupImpl implements Quarantine
                 }
             }
             DynamicTasks.queueIfPossible(Tasks.parallel("stopping "+tasks.size()+" member"+Strings.s(tasks.size())+" (parallel)", tasks.values())).orSubmitAsync(this);
-            waitForTasksOnExpungeMembers(tasks);
+            try {
+                waitForTasksOnExpungeMembers(tasks);
+            } catch (RuntimeException e) {
+                Exceptions.propagateIfFatal(e);
+                exception = e;
+                LOG.warn("Problem stopping members of quarantine group "+this+" (rethrowing after unmanaging members): "+e);
+            }
         }
         for (Entity member : members) {
+            removeMember(member);
             Entities.unmanage(member);
         }
-        Set<Entity> children = ImmutableSet.copyOf(getChildren());
-        for (Entity child : children) {
-            Entities.unmanage(child);
+        if (exception != null) {
+            throw exception;
         }
     }
     


[4/6] git commit: Deprecate DynamicGroup.stop

Posted by he...@apache.org.
Deprecate DynamicGroup.stop


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

Branch: refs/heads/master
Commit: 258bd7dacc8b5fbb026a8214d99c9d541cd6474d
Parents: e2207db
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jun 24 20:37:49 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Jun 26 15:48:43 2014 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/basic/DynamicGroup.java     |  5 +++++
 .../brooklyn/entity/basic/DynamicGroupTest.java | 20 +++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/258bd7da/core/src/main/java/brooklyn/entity/basic/DynamicGroup.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/DynamicGroup.java b/core/src/main/java/brooklyn/entity/basic/DynamicGroup.java
index 6448e88..0143acd 100644
--- a/core/src/main/java/brooklyn/entity/basic/DynamicGroup.java
+++ b/core/src/main/java/brooklyn/entity/basic/DynamicGroup.java
@@ -19,6 +19,7 @@ import groovy.lang.Closure;
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.Entity;
 import brooklyn.entity.proxying.ImplementedBy;
+import brooklyn.entity.trait.Startable;
 import brooklyn.event.AttributeSensor;
 import brooklyn.event.Sensor;
 import brooklyn.event.SensorEvent;
@@ -43,7 +44,11 @@ public interface DynamicGroup extends AbstractGroup {
      * <p>
      * Does not stop any of its members. De-activates the filter and unsubscribes to
      * entity-updates, so the membership of the group will not change.
+     * 
+     * @deprecated since 0.7; no longer supported (was only used in tests, and by classes that
+     *             also implemented {@link Startable#stop()}!)
      */
+    @Deprecated
     void stop();
 
     /** Rescans <em>all</em> entities to determine whether they match the filter. */

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/258bd7da/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java b/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java
index cd3ebdd..560fbd0 100644
--- a/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java
+++ b/core/src/test/java/brooklyn/entity/basic/DynamicGroupTest.java
@@ -188,6 +188,10 @@ public class DynamicGroupTest {
             }});
     }
     
+    // TODO Previously did Entities.unmanage(e1), but that now causes the group to be told
+    //      (to preserve referential integrity). Now doing Entities.unmanage(e3) instead.
+    //      Note that group.stop is now deprecated, so can delete this test when the method 
+    //      is deleted!
     @Test
     public void testStoppedGroupIgnoresComingAndGoingsOfEntities() throws Exception {
         Entity e3 = new AbstractEntity() {};
@@ -202,13 +206,27 @@ public class DynamicGroupTest {
                 assertEquals(ImmutableSet.copyOf(group.getMembers()), ImmutableSet.of(e1, e2));
             }});
                 
-        Entities.unmanage(e1);
+        Entities.unmanage(e3);
         Asserts.succeedsContinually(MutableMap.of("timeout", VERY_SHORT_WAIT_MS), new Runnable() {
             public void run() {
                 assertEqualsIgnoringOrder(ImmutableSet.copyOf(group.getMembers()), ImmutableSet.of(e1, e2));
             }});
     }
     
+    @Test
+    public void testUnmanagedGroupIgnoresComingAndGoingsOfEntities() {
+        Entity e3 = new AbstractEntity() {};
+        group.setEntityFilter(Predicates.instanceOf(TestEntity.class));
+        assertEqualsIgnoringOrder(group.getMembers(), ImmutableSet.of(e1, e2));
+        Entities.unmanage(group);
+        
+        e3.setParent(app);
+        Entities.manage(e3);
+        Asserts.succeedsContinually(MutableMap.of("timeout", VERY_SHORT_WAIT_MS), new Runnable() {
+            public void run() {
+                assertEqualsIgnoringOrder(ImmutableSet.copyOf(group.getMembers()), ImmutableSet.of(e1, e2));
+            }});
+    }
 
     // Motivated by strange behavior observed testing load-balancing policy, but this passed...
     //