brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rich...@apache.org
Subject [2/3] git commit: undo the MapConverter retry-with-copy, and other items as discussed in code review
Date Tue, 10 Jun 2014 02:35:17 GMT
undo the MapConverter retry-with-copy, and other items as discussed in code review


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

Branch: refs/heads/master
Commit: fcf345e763d2dbf1f706bb33831c94fec071ab4c
Parents: e0591f0
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Mon Jun 9 03:32:51 2014 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Mon Jun 9 19:25:45 2014 -0700

----------------------------------------------------------------------
 .../java/brooklyn/entity/basic/Entities.java    | 65 ++++++++++++--------
 .../brooklyn/util/xstream/MapConverter.java     | 25 +++++---
 .../brooklyn/entity/basic/EntitiesTest.java     | 12 +++-
 3 files changed, 65 insertions(+), 37 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fcf345e7/core/src/main/java/brooklyn/entity/basic/Entities.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/Entities.java b/core/src/main/java/brooklyn/entity/basic/Entities.java
index ff523c4..f47dea3 100644
--- a/core/src/main/java/brooklyn/entity/basic/Entities.java
+++ b/core/src/main/java/brooklyn/entity/basic/Entities.java
@@ -71,6 +71,7 @@ import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -444,6 +445,9 @@ public class Entities {
         return result;
     }
 
+    /** true if the given descendant includes the given ancestor in its chain.
+     * does NOT count a node as its ancestor.
+     */
     public static boolean isAncestor(Entity descendant, Entity potentialAncestor) {
         Entity ancestor = descendant.getParent();
         while (ancestor != null) {
@@ -452,8 +456,10 @@ public class Entities {
         }
         return false;
     }
-
-    /** note, it is usually preferred to use isAncestor() and swap the order, it is a cheaper
method */
+    
+    /** checks whether the descendants of the given ancestor contains the given potentialDescendant.
+     * in this test, unlike in {@link #descendants(Entity)}, an entity is not counted as
a descendant.
+     * note, it is usually preferred to use isAncestor() and swap the order, it is a cheaper
method. */
     public static boolean isDescendant(Entity ancestor, Entity potentialDescendant) {
         Set<Entity> inspected = Sets.newLinkedHashSet();
         Stack<Entity> toinspect = new Stack<Entity>();
@@ -471,8 +477,39 @@ public class Entities {
 
         return false;
     }
+    
+    /** return all descendants of given entity matching the given predicate.
+     * see {@link EntityPredicates} for useful second arguments! */
+    public static Iterable<Entity> descendants(Entity root, Predicate<? super Entity>
matching, boolean includeSelf) {
+        Iterable<Entity> descs = Iterables.concat(Iterables.transform(root.getChildren(),
new Function<Entity,Iterable<Entity>>() {
+            @Override
+            public Iterable<Entity> apply(Entity input) {
+                return descendants(input);
+            }
+        }));
+        return Iterables.filter(Iterables.concat(descs, Collections.singleton(root)), matching);
+    }
+
+    /** as {@link #descendants(Entity, Predicate)}, for common case of including self */

+    public static Iterable<Entity> descendants(Entity root, Predicate<Entity>
matching) {
+        return descendants(root, matching, true);
+    }
 
     /**
+     * returns the entity, its children, and all its children, and so on. 
+     * as {@link #descendants(Entity, Predicate)}, for common case of matching everything
and including self. */ 
+    public static Iterable<Entity> descendants(Entity root) {
+        return descendants(root, Predicates.alwaysTrue(), true);
+    }
+
+    /** return all descendants of given entity of the given type, potentially including the
given root.
+     * as {@link #descendants(Entity, Predicate)}, for common case of {@link Predicates#instanceOf(Class)},

+     * including self, and returning the correct generics signature. */
+    public static <T extends Entity> Iterable<T> descendants(Entity root, Class<T>
ofType) {
+        return Iterables.filter(descendants(root), ofType);
+    }
+    
+    /**
      * @deprecated since 0.7.0; instead use {@link BrooklynShutdownHooks#invokeStopOnShutdown(Entity)}
      */
     @Deprecated
@@ -813,28 +850,4 @@ public class Entities {
         return t;
     }
     
-    /** returns the entity, its children, and all its children, and so on;
-     * @param root entity whose descendants should be iterated
-     */
-    public static Iterable<Entity> descendants(Entity root) {
-        Iterable<Entity> descs = Iterables.concat(Iterables.transform(root.getChildren(),
new Function<Entity,Iterable<Entity>>() {
-            @Override
-            public Iterable<Entity> apply(Entity input) {
-                return descendants(input);
-            }
-        }));
-        return Iterables.concat(descs, Collections.singleton(root));
-    }
-
-    /** return all descendants of given entity matching the given predicate.
-     * see {@link EntityPredicates} for useful second arguments! */
-    public static Iterable<Entity> descendants(Entity root, Predicate<Entity>
matching) {
-        return Iterables.filter(descendants(root), matching);
-    }
-
-    /** return all descendants of given entity of the given type */
-    public static <T extends Entity> Iterable<T> descendants(Entity root, Class<T>
ofType) {
-        return Iterables.filter(descendants(root), ofType);
-    }
-    
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fcf345e7/core/src/main/java/brooklyn/util/xstream/MapConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/xstream/MapConverter.java b/core/src/main/java/brooklyn/util/xstream/MapConverter.java
index 837b7ed..5f07ebd 100644
--- a/core/src/main/java/brooklyn/util/xstream/MapConverter.java
+++ b/core/src/main/java/brooklyn/util/xstream/MapConverter.java
@@ -7,7 +7,6 @@ import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.ImmutableList;
 import com.thoughtworks.xstream.converters.MarshallingContext;
 import com.thoughtworks.xstream.converters.UnmarshallingContext;
 import com.thoughtworks.xstream.core.ReferencingMarshallingContext;
@@ -25,7 +24,7 @@ public class MapConverter extends com.thoughtworks.xstream.converters.collection
         super(mapper);
     }
 
-    @SuppressWarnings({ "rawtypes", "unchecked" })
+    @SuppressWarnings({ "rawtypes" })
     public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext
context) {
         Map map = (Map) source;
         try {
@@ -34,16 +33,22 @@ public class MapConverter extends com.thoughtworks.xstream.converters.collection
                 marshalEntry(writer, context, entry);
             }
         } catch (ConcurrentModificationException e) {
-            log.warn("Map "
+            log.debug("Map "
                 // seems there is no non-deprecated way to get the path...
                 + (context instanceof ReferencingMarshallingContext ? "at "+((ReferencingMarshallingContext)context).currentPath()
: "")
-                + "["+source+"] modified while serializing; trying alternate technique");
-            ImmutableList entries = ImmutableList.copyOf(map.entrySet());
-            // FIXME i think this will probably cause bogus output as it will have written
some non-terminated things ... but we can try!
-            for (Iterator iterator = entries.iterator(); iterator.hasNext();) {
-                Map.Entry entry = (Map.Entry) iterator.next();
-                marshalEntry(writer, context, entry);                
-            }
+                + "["+source+"] modified while serializing; will fail, and retry may be attempted");
+            throw e;
+            // would be nice to attempt to re-serialize being slightly more defensive, as
code below;
+            // but the code above may have written partial data so that is dangerous, we
could have corrupted output. 
+            // if we could mark and restore in the output stream then we could do this below
(but we don't have that in our stream),
+            // or we could try this copying code in the first instance (but that's slow);
+            // error is rare most of the time (e.g. attribute being updated) so we bail on
this whole attempt
+            // and simply try serializing the map-owner (e.g. an entity) again.
+//            ImmutableList entries = ImmutableList.copyOf(map.entrySet());
+//            for (Iterator iterator = entries.iterator(); iterator.hasNext();) {
+//                Map.Entry entry = (Map.Entry) iterator.next();
+//                marshalEntry(writer, context, entry);                
+//            }
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fcf345e7/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java b/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
index 8685f0c..00413ab 100644
--- a/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
+++ b/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
@@ -6,17 +6,21 @@ import static org.testng.Assert.assertTrue;
 
 import java.util.concurrent.atomic.AtomicReference;
 
+import junit.framework.Assert;
+
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 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.Iterables;
 
 public class EntitiesTest {
 
@@ -28,8 +32,8 @@ public class EntitiesTest {
     
     @BeforeMethod(alwaysRun=true)
     public void setUp() throws Exception {
-        loc = new SimulatedLocation();
         app = ApplicationBuilder.newManagedApp(TestApplication.class);
+        loc = app.getManagementContext().getLocationManager().createLocation(LocationSpec.create(SimulatedLocation.class));
         entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
         app.start(ImmutableList.of(loc));
     }
@@ -40,6 +44,12 @@ public class EntitiesTest {
     }
     
     @Test
+    public void testDescendants() throws Exception {
+        Assert.assertEquals(Iterables.size(Entities.descendants(app)), 2);
+        Assert.assertEquals(Iterables.getOnlyElement(Entities.descendants(app, TestEntity.class)),
entity);
+    }
+    
+    @Test
     public void testAttributeSupplier() throws Exception {
         entity.setAttribute(TestEntity.NAME, "myname");
         assertEquals(Entities.attributeSupplier(entity, TestEntity.NAME).get(), "myname");


Mime
View raw message