You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/07/22 19:22:37 UTC

[1/2] brooklyn-server git commit: orphaned-state deletion: keep PortForwardManagers

Repository: brooklyn-server
Updated Branches:
  refs/heads/master 404294956 -> 29039cb8c


orphaned-state deletion: keep PortForwardManagers


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

Branch: refs/heads/master
Commit: 19abd2282c7df9261d3bcec908895a15b46ed3a1
Parents: b87c20b
Author: Aled Sage <al...@gmail.com>
Authored: Fri Jul 22 19:07:00 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Jul 22 20:21:53 2016 +0100

----------------------------------------------------------------------
 .../impl/DeleteOrphanedStateTransformer.java    | 371 ++++++++++++++-----
 .../AbstractCleanOrphanedStateTest.java         |  36 +-
 .../CleanOrphanedLocationsIntegrationTest.java  | 119 +-----
 .../launcher/CleanOrphanedLocationsTest.java    |  31 +-
 4 files changed, 342 insertions(+), 215 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19abd228/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java
index 9b76e28..e92750d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java
@@ -24,6 +24,8 @@ import java.util.Set;
 import javax.xml.xpath.XPathConstants;
 
 import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
+import org.apache.brooklyn.api.objs.BrooklynObjectType;
+import org.apache.brooklyn.core.location.access.PortForwardManager;
 import org.apache.brooklyn.core.mgmt.rebind.transformer.CompoundTransformer;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -34,7 +36,9 @@ import org.slf4j.LoggerFactory;
 import org.w3c.dom.Node;
 
 import com.google.common.annotations.Beta;
-import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 
 @Beta
@@ -64,12 +68,24 @@ public class DeleteOrphanedStateTransformer extends CompoundTransformer {
         super(builder);
     }
 
+    /**
+     * We inspect the state in two different ways to find the entities that will be deleted:
+     * belt-and-braces! Only if the state is not referenced by either of those two approaches
+     * will we delete it.
+     */
     @Override
     public BrooklynMementoRawData transform(BrooklynMementoRawData input) {
-        Map<String, String> locationsToKeep = findLocationsToKeep(input);
-        Map<String, String> enrichersToKeep = copyRetainingKeys(input.getEnrichers(), findAllReferencedEnrichers(input));
-        Map<String, String> policiesToKeep = copyRetainingKeys(input.getPolicies(), findAllReferencedPolicies(input));
-        Map<String, String> feedsToKeep = copyRetainingKeys(input.getFeeds(), findAllReferencedFeeds(input));
+        ReferencedState stateReferencedFromXpath = new ReachabilityXpathInspector().inspect(input);
+        ReferencedState stateToKeepFromGrep = new ReachabilityGrepInspector().inspect(input);
+        ReferencedState stateToKeepFromXpath = stateReferencedFromXpath.filterForExtant(input);
+        ReferencedState.warnOfDifferences(stateToKeepFromXpath, stateToKeepFromGrep);
+
+        ReferencedState stateToKeep = ReferencedState.union(stateToKeepFromXpath, stateToKeepFromGrep);
+        
+        Map<String, String> locationsToKeep = copyRetainingKeys(input.getLocations(), stateToKeep.locations);
+        Map<String, String> enrichersToKeep = copyRetainingKeys(input.getEnrichers(), stateToKeep.enrichers);
+        Map<String, String> policiesToKeep = copyRetainingKeys(input.getPolicies(), stateToKeep.policies);
+        Map<String, String> feedsToKeep = copyRetainingKeys(input.getFeeds(), stateToKeep.feeds);
 
         Set<String> locsToDelete = Sets.difference(input.getLocations().keySet(), locationsToKeep.keySet());
         Set<String> enrichersToDelete = Sets.difference(input.getEnrichers().keySet(), enrichersToKeep.keySet());
@@ -91,93 +107,219 @@ public class DeleteOrphanedStateTransformer extends CompoundTransformer {
                 .build();
     }
 
-    protected Set<String> findAllReferencedEnrichers(BrooklynMementoRawData input) {
-        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/enrichers/string");
-    }
-
-    protected Set<String> findAllReferencedPolicies(BrooklynMementoRawData input) {
-        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/policies/string");
-    }
-
-    protected Set<String> findAllReferencedFeeds(BrooklynMementoRawData input) {
-        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/feeds/string");
-    }
-
-    protected Set<String> findAllReferencedAdjuncts(Map<String, String> items, String xpath) {
-        Set<String> result = Sets.newLinkedHashSet();
-
-        for(Map.Entry<String, String> entry : items.entrySet()) {
-            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), xpath, XPathConstants.NODESET)));
+    /**
+     * Searches the state, based on using xpath to find references, starting at the roots of
+     * reachability. 
+     * 
+     * For locations, we find all locations referenced by entities, enrichers, policies or feeds 
+     * (looking at the xpath for the locations or location refs in the persisted state). We then 
+     * search through those locations for other locations reachable by them, and so on.
+     * 
+     * We also keep any location that is of type {@link PortForwardManager}.
+     */
+    protected static class ReachabilityXpathInspector {
+        public ReferencedState inspect(BrooklynMementoRawData input) {
+            return new ReferencedState()
+                    .locations(findAllReferencedLocations(input))
+                    .enrichers(findAllReferencedEnrichers(input))
+                    .policies(findAllReferencedPolicies(input))
+                    .feeds(findAllReferencedFeeds(input));
         }
 
-        return result;
-    }
-
-    @VisibleForTesting
-    public Map<String, String> findLocationsToKeep(BrooklynMementoRawData input) {
-        Set<String> allReferencedLocations = findAllReferencedLocations(input);
-        return copyRetainingKeys(input.getLocations(), allReferencedLocations);
-    }
-
-    @VisibleForTesting
-    public Set<String> findAllReferencedLocations(BrooklynMementoRawData input) {
-        Set<String> result = Sets.newLinkedHashSet();
-
-        result.addAll(searchLocationsToKeep(input.getEntities(), "/entity"));
-        result.addAll(searchLocationsToKeep(input.getPolicies(), "/policy"));
-        result.addAll(searchLocationsToKeep(input.getEnrichers(), "/enricher"));
-        result.addAll(searchLocationsToKeep(input.getFeeds(), "/feed"));
-        result.addAll(searchLocationsToKeepInLocations(input.getLocations(), result));
-
-        return result;
-    }
-
-    protected Set<String> searchLocationsToKeep(Map<String, String> items, String searchInTypePrefix) {
-        String locationsXpath = searchInTypePrefix+"/locations/string";
-        String locationProxyXpath = searchInTypePrefix+"//locationProxy";
-
-        Set<String> result = Sets.newLinkedHashSet();
-
-        for(Map.Entry<String, String> entry : items.entrySet()) {
-            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationsXpath, XPathConstants.NODESET)));
-            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationProxyXpath, XPathConstants.NODESET)));
+        protected Set<String> findAllReferencedEnrichers(BrooklynMementoRawData input) {
+            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/enrichers/string");
+        }
+    
+        protected Set<String> findAllReferencedPolicies(BrooklynMementoRawData input) {
+            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/policies/string");
+        }
+    
+        protected Set<String> findAllReferencedFeeds(BrooklynMementoRawData input) {
+            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/feeds/string");
+        }
+    
+        protected Set<String> findAllReferencedAdjuncts(Map<String, String> items, String xpath) {
+            Set<String> result = Sets.newLinkedHashSet();
+    
+            for(Map.Entry<String, String> entry : items.entrySet()) {
+                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), xpath, XPathConstants.NODESET)));
+            }
+    
+            return result;
+        }
+    
+        protected Set<String> findAllReferencedLocations(BrooklynMementoRawData input) {
+            Set<String> result = Sets.newLinkedHashSet();
+    
+            result.addAll(searchLocationsToKeep(input.getEntities(), "/entity"));
+            result.addAll(searchLocationsToKeep(input.getPolicies(), "/policy"));
+            result.addAll(searchLocationsToKeep(input.getEnrichers(), "/enricher"));
+            result.addAll(searchLocationsToKeep(input.getFeeds(), "/feed"));
+            result.addAll(searchLocationsToKeepInLocations(input.getLocations(), result));
+    
+            return result;
+        }
+    
+        protected Set<String> searchLocationsToKeep(Map<String, String> items, String searchInTypePrefix) {
+            String locationsXpath = searchInTypePrefix+"/locations/string";
+            String locationProxyXpath = searchInTypePrefix+"//locationProxy";
+    
+            Set<String> result = Sets.newLinkedHashSet();
+    
+            for(Map.Entry<String, String> entry : items.entrySet()) {
+                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationsXpath, XPathConstants.NODESET)));
+                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationProxyXpath, XPathConstants.NODESET)));
+            }
+    
+            return result;
+        }
+    
+        protected Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
+            Set<String> result = Sets.newLinkedHashSet();
+    
+            String prefix = "/location";
+            String locationTypeXpath = prefix+"/type";
+            String locationChildrenXpath = prefix+"/children/string";
+            String locationParentDirectTagXpath = prefix+"/parent";
+            String locationProxyXpath = prefix+"//locationProxy";
+    
+            Set<String> locsToInspect = MutableSet.copyOf(alreadyReferencedLocations);
+    
+            // Keep org.apache.brooklyn.core.location.access.PortForwardManager, even if not referenced.
+            // It is found and loaded by PortForwardManagerLocationResolver.
+            for (Map.Entry<String, String> entry : locations.entrySet()) {
+                String locId = entry.getKey();
+                if (!alreadyReferencedLocations.contains(locId)) {
+                    String locType = XmlUtil.xpath(entry.getValue(), locationTypeXpath);
+                    if (locType != null && locType.contains("PortForwardManager")) {
+                        result.add(locId);
+                        locsToInspect.add(locId);
+                    }
+                }
+            }
+    
+            while (locsToInspect.size() > 0) {
+                Set<String> referencedLocs = Sets.newLinkedHashSet();
+                for (String id : locsToInspect) {
+                    String xmlData = locations.get(id);
+                    if (xmlData != null) {
+                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationChildrenXpath, XPathConstants.NODESET)));
+                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationParentDirectTagXpath, XPathConstants.NODESET)));
+                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationProxyXpath, XPathConstants.NODESET)));
+                    }
+                }
+                Set<String> newlyDiscoveredLocs = MutableSet.<String>builder()
+                        .addAll(referencedLocs)
+                        .removeAll(alreadyReferencedLocations)
+                        .removeAll(result)
+                        .build();
+                result.addAll(newlyDiscoveredLocs);
+                locsToInspect = newlyDiscoveredLocs;
+            }
+    
+            return result;
         }
+        
+        protected Set<String> getAllNodesFromXpath(org.w3c.dom.NodeList nodeList) {
+            Set<String> result = Sets.newLinkedHashSet();
 
-        return result;
+            for (int i = 0; i < nodeList.getLength(); i++) {
+                Node nextNode = nodeList.item(i);
+                if (nextNode != null) {
+                    result.add(nextNode.getTextContent());
+                }
+            }
+            
+            return result;
+        }
     }
-
-    protected Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
-        Set<String> result = Sets.newLinkedHashSet();
-
-        String prefix = "/location";
-        String locationChildrenXpath = prefix+"/children/string";
-        String locationParentDirectTagXpath = prefix+"/parent";
-        String locationProxyXpath = prefix+"//locationProxy";
-
-        Set<String> locsToInspect = alreadyReferencedLocations;
+    
+    /**
+     * Searches the state, based on state.contains(id). We don't care about the structure or where
+     * in the state the id was mentioned.
+     * 
+     * The rules of reachability (in terms of the roots used etc) are the same as for 
+     * {@link ReachabilityXpathInspector}.
+     */
+    protected static class ReachabilityGrepInspector {
+        protected ReferencedState inspect(BrooklynMementoRawData input) {
+            Set<String> locations = Sets.newLinkedHashSet();
+            Set<String> feeds = Sets.newLinkedHashSet();
+            Set<String> enrichers = Sets.newLinkedHashSet();
+            Set<String> policies = Sets.newLinkedHashSet();
+
+            for (String id : input.getEnrichers().keySet()) {
+                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
+                    enrichers.add(id);
+                }
+            }
+            for (String id : input.getPolicies().keySet()) {
+                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
+                    policies.add(id);
+                }
+            }
+            for (String id : input.getFeeds().keySet()) {
+                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
+                    feeds.add(id);
+                }
+            }
+            
+            // Initial pass of locations (from the roots of reachability)
+            for (Map.Entry<String, String> entry : input.getLocations().entrySet()) {
+                String id = entry.getKey();
+                String locationState = entry.getValue();
+                if (locationState.contains("PortForwardManager")) {
+                    locations.add(id);
+                    continue;
+                }
+                for (BrooklynObjectType type : ImmutableList.of(BrooklynObjectType.ENTITY, BrooklynObjectType.ENRICHER, BrooklynObjectType.POLICY, BrooklynObjectType.FEED, BrooklynObjectType.CATALOG_ITEM)) {
+                    if (isMentionedBy(id, type, input)) {
+                        locations.add(id);
+                        break;
+                    }
+                }
+            }
+    
+            // Find the transitive reachabily locations from those we have already reached.
+            Set<String> locsToInspect = MutableSet.copyOf(locations);
+            
+            while (locsToInspect.size() > 0) {
+                Map<String, String> locStateToInspect = MutableMap.copyOf(input.getLocations());
+                locStateToInspect.keySet().retainAll(locsToInspect);
+ 
+                Set<String> unreachedLocs = Sets.difference(input.getLocations().keySet(), locations);
+                Set<String> newlyDiscoveredLocs = Sets.newLinkedHashSet();
+
+                for (String id : unreachedLocs) {
+                    if (isMentionedBy(id, locStateToInspect.values())) {
+                        newlyDiscoveredLocs.add(id);
+                    }
+                }
+                locations.addAll(newlyDiscoveredLocs);
+                locsToInspect = newlyDiscoveredLocs;
+            }
+    
+            return new ReferencedState()
+                    .locations(locations)
+                    .enrichers(enrichers)
+                    .policies(policies)
+                    .feeds(feeds);
+        }
         
-        while (locsToInspect.size() > 0) {
-            Set<String> referencedLocs = Sets.newLinkedHashSet();
-            for (String id : locsToInspect) {
-                String xmlData = locations.get(id);
-                if (xmlData != null) {
-                    referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationChildrenXpath, XPathConstants.NODESET)));
-                    referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationParentDirectTagXpath, XPathConstants.NODESET)));
-                    referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationProxyXpath, XPathConstants.NODESET)));
+        protected boolean isMentionedBy(String id, BrooklynObjectType type, BrooklynMementoRawData input) {
+            return isMentionedBy(id, input.getObjectsOfType(type).values());
+        }
+        
+        protected boolean isMentionedBy(String id, Iterable<String> states) {
+            for (String state : states) {
+                if (state.contains(id)) {
+                    return true;
                 }
             }
-            Set<String> newlyDiscoveredLocs = MutableSet.<String>builder()
-                    .addAll(referencedLocs)
-                    .removeAll(alreadyReferencedLocations)
-                    .removeAll(result)
-                    .build();
-            result.addAll(newlyDiscoveredLocs);
-            locsToInspect = newlyDiscoveredLocs;
+            return false;
         }
-
-        return result;
     }
-
+    
     protected <K, V> Map<K, V> copyRetainingKeys(Map<K, V> orig, Set<? extends K> keysToKeep) {
         Map<K, V> result = MutableMap.of();
         for (Map.Entry<K, V> entry : orig.entrySet()) {
@@ -188,16 +330,65 @@ public class DeleteOrphanedStateTransformer extends CompoundTransformer {
         return result;
     }
 
-    protected Set<String> getAllNodesFromXpath(org.w3c.dom.NodeList nodeList) {
-        Set<String> result = Sets.newLinkedHashSet();
-
-        for (int i = 0; i < nodeList.getLength(); i++) {
-            Node nextNode = nodeList.item(i);
-            if (nextNode != null) {
-                result.add(nextNode.getTextContent());
+    protected static class ReferencedState {
+        public static ReferencedState union(ReferencedState s1, ReferencedState s2) {
+            ReferencedState result = new ReferencedState();
+            result.locations(ImmutableSet.copyOf(Iterables.concat(s1.locations, s2.locations)));
+            result.enrichers(ImmutableSet.copyOf(Iterables.concat(s1.enrichers, s2.enrichers)));
+            result.policies(ImmutableSet.copyOf(Iterables.concat(s1.policies, s2.policies)));
+            result.feeds(ImmutableSet.copyOf(Iterables.concat(s1.feeds, s2.feeds)));
+            return result;
+        }
+        
+        public static void warnOfDifferences(ReferencedState s1, ReferencedState s2) {
+            Set<String> locDiffs = Sets.symmetricDifference(s1.locations, s2.locations);
+            Set<String> enricherDiffs = Sets.symmetricDifference(s1.enrichers, s2.enrichers);
+            Set<String> policyDiffs = Sets.symmetricDifference(s1.policies, s2.policies);
+            Set<String> feedDiffs = Sets.symmetricDifference(s1.feeds, s2.feeds);
+            
+            if (locDiffs.size() > 0) {
+                LOG.warn("Deletion of orphan state found unusually referenced locations (keeping): " + locDiffs);
+            }
+            if (enricherDiffs.size() > 0) {
+                LOG.warn("Deletion of orphan state found unusually referenced enrichers (keeping): " + enricherDiffs);
+            }
+            if (policyDiffs.size() > 0) {
+                LOG.warn("Deletion of orphan state found unusually referenced policies (keeping): " + policyDiffs);
+            }
+            if (feedDiffs.size() > 0) {
+                LOG.warn("Deletion of orphan state found unusually referenced feeds (keeping): " + feedDiffs);
             }
         }
+
+        protected Set<String> locations = ImmutableSet.of();
+        protected Set<String> feeds = ImmutableSet.of();
+        protected Set<String> enrichers = ImmutableSet.of();
+        protected Set<String> policies = ImmutableSet.of();
         
-        return result;
+        public ReferencedState filterForExtant(BrooklynMementoRawData input) {
+            ReferencedState result = new ReferencedState();
+            result.locations(Sets.intersection(locations, input.getLocations().keySet()));
+            result.enrichers(Sets.intersection(enrichers, input.getEnrichers().keySet()));
+            result.policies(Sets.intersection(policies, input.getPolicies().keySet()));
+            result.feeds(Sets.intersection(feeds, input.getFeeds().keySet()));
+            return result;
+        }
+
+        protected ReferencedState locations(Set<String> vals) {
+            locations = vals;
+            return this;
+        }
+        protected ReferencedState feeds(Set<String> vals) {
+            feeds = vals;
+            return this;
+        }
+        protected ReferencedState enrichers(Set<String> vals) {
+            enrichers = vals;
+            return this;
+        }
+        protected ReferencedState policies(Set<String> vals) {
+            policies = vals;
+            return this;
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19abd228/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractCleanOrphanedStateTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractCleanOrphanedStateTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractCleanOrphanedStateTest.java
index d26c411..a594f2a 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractCleanOrphanedStateTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/AbstractCleanOrphanedStateTest.java
@@ -39,6 +39,7 @@ import org.apache.commons.lang.builder.EqualsBuilder;
 import com.beust.jcommander.internal.Sets;
 import com.google.common.base.Function;
 import com.google.common.base.Functions;
+import com.google.common.collect.Iterables;
 
 public abstract class AbstractCleanOrphanedStateTest extends RebindTestFixtureWithApp {
     
@@ -63,33 +64,42 @@ public abstract class AbstractCleanOrphanedStateTest extends RebindTestFixtureWi
         }
     }
 
-    protected void assertTransformIsNoop() throws Exception {
-        assertTransformIsNoop(getRawData());
+    protected BrooklynMementoRawData assertTransformIsNoop() throws Exception {
+        return assertTransformIsNoop(getRawData());
     }
 
-    protected void assertTransformIsNoop(BrooklynMementoRawData origData) throws Exception {
-        assertTransformIsNoop(origData, Functions.<BrooklynMementoRawData>identity());
+    protected BrooklynMementoRawData assertTransformIsNoop(BrooklynMementoRawData origData) throws Exception {
+        return assertTransformIsNoop(origData, Functions.<BrooklynMementoRawData>identity());
     }
 
-    protected void assertTransformIsNoop(Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
-        assertTransformIsNoop(getRawData(), origDataTweaker);
+    protected BrooklynMementoRawData assertTransformIsNoop(Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
+        return assertTransformIsNoop(getRawData(), origDataTweaker);
     }
     
-    protected void assertTransformIsNoop(BrooklynMementoRawData origData, Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
+    protected BrooklynMementoRawData assertTransformIsNoop(BrooklynMementoRawData origData, Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
         BrooklynMementoRawData origDataTweaked = origDataTweaker.apply(origData);
         BrooklynMementoRawData transformedData = transformRawData(origDataTweaked);
         assertRawData(transformedData, origDataTweaked);
+        return transformedData;
     }
     
-    protected void assertTransformDeletes(Deletions deletions) throws Exception {
-        assertTransformDeletes(deletions, Functions.<BrooklynMementoRawData>identity());
+    protected BrooklynMementoRawData assertTransformDeletes(Deletions deletions) throws Exception {
+        return assertTransformDeletes(deletions, Functions.<BrooklynMementoRawData>identity());
     }
 
-    protected void assertTransformDeletes(Deletions deletions, Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
-        BrooklynMementoRawData origData = getRawData();
+    protected BrooklynMementoRawData assertTransformDeletes(Deletions deletions, BrooklynMementoRawData origData) throws Exception {
+        return assertTransformDeletes(deletions, origData, Functions.<BrooklynMementoRawData>identity());
+    }
+
+    protected BrooklynMementoRawData assertTransformDeletes(Deletions deletions, Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
+        return assertTransformDeletes(deletions, getRawData(), origDataTweaker);
+    }
+    
+    protected BrooklynMementoRawData assertTransformDeletes(Deletions deletions, BrooklynMementoRawData origData, Function<? super BrooklynMementoRawData, ? extends BrooklynMementoRawData> origDataTweaker) throws Exception {
         BrooklynMementoRawData origDataTweaked = origDataTweaker.apply(origData);
         BrooklynMementoRawData transformedData = transformRawData(origDataTweaked);
         assertRawData(transformedData, origDataTweaked, deletions);
+        return transformedData;
     }
 
     protected BrooklynMementoRawData getRawData() throws Exception {
@@ -147,6 +157,10 @@ public abstract class AbstractCleanOrphanedStateTest extends RebindTestFixtureWi
             if (vals != null) locations.addAll(Arrays.asList(vals));
             return this;
         }
+        protected Deletions locations(Iterable<String> vals) {
+            if (vals != null) Iterables.addAll(locations, vals);
+            return this;
+        }
         protected Deletions feeds(String... vals) {
             if (vals != null) feeds.addAll(Arrays.asList(vals));
             return this;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19abd228/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsIntegrationTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsIntegrationTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsIntegrationTest.java
index 89f677e..3f2bcc0 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsIntegrationTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsIntegrationTest.java
@@ -18,11 +18,14 @@
  */
 package org.apache.brooklyn.launcher;
 
+import java.util.Set;
+
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
 import org.apache.brooklyn.api.mgmt.rebind.PersistenceExceptionHandler;
 import org.apache.brooklyn.api.mgmt.rebind.RebindManager;
 import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
+import org.apache.brooklyn.core.internal.BrooklynProperties;
 import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
@@ -32,24 +35,16 @@ import org.apache.brooklyn.core.mgmt.persist.PersistMode;
 import org.apache.brooklyn.core.mgmt.persist.PersistenceObjectStore;
 import org.apache.brooklyn.core.mgmt.rebind.PersistenceExceptionHandlerImpl;
 import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl;
-import org.apache.brooklyn.core.mgmt.rebind.transformer.impl.DeleteOrphanedStateTransformer;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
 import org.apache.brooklyn.core.server.BrooklynServerPaths;
 import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.time.Duration;
-import org.apache.commons.lang.builder.EqualsBuilder;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-import org.apache.brooklyn.core.internal.BrooklynProperties;
-
-import java.util.Map;
-import java.util.Set;
-
-public class CleanOrphanedLocationsIntegrationTest {
+public class CleanOrphanedLocationsIntegrationTest extends AbstractCleanOrphanedStateTest {
 
     private String PERSISTED_STATE_PATH_WITH_ORPHANED_LOCATIONS = "/orphaned-locations-test-data/data-with-orphaned-locations";
     private String PERSISTED_STATE_PATH_WITH_MULTIPLE_LOCATIONS_OCCURRENCE = "/orphaned-locations-test-data/fake-multiple-location-for-multiple-search-tests";
@@ -63,8 +58,6 @@ public class CleanOrphanedLocationsIntegrationTest {
     private String destinationDir;
     private String persistenceLocation;
 
-    private DeleteOrphanedStateTransformer transformer;
-
     private ManagementContext managementContext;
 
     @BeforeMethod(alwaysRun = true)
@@ -74,8 +67,6 @@ public class CleanOrphanedLocationsIntegrationTest {
         persistenceDirWithMultipleLocationsOccurrence = getClass().getResource(PERSISTED_STATE_PATH_WITH_MULTIPLE_LOCATIONS_OCCURRENCE).getFile();
 
         destinationDir = getClass().getResource(PERSISTED_STATE_DESTINATION_PATH).getFile();
-
-        transformer = DeleteOrphanedStateTransformer.builder().build();
     }
 
     private void initManagementContextAndPersistence(String persistenceDir) {
@@ -102,13 +93,8 @@ public class CleanOrphanedLocationsIntegrationTest {
     }
 
     @Test
-    public void testSelectionWithOrphanedLocationsInData() {
-        final Set<String> locationsToKeep = MutableSet.of(
-                "tjdywoxbji",
-                "pudtixbw89"
-        );
+    public void testSelectionWithOrphanedLocationsInData() throws Exception {
         final Set<String> orphanedLocations = MutableSet.of(
-                "banby1jx4j",
                 "msyp655po0",
                 "ppamsemxgo"
         );
@@ -116,74 +102,21 @@ public class CleanOrphanedLocationsIntegrationTest {
         initManagementContextAndPersistence(persistenceDirWithOrphanedLocations);
         BrooklynMementoRawData mementoRawData = managementContext.getRebindManager().retrieveMementoRawData();
 
-        Set<String> allReferencedLocationsFoundByTransformer = transformer.findAllReferencedLocations(mementoRawData);
-        Map<String, String> locationsToKeepFoundByTransformer = transformer.findLocationsToKeep(mementoRawData);
-
-        Asserts.assertEquals(allReferencedLocationsFoundByTransformer, locationsToKeep);
-        Asserts.assertEquals(locationsToKeepFoundByTransformer.keySet(), locationsToKeep);
-
-        Map<String, String> locationsNotToKeepDueToTransormer = MutableMap.of();
-        locationsNotToKeepDueToTransormer.putAll(mementoRawData.getLocations());
-        for (Map.Entry location: locationsToKeepFoundByTransformer.entrySet()) {
-            locationsNotToKeepDueToTransormer.remove(location.getKey());
-        }
-        Set<String> notReferencedLocationsDueToTransormer = MutableSet.of();
-        notReferencedLocationsDueToTransormer.addAll(mementoRawData.getLocations().keySet());
-        for (String location: allReferencedLocationsFoundByTransformer) {
-            notReferencedLocationsDueToTransormer.remove(location);
-        }
-
-        Asserts.assertEquals(notReferencedLocationsDueToTransormer, orphanedLocations);
-        Asserts.assertEquals(locationsNotToKeepDueToTransormer.keySet(), orphanedLocations);
-
-
-        BrooklynMementoRawData transformedMemento = transformer.transform(mementoRawData);
-        Asserts.assertFalse(EqualsBuilder.reflectionEquals(mementoRawData, transformedMemento));
-        Asserts.assertTrue(EqualsBuilder.reflectionEquals(mementoRawData.getEntities(), transformedMemento.getEntities()));
-        Asserts.assertTrue(EqualsBuilder.reflectionEquals(mementoRawData.getEnrichers(), transformedMemento.getEnrichers()));
-        Asserts.assertTrue(EqualsBuilder.reflectionEquals(mementoRawData.getPolicies(), transformedMemento.getPolicies()));
-        Asserts.assertFalse(EqualsBuilder.reflectionEquals(mementoRawData.getLocations(), transformedMemento.getLocations()));
+        assertTransformDeletes(new Deletions().locations(orphanedLocations), mementoRawData);
     }
 
     @Test
-    public void testSelectionWithoutOrphanedLocationsInData() {
-
-        Set<String> locationsToKeep = MutableSet.of(
-                "tjdywoxbji",
-                "pudtixbw89"
-        );
-
+    public void testSelectionWithoutOrphanedLocationsInData() throws Exception {
         initManagementContextAndPersistence(persistenceDirWithoutOrphanedLocations);
         BrooklynMementoRawData mementoRawData = managementContext.getRebindManager().retrieveMementoRawData();
 
-        Set<String> allReferencedLocationsFoundByTransformer = transformer.findAllReferencedLocations(mementoRawData);
-        Map<String, String> locationsToKeepFoundByTransformer = transformer.findLocationsToKeep(mementoRawData);
-
-        Asserts.assertEquals(allReferencedLocationsFoundByTransformer, locationsToKeep);
-        Asserts.assertEquals(locationsToKeepFoundByTransformer.keySet(), locationsToKeep);
-
-        Map<String, String> locationsNotToKeepDueToTransormer = MutableMap.of();
-        locationsNotToKeepDueToTransormer.putAll(mementoRawData.getLocations());
-        for (Map.Entry location: locationsToKeepFoundByTransformer.entrySet()) {
-            locationsNotToKeepDueToTransormer.remove(location.getKey());
-        }
-        Set<String> notReferencedLocationsDueToTransormer = MutableSet.of();
-        notReferencedLocationsDueToTransormer.addAll(mementoRawData.getLocations().keySet());
-        for (String location: allReferencedLocationsFoundByTransformer) {
-            notReferencedLocationsDueToTransormer.remove(location);
-        }
-
-        Asserts.assertSize(locationsNotToKeepDueToTransormer.keySet(), 0);
-        Asserts.assertSize(notReferencedLocationsDueToTransormer, 0);
-
-        BrooklynMementoRawData transformedMemento = transformer.transform(mementoRawData);
-        Asserts.assertTrue(EqualsBuilder.reflectionEquals(mementoRawData, transformedMemento));
+        assertTransformIsNoop(mementoRawData);
     }
 
     @Test
     public void testCleanedCopiedPersistedState() throws Exception {
-
         BrooklynLauncher launcher = BrooklynLauncher.newInstance()
+                .webconsole(false)
                 .brooklynProperties(OsgiManager.USE_OSGI, false)
                 .persistMode(PersistMode.AUTO)
                 .persistenceDir(persistenceDirWithOrphanedLocations)
@@ -192,8 +125,6 @@ public class CleanOrphanedLocationsIntegrationTest {
 
         try {
             launcher.cleanOrphanedState(destinationDir, null);
-        } catch (Exception e) {
-            throw new Exception(e);
         } finally {
             launcher.terminate();
         }
@@ -203,42 +134,18 @@ public class CleanOrphanedLocationsIntegrationTest {
         Asserts.assertTrue(mementoRawDataFromCleanedState.getEntities().size() != 0);
         Asserts.assertTrue(mementoRawDataFromCleanedState.getLocations().size() != 0);
 
-
         initManagementContextAndPersistence(persistenceDirWithoutOrphanedLocations);
         BrooklynMementoRawData mementoRawData = managementContext.getRebindManager().retrieveMementoRawData();
 
-        Asserts.assertTrue(EqualsBuilder.reflectionEquals(mementoRawData, mementoRawDataFromCleanedState));
+        assertRawData(mementoRawData, mementoRawDataFromCleanedState);
     }
 
     @Test
-    public void testMultipleLocationOccurrenceInEntity() {
-        Set<String> allReferencedLocations = MutableSet.of(
-                "m72nvkl799",
-                "m72nTYl799",
-                "m72LKVN799",
-                "jf4rwubqyb",
-                "jf4rwuTTTb",
-                "jf4rwuHHHb",
-                "m72nvkl799",
-                "m72nPTUF99",
-                "m72nhtDr99",
-                "pski1c4s14"
-        );
-
-        Set<String> locationsToKeep = MutableSet.of(
-                "m72nvkl799",
-                "jf4rwubqyb",
-                "pski1c4s14"
-        );
-
+    public void testMultipleLocationOccurrenceInEntity() throws Exception {
         initManagementContextAndPersistence(persistenceDirWithMultipleLocationsOccurrence);
-
         BrooklynMementoRawData mementoRawData = managementContext.getRebindManager().retrieveMementoRawData();
-        Set<String> allReferencedLocationsFoundByTransformer = transformer.findAllReferencedLocations(mementoRawData);
-        Map<String, String> locationsToKeepFoundByTransformer = transformer.findLocationsToKeep(mementoRawData);
-
-        Asserts.assertEquals(allReferencedLocationsFoundByTransformer, allReferencedLocations);
-        Asserts.assertEquals(locationsToKeepFoundByTransformer.keySet(), locationsToKeep);
+        
+        assertTransformIsNoop(mementoRawData);
     }
 
     @AfterMethod

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/19abd228/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
index 258060b..0000905 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
@@ -19,20 +19,25 @@
 package org.apache.brooklyn.launcher;
 
 import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 
 import org.apache.brooklyn.api.location.Location;
 import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
 import org.apache.brooklyn.api.policy.PolicySpec;
 import org.apache.brooklyn.api.sensor.EnricherSpec;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.location.Locations;
+import org.apache.brooklyn.core.location.access.PortForwardManager;
+import org.apache.brooklyn.core.location.access.PortForwardManagerLocationResolver;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.net.HostAndPort;
 
 public class CleanOrphanedLocationsTest extends AbstractCleanOrphanedStateTest {
 
@@ -128,14 +133,7 @@ public class CleanOrphanedLocationsTest extends AbstractCleanOrphanedStateTest {
         assertTransformIsNoop();
     }
     
-    /**
-     * TODO Fails because it is persisted as {@code <defaultValue class="locationProxy">biqwd7ukcd</defaultValue>},
-     * rather than having locationProxy as the tag.
-     * 
-     * I (Aled) think we can live without this - hopefully no-one is setting location instances as 
-     * config default values!
-     */
-    @Test(groups="WIP", enabled=false)
+    @Test
     public void testKeepsLocationsReferencedInConfigKeyDefault() throws Exception {
         SshMachineLocation loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
         origApp.config().set(ConfigKeys.newConfigKey(MachineLocation.class, "myconfig", "my description", loc), (MachineLocation)null);
@@ -216,4 +214,21 @@ public class CleanOrphanedLocationsTest extends AbstractCleanOrphanedStateTest {
         origApp.feeds().add(feed);
         assertTransformIsNoop();
     }
+    
+    @Test
+    public void testKeepsPortForwardManager() throws Exception {
+        PortForwardManager pfm = (PortForwardManager) mgmt().getLocationRegistry().getLocationManaged(PortForwardManagerLocationResolver.PFM_GLOBAL_SPEC);
+        BrooklynMementoRawData transformedData = assertTransformIsNoop();
+        assertTrue(transformedData.getLocations().containsKey(pfm.getId()));
+    }
+    
+    @Test
+    public void testKeepsPortForwardManagerAssociatedMachines() throws Exception {
+        SshMachineLocation machine = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+        PortForwardManager pfm = (PortForwardManager) mgmt().getLocationRegistry().getLocationManaged(PortForwardManagerLocationResolver.PFM_GLOBAL_SPEC);
+        pfm.associate("mypublicid", HostAndPort.fromParts("mypublicip", 1234), machine, 22);
+        BrooklynMementoRawData transformedData = assertTransformIsNoop();
+        assertTrue(transformedData.getLocations().containsKey(pfm.getId()));
+        assertTrue(transformedData.getLocations().containsKey(machine.getId()));
+    }
 }


[2/2] brooklyn-server git commit: This closes #272

Posted by al...@apache.org.
This closes #272


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

Branch: refs/heads/master
Commit: 29039cb8ce844c953136251322c12efb27144088
Parents: 4042949 19abd22
Author: Aled Sage <al...@gmail.com>
Authored: Fri Jul 22 20:22:18 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Jul 22 20:22:18 2016 +0100

----------------------------------------------------------------------
 .../impl/DeleteOrphanedStateTransformer.java    | 371 ++++++++++++++-----
 .../AbstractCleanOrphanedStateTest.java         |  36 +-
 .../CleanOrphanedLocationsIntegrationTest.java  | 119 +-----
 .../launcher/CleanOrphanedLocationsTest.java    |  31 +-
 4 files changed, 342 insertions(+), 215 deletions(-)
----------------------------------------------------------------------