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/20 17:20:14 UTC
[07/11] brooklyn-server git commit: deleting orphaned loss: more
testing + code tidy
deleting orphaned loss: more testing + code tidy
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/4d0c4b8c
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/4d0c4b8c
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/4d0c4b8c
Branch: refs/heads/master
Commit: 4d0c4b8c805c7cea399f88f14525ce77f4287d7f
Parents: 5253d4b
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jul 19 23:06:21 2016 +0100
Committer: Ivana Yovcheva <iv...@gmail.com>
Committed: Wed Jul 20 12:18:48 2016 +0300
----------------------------------------------------------------------
.../DeleteOrphanedLocationsTransformer.java | 103 ++++++-------------
.../launcher/CleanOrphanedLocationsTest.java | 58 +++++++++--
2 files changed, 81 insertions(+), 80 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4d0c4b8c/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedLocationsTransformer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedLocationsTransformer.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedLocationsTransformer.java
index 9f87984..a26d807 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedLocationsTransformer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedLocationsTransformer.java
@@ -18,30 +18,27 @@
*/
package org.apache.brooklyn.core.mgmt.rebind.transformer.impl;
-import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import com.sun.org.apache.xml.internal.dtm.ref.DTMNodeList;
+import javax.xml.xpath.XPathConstants;
+
import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMemento;
import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
import org.apache.brooklyn.api.mgmt.rebind.mementos.EntityMemento;
import org.apache.brooklyn.api.mgmt.rebind.mementos.LocationMemento;
-import org.apache.brooklyn.core.mgmt.rebind.dto.BrooklynMementoImpl;
import org.apache.brooklyn.core.mgmt.rebind.transformer.BrooklynMementoTransformer;
import org.apache.brooklyn.core.mgmt.rebind.transformer.CompoundTransformer;
-import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.xstream.XmlUtil;
+import org.w3c.dom.Node;
import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
-import org.apache.brooklyn.util.core.xstream.XmlUtil;
-import org.w3c.dom.Node;
-
-import javax.xml.xpath.XPathConstants;
@Beta
public class DeleteOrphanedLocationsTransformer extends CompoundTransformer implements BrooklynMementoTransformer {
@@ -56,11 +53,13 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
public static class Builder extends CompoundTransformer.Builder {
+ @Override
public DeleteOrphanedLocationsTransformer build() {
return new DeleteOrphanedLocationsTransformer(this);
}
}
+ @Override
public BrooklynMementoRawData transform(BrooklynMementoRawData input) {
Map<String, String> locationsToKeep = findLocationsToKeep(input);
@@ -74,55 +73,12 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
.build();
}
- // TODO Work in progress; untested code!
-
+ @Override
public BrooklynMemento transform(BrooklynMemento input) throws Exception {
- Set<String> referencedLocationIds = findReferencedLocationIds(input);
- Set<String> unreferencedLocationIds = Sets.newLinkedHashSet();
- List<String> toCheck = Lists.newLinkedList(input.getLocationIds());
-
- while (!toCheck.isEmpty()) {
- String locationId = toCheck.remove(0);
- List<String> locationsInHierarchy = MutableList.<String>builder()
- .add(locationId)
- .addAll(findLocationAncestors(input, locationId))
- .addAll(findLocationDescendents(input, locationId))
- .build();
-
- if (containsAny(referencedLocationIds, locationsInHierarchy)) {
- // keep them all
- } else {
- unreferencedLocationIds.addAll(locationsInHierarchy);
- }
- toCheck.removeAll(locationsInHierarchy);
- }
-
- // TODO What about brooklyn version?
- return BrooklynMementoImpl.builder()
- .applicationIds(input.getApplicationIds())
- .topLevelLocationIds(MutableSet.<String>builder()
- .addAll(input.getTopLevelLocationIds())
- .removeAll(unreferencedLocationIds)
- .build())
- .entities(input.getEntityMementos())
- .locations(MutableMap.<String, LocationMemento>builder()
- .putAll(input.getLocationMementos())
- .removeAll(unreferencedLocationIds)
- .build())
- .policies(input.getPolicyMementos())
- .enrichers(input.getEnricherMementos())
- .catalogItems(input.getCatalogItemMementos())
- .build();
- }
-
- public boolean containsAny(Collection<?> container, Iterable<?> contenders) {
- for (Object contender : contenders) {
- if (container.contains(contender)) return true;
- }
- return false;
+ throw new UnsupportedOperationException();
}
- public Set<String> findReferencedLocationIds(BrooklynMemento input) {
+ protected Set<String> findReferencedLocationIds(BrooklynMemento input) {
Set<String> result = Sets.newLinkedHashSet();
for (EntityMemento entity : input.getEntityMementos().values()) {
@@ -131,18 +87,20 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
return result;
}
+ @VisibleForTesting
public Map<String, String> findLocationsToKeep(BrooklynMementoRawData input) {
Map<String, String> locationsToKeep = MutableMap.of();
Set<String> allReferencedLocations = findAllReferencedLocations(input);
- for (Map.Entry location: input.getLocations().entrySet()) {
+ for (Map.Entry<String, String> location: input.getLocations().entrySet()) {
if (allReferencedLocations.contains(location.getKey())) {
- locationsToKeep.put((String) location.getKey(), (String) location.getValue());
+ locationsToKeep.put(location.getKey(), location.getValue());
}
}
return locationsToKeep;
}
+ @VisibleForTesting
public Set<String> findAllReferencedLocations(BrooklynMementoRawData input) {
Set<String> allReferencedLocations = MutableSet.of();
@@ -155,46 +113,43 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
return allReferencedLocations;
}
- private Set<String> searchLocationsToKeep(Map<String, String> entities, String searchInTypePrefix) {
+ protected Set<String> searchLocationsToKeep(Map<String, String> entities, String searchInTypePrefix) {
Set<String> result = MutableSet.of();
- for(Map.Entry entity: entities.entrySet()) {
+ for(Map.Entry<String, String> entity: entities.entrySet()) {
- String prefix = "/entity";
String location = "/locations/string";
- String locationProxy = "/attributes//locationProxy";
- String locationInConfig = "/config//locationProxy";
+ String locationProxy = "//locationProxy";
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) entity.getValue(), searchInTypePrefix+location, XPathConstants.NODESET)));
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) entity.getValue(), searchInTypePrefix+locationProxy, XPathConstants.NODESET)));
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) entity.getValue(), searchInTypePrefix+locationInConfig, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entity.getValue(), searchInTypePrefix+location, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entity.getValue(), searchInTypePrefix+locationProxy, XPathConstants.NODESET)));
}
return result;
}
- private Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
+ protected Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
Set<String> result = MutableSet.of();
String prefix = "/location";
String locationId = "/id";
String locationChildren = "/children/string";
String locationParentDirectTag = "/parent";
- String locationProxy = "/locationConfig//locationProxy";
+ String locationProxy = "//locationProxy";
- for (Map.Entry location: locations.entrySet()) {
+ for (Map.Entry<String, String> location: locations.entrySet()) {
if (alreadyReferencedLocations.contains(location.getKey())) {
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) location.getValue(), prefix+locationId, XPathConstants.NODESET)));
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) location.getValue(), prefix+locationChildren, XPathConstants.NODESET)));
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) location.getValue(), prefix+locationParentDirectTag, XPathConstants.NODESET)));
- result.addAll(getAllNodesFromXpath((DTMNodeList) XmlUtil.xpath((String) location.getValue(), prefix+locationProxy, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(location.getValue(), prefix+locationId, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(location.getValue(), prefix+locationChildren, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(location.getValue(), prefix+locationParentDirectTag, XPathConstants.NODESET)));
+ result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(location.getValue(), prefix+locationProxy, XPathConstants.NODESET)));
}
}
return result;
}
- private Set<String> getAllNodesFromXpath(DTMNodeList nodeList) {
+ protected Set<String> getAllNodesFromXpath(org.w3c.dom.NodeList nodeList) {
Set<String> result = MutableSet.of();
int i = 0;
@@ -207,7 +162,7 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
return result;
}
- public Set<String> findLocationAncestors(BrooklynMemento input, String locationId) {
+ protected Set<String> findLocationAncestors(BrooklynMemento input, String locationId) {
Set<String> result = Sets.newLinkedHashSet();
String parentId = null;
@@ -220,7 +175,7 @@ public class DeleteOrphanedLocationsTransformer extends CompoundTransformer impl
return result;
}
- public Set<String> findLocationDescendents(BrooklynMemento input, String locationId) {
+ protected Set<String> findLocationDescendents(BrooklynMemento input, String locationId) {
Set<String> result = Sets.newLinkedHashSet();
List<String> tovisit = Lists.newLinkedList();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/4d0c4b8c/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 ff8515a..7be6bd5 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/CleanOrphanedLocationsTest.java
@@ -39,6 +39,7 @@ import org.apache.brooklyn.core.policy.AbstractPolicy;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.location.ssh.SshMachineLocation;
import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.flags.SetFromFlag;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.testng.annotations.Test;
@@ -103,8 +104,31 @@ public class CleanOrphanedLocationsTest extends RebindTestFixtureWithApp {
}
@Test
- public void testKeepsParentOfReachableLocation() throws Exception {
- SshMachineLocation parentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ public void testKeepsLocationsReferencedInTag() throws Exception {
+ SshMachineLocation loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ origApp.tags().addTag(loc);
+ 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)
+ public void testKeepsLocationsReferencedInConfigKeyDefault() throws Exception {
+ SshMachineLocation loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ origApp.config().set(ConfigKeys.newConfigKey(Object.class, "myconfig", "my description", loc), null);
+ assertTransformIsNoop();
+ }
+
+ @Test
+ public void testKeepsAncestorsOfReachableLocation() throws Exception {
+ SshMachineLocation grandparentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ SshMachineLocation parentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .parent((grandparentLoc)));
SshMachineLocation childLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
.parent((parentLoc)));
origApp.addLocations(ImmutableList.of(childLoc));
@@ -112,11 +136,13 @@ public class CleanOrphanedLocationsTest extends RebindTestFixtureWithApp {
}
@Test
- public void testKeepsChildrenOfReachableLocation() throws Exception {
- SshMachineLocation parentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ public void testKeepsDescendantsOfReachableLocation() throws Exception {
+ SshMachineLocation grandparentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ SshMachineLocation parentLoc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .parent((grandparentLoc)));
mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
.parent((parentLoc)));
- origApp.addLocations(ImmutableList.of(parentLoc));
+ origApp.addLocations(ImmutableList.of(grandparentLoc));
assertTransformIsNoop();
}
@@ -128,7 +154,18 @@ public class CleanOrphanedLocationsTest extends RebindTestFixtureWithApp {
origApp.addLocations(ImmutableList.of(refereeLoc));
assertTransformIsNoop();
}
-
+
+ @Test
+ public void testKeepsTransitiveReferencesOfReachableLocation() throws Exception {
+ Location loc3 = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ Location loc2 = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .configure(ConfigKeys.newConfigKey(Object.class, "myconfig"), loc3));
+ Location loc1 = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .configure(ConfigKeys.newConfigKey(Object.class, "myconfig"), loc2));
+ origApp.addLocations(ImmutableList.of(loc1));
+ assertTransformIsNoop();
+ }
+
@Test
public void testKeepsLocationsReferencedByEnricher() throws Exception {
Location loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
@@ -138,6 +175,14 @@ public class CleanOrphanedLocationsTest extends RebindTestFixtureWithApp {
}
@Test
+ public void testKeepsLocationsReferencedByEnricherInFlag() throws Exception {
+ Location loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
+ origApp.enrichers().add(EnricherSpec.create(MyEnricher.class)
+ .configure("myobj", loc));
+ assertTransformIsNoop();
+ }
+
+ @Test
public void testKeepsLocationsReferencedByPolicy() throws Exception {
Location loc = mgmt().getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class));
origApp.policies().add(PolicySpec.create(MyPolicy.class)
@@ -231,6 +276,7 @@ public class CleanOrphanedLocationsTest extends RebindTestFixtureWithApp {
}
public static class MyEnricher extends AbstractEnricher {
+ @SetFromFlag Object obj;
}
public static class MyPolicy extends AbstractPolicy {