Author: asavu
Date: Fri Oct 14 15:03:32 2011
New Revision: 1183382
URL: http://svn.apache.org/viewvc?rev=1183382&view=rev
Log:
WHIRR-394. NPE used for flow control (David Alves via asavu)
Modified:
whirr/trunk/CHANGES.txt
whirr/trunk/core/src/main/java/org/apache/whirr/HandlerMapFactory.java
whirr/trunk/core/src/main/java/org/apache/whirr/actions/ScriptBasedClusterAction.java
whirr/trunk/core/src/test/java/org/apache/whirr/actions/BootstrapClusterActionTest.java
Modified: whirr/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/whirr/trunk/CHANGES.txt?rev=1183382&r1=1183381&r2=1183382&view=diff
==============================================================================
--- whirr/trunk/CHANGES.txt (original)
+++ whirr/trunk/CHANGES.txt Fri Oct 14 15:03:32 2011
@@ -41,6 +41,8 @@ Trunk (unreleased changes)
WHIRR-377. Fix broken CLI logging config. (asavu via tomwhite)
+ WHIRR-394. NPE used for flow control (David Alves via asavu)
+
Release 0.6.0 - 2011-08-17
NEW FEATURES
Modified: whirr/trunk/core/src/main/java/org/apache/whirr/HandlerMapFactory.java
URL:
http://svn.apache.org/viewvc/whirr/trunk/core/src/main/java/org/apache/whirr/HandlerMapFactory.java?rev=1183382&r1=1183381&r2=1183382&view=diff
==============================================================================
--- whirr/trunk/core/src/main/java/org/apache/whirr/HandlerMapFactory.java
(original)
+++ whirr/trunk/core/src/main/java/org/apache/whirr/HandlerMapFactory.java Fri
Oct 14 15:03:32 2011
@@ -56,24 +56,33 @@ public class HandlerMapFactory {
public ClusterActionHandler apply(String arg0) {
checkNotNull(arg0, "role");
for (String prefix : factoryMap.keySet()) {
- if (arg0.startsWith(prefix))
- return
factoryMap.get(prefix).create(arg0.substring(prefix.length()));
+ if (arg0.startsWith(prefix)) {
+ return checkNotNull(
+
factoryMap.get(prefix).create(arg0.substring(prefix.length())),
+ "Unable to create the action handler"
+ );
+ }
}
- return handlerMap.get(arg0);
+ return checkNotNull(handlerMap.get(arg0), "Action handler not found");
}
}
public static Map<String, ClusterActionHandler> create() {
- return create(ServiceLoader.load(ClusterActionHandlerFactory.class),
ServiceLoader
- .load(ClusterActionHandler.class));
+ return create(ServiceLoader.load(ClusterActionHandlerFactory.class),
+ ServiceLoader.load(ClusterActionHandler.class));
}
- public static Map<String, ClusterActionHandler>
create(Iterable<ClusterActionHandlerFactory> factories,
- Iterable<ClusterActionHandler> handlers) {
- Map<String, ClusterActionHandlerFactory> factoryMap =
indexFactoriesByRolePrefix(checkNotNull(factories,
- "factories"));
- Map<String, ClusterActionHandler> handlerMap =
indexHandlersByRole(checkNotNull(handlers, "handlers"));
- return new MapMaker().makeComputingMap(new
ReturnHandlerByRoleOrPrefix(factoryMap, handlerMap));
+ public static Map<String, ClusterActionHandler> create(
+ Iterable<ClusterActionHandlerFactory> factories,
+ Iterable<ClusterActionHandler> handlers
+ ) {
+ Map<String, ClusterActionHandlerFactory> factoryMap =
+ indexFactoriesByRolePrefix(checkNotNull(factories, "factories"));
+ Map<String, ClusterActionHandler> handlerMap =
+ indexHandlersByRole(checkNotNull(handlers, "handlers"));
+
+ return new MapMaker().makeComputingMap(
+ new ReturnHandlerByRoleOrPrefix(factoryMap, handlerMap));
}
static Map<String, ClusterActionHandlerFactory> indexFactoriesByRolePrefix(
Modified:
whirr/trunk/core/src/main/java/org/apache/whirr/actions/ScriptBasedClusterAction.java
URL:
http://svn.apache.org/viewvc/whirr/trunk/core/src/main/java/org/apache/whirr/actions/ScriptBasedClusterAction.java?rev=1183382&r1=1183381&r2=1183382&view=diff
==============================================================================
---
whirr/trunk/core/src/main/java/org/apache/whirr/actions/ScriptBasedClusterAction.java
(original)
+++
whirr/trunk/core/src/main/java/org/apache/whirr/actions/ScriptBasedClusterAction.java
Fri Oct 14 15:03:32 2011
@@ -19,6 +19,7 @@
package org.apache.whirr.actions;
import com.google.common.base.Function;
+import com.google.common.collect.ComputationException;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
@@ -35,6 +36,8 @@ import org.apache.whirr.service.Firewall
import org.apache.whirr.service.jclouds.StatementBuilder;
import org.jclouds.compute.ComputeServiceContext;
+import static com.google.common.base.Preconditions.checkNotNull;
+
/**
* A {@link ClusterAction} that provides the base functionality for running
* scripts on instances in the cluster.
@@ -46,7 +49,7 @@ public abstract class ScriptBasedCluster
protected ScriptBasedClusterAction(Function<ClusterSpec,
ComputeServiceContext> getCompute,
final Map<String, ClusterActionHandler> handlerMap) {
super(getCompute);
- this.handlerMap = handlerMap;
+ this.handlerMap = checkNotNull(handlerMap, "handlerMap");
}
protected abstract void doAction(Map<InstanceTemplate, ClusterActionEvent>
eventMap)
@@ -68,13 +71,11 @@ public abstract class ScriptBasedCluster
eventMap.put(instanceTemplate, event);
for (String role : instanceTemplate.getRoles()) {
- try {
- handlerMap.get(role).beforeAction(event);
- } catch (NullPointerException e) {
- throw new IllegalArgumentException("No handler for role " + role);
- }
+ safeGetActionHandler(role).beforeAction(event);
}
- newCluster = event.getCluster(); // cluster may have been updated by
handler
+
+ // cluster may have been updated by handler
+ newCluster = event.getCluster();
}
doAction(eventMap);
@@ -83,19 +84,34 @@ public abstract class ScriptBasedCluster
newCluster = Iterables.get(eventMap.values(), 0).getCluster();
for (InstanceTemplate instanceTemplate :
clusterSpec.getInstanceTemplates()) {
+ ClusterActionEvent event = eventMap.get(instanceTemplate);
for (String role : instanceTemplate.getRoles()) {
- ClusterActionEvent event = eventMap.get(instanceTemplate);
event.setCluster(newCluster);
- try {
- handlerMap.get(role).afterAction(event);
- } catch (NullPointerException e) {
- throw new IllegalArgumentException("No handler for role " + role);
- }
- newCluster = event.getCluster(); // cluster may have been updated by
handler
+ safeGetActionHandler(role).afterAction(event);
+
+ // cluster may have been updated by handler
+ newCluster = event.getCluster();
}
}
return newCluster;
}
+ /**
+ * Try to get an {@see ClusterActionHandler } instance or throw an
+ * IllegalArgumentException if not found for this role name
+ */
+ private ClusterActionHandler safeGetActionHandler(String role) {
+ try {
+ ClusterActionHandler handler = handlerMap.get(role);
+ if (handler == null) {
+ throw new IllegalArgumentException("No handler for role " + role);
+ }
+ return handler;
+
+ } catch (ComputationException e) {
+ throw new IllegalArgumentException(e.getCause());
+ }
+ }
+
}
Modified:
whirr/trunk/core/src/test/java/org/apache/whirr/actions/BootstrapClusterActionTest.java
URL:
http://svn.apache.org/viewvc/whirr/trunk/core/src/test/java/org/apache/whirr/actions/BootstrapClusterActionTest.java?rev=1183382&r1=1183381&r2=1183382&view=diff
==============================================================================
---
whirr/trunk/core/src/test/java/org/apache/whirr/actions/BootstrapClusterActionTest.java
(original)
+++
whirr/trunk/core/src/test/java/org/apache/whirr/actions/BootstrapClusterActionTest.java
Fri Oct 14 15:03:32 2011
@@ -78,7 +78,7 @@ public class BootstrapClusterActionTest
LoggerFactory.getLogger(BootstrapClusterActionTest.class);
@SuppressWarnings("unchecked")
-@Test
+ @Test
public void testDoActionRetriesSucceed() throws Exception {
CompositeConfiguration config = new CompositeConfiguration();
if (System.getProperty("config") != null) {
@@ -408,8 +408,8 @@ public class BootstrapClusterActionTest
when(puppetHandlerFactory.getRolePrefix()).thenReturn("puppet");
when(handler.getRole()).thenReturn("something-else");
- Map<String, ClusterActionHandler> handlerMap =
HandlerMapFactory.create(ImmutableSet.of(puppetHandlerFactory),
- ImmutableSet.of(handler));
+ Map<String, ClusterActionHandler> handlerMap = HandlerMapFactory.create(
+ ImmutableSet.of(puppetHandlerFactory), ImmutableSet.of(handler));
Function<ClusterSpec, ComputeServiceContext> getCompute =
mock(Function.class);
ComputeServiceContext serviceContext = mock(ComputeServiceContext.class);
@@ -426,7 +426,7 @@ public class BootstrapClusterActionTest
Map<Set<String>, Stack<Integer>> reaction = Maps.newHashMap();
Stack<Integer> nnStack = new Stack<Integer>();
- nnStack.push(new Integer(1));
+ nnStack.push(1);
reaction.put(nn, nnStack);
nodeStarterFactory = new TestNodeStarterFactory(reaction);
