ambari git commit: AMBARI-8647. Pass Injector to ServerActionExecutor so objects can be injected into ServerAction implementations (Rob Levas via ncole)

[nc...@apache.org]

Repository: ambari
Updated Branches:
  refs/heads/trunk 1d53dc8f8 -> a1b2d1dd7


AMBARI-8647. Pass Injector to ServerActionExecutor so objects can be injected into ServerAction implementations (Rob Levas via ncole)


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

Branch: refs/heads/trunk
Commit: a1b2d1dd7e30fc1b5e8244186d560406e7482d88
Parents: 1d53dc8
Author: Nate Cole <nc...@hortonworks.com>
Authored: Sun Dec 14 07:49:53 2014 -0500
Committer: Nate Cole <nc...@hortonworks.com>
Committed: Sun Dec 14 07:49:53 2014 -0500

----------------------------------------------------------------------
 .../serveraction/ServerActionExecutor.java      | 29 ++++++++++++++++----
 .../actionmanager/TestActionScheduler.java      | 22 ++++++++++++++-
 .../server/serveraction/MockServerAction.java   | 11 ++++++--
 .../serveraction/ServerActionExecutorTest.java  | 29 ++++++++++++++++----
 4 files changed, 77 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/a1b2d1dd/ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
index fd61cc2..ff50bc9 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java
@@ -18,8 +18,11 @@
 
 package org.apache.ambari.server.serveraction;
 
+import com.google.inject.Inject;
+import com.google.inject.Injector;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.Role;
+import org.apache.ambari.server.StaticallyInject;
 import org.apache.ambari.server.actionmanager.*;
 import org.apache.ambari.server.agent.CommandReport;
 import org.apache.ambari.server.agent.ExecutionCommand;
@@ -39,6 +42,7 @@ import java.util.concurrent.ConcurrentMap;
  * ActionScheduler such that it is started when the ActionScheduler is started and stopped when the
  * ActionScheduler is stopped.
  */
+@StaticallyInject
 public class ServerActionExecutor {
 
   private final static Logger LOG = LoggerFactory.getLogger(ServerActionExecutor.class);
@@ -46,6 +50,12 @@ public class ServerActionExecutor {
   private final static Long POLLING_TIMEOUT_MS = 1000L * 5;
 
   /**
+   * An injector to use to inject objects into ServerAction instances.
+   */
+  @Inject
+  private static Injector injector;
+
+  /**
    * Maps request IDs to "blackboards" of shared data.
    * <p/>
    * This map is not synchronized, so any access to it should synchronize on
@@ -88,6 +98,17 @@ public class ServerActionExecutor {
   private Thread executorThread = null;
 
   /**
+   * Statically initialize the Injector
+   * <p/>
+   * This should only be used for unit tests.
+   *
+   * @param injector the Injector to (manually) statically inject
+   */
+  public static void init(Injector injector) {
+    ServerActionExecutor.injector = injector;
+  }
+
+  /**
    * Creates a new ServerActionExecutor
    *
    * @param db          the ActionDBAccessor to use to read and update tasks
@@ -256,7 +277,7 @@ public class ServerActionExecutor {
     commandReport.setStatus(HostRoleStatus.FAILED.toString());
     commandReport.setExitCode(1);
     commandReport.setStdOut("Server action failed");
-    commandReport.setStdErr(message);
+    commandReport.setStdErr((message == null) ? "Server action failed" : message);
     return commandReport;
   }
 
@@ -507,15 +528,11 @@ public class ServerActionExecutor {
           if (serverActionClass == null) {
             throw new AmbariException("Unable to execute server action class, invalid type: " + classname);
           } else {
-            return serverActionClass.newInstance();
+            return injector.getInstance(serverActionClass);
           }
         }
       } catch (ClassNotFoundException e) {
         throw new AmbariException("Unable to load server action class: " + classname, e);
-      } catch (InstantiationException e) {
-        throw new AmbariException("Unable to create an instance of the server action class: " + classname, e);
-      } catch (IllegalAccessException e) {
-        throw new AmbariException("Unable to create an instance of the server action class: " + classname, e);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/a1b2d1dd/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
index d14f126..8ce4ff2 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java
@@ -28,6 +28,9 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import com.google.common.reflect.TypeToken;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
 import com.google.inject.persist.UnitOfWork;
 import junit.framework.Assert;
 import org.apache.ambari.server.AmbariException;
@@ -45,6 +48,7 @@ import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.controller.HostsMap;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
 import org.apache.ambari.server.serveraction.MockServerAction;
+import org.apache.ambari.server.serveraction.ServerActionExecutor;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Host;
@@ -60,6 +64,7 @@ import org.apache.ambari.server.state.svccomphost.ServiceComponentHostUpgradeEve
 import org.apache.ambari.server.utils.StageUtils;
 import org.easymock.Capture;
 import org.easymock.EasyMock;
+import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -78,10 +83,16 @@ public class TestActionScheduler {
       + " c6402.ambari.apache.org], slave_hosts=[c6401.ambari.apache.org,"
       + " c6402.ambari.apache.org]}";
 
+  private static Injector injector;
+
   private final String serverHostname = StageUtils.getHostName();
   private final String hostname = "ahost.ambari.apache.org";
   private final int MAX_CYCLE_ITERATIONS = 100;
 
+  @BeforeClass
+  public static void beforeClass() throws AmbariException {
+    injector = Guice.createInjector(new MockModule());
+  }
 
   /**
    * This test sends a new action to the action scheduler and verifies that the action
@@ -555,6 +566,7 @@ public class TestActionScheduler {
       }
     }).when(db).getTasksByHostRoleAndStatus(anyString(), anyString(), any(HostRoleStatus.class));
 
+    ServerActionExecutor.init(injector);
     ActionScheduler scheduler = new ActionScheduler(100, 50, db, aq, fsm, 3,
         new HostsMap((String) null), unitOfWork, null, conf);
 
@@ -647,7 +659,7 @@ public class TestActionScheduler {
       }
     }).when(db).getTasksByHostRoleAndStatus(anyString(), anyString(), any(HostRoleStatus.class));
 
-
+    ServerActionExecutor.init(injector);
     ActionScheduler scheduler = new ActionScheduler(100, 50, db, aq, fsm, 3,
         new HostsMap((String) null), unitOfWork, null, conf);
 
@@ -1833,6 +1845,7 @@ public class TestActionScheduler {
       }
     }).when(db).getTask(anyLong());
 
+    ServerActionExecutor.init(injector);
     ActionScheduler scheduler = new ActionScheduler(100, 50, db, aq, fsm, 3,
         new HostsMap((String) null), unitOfWork, null, conf);
 
@@ -2175,4 +2188,11 @@ public class TestActionScheduler {
 
   }
 
+  public static class MockModule extends AbstractModule {
+    @Override
+    protected void configure() {
+      bind(Clusters.class).toInstance(mock(Clusters.class));
+    }
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/a1b2d1dd/ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
index ba9a5af..cc6f9af 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java
@@ -18,10 +18,12 @@
 
 package org.apache.ambari.server.serveraction;
 
+import com.google.inject.Inject;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.actionmanager.HostRoleStatus;
 import org.apache.ambari.server.agent.CommandReport;
 import org.apache.ambari.server.agent.ExecutionCommand;
+import org.apache.ambari.server.state.Clusters;
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentMap;
@@ -38,7 +40,7 @@ import java.util.concurrent.ConcurrentMap;
  * - Causes the action to fail by timing out (the COMMAND_TIMEOUT value must be set to a reasonable
  * value)</li>
  * </dl>
- *
+ * <p/>
  * If not instructed to fail, this implementation will attempt to increment a "data" counter in a
  * shared data context - if available.
  */
@@ -46,13 +48,18 @@ public class MockServerAction extends AbstractServerAction {
 
   public static final String PAYLOAD_FORCE_FAIL = "force_fail";
 
+  @Inject
+  private Clusters clusters;
+
   @Override
   public CommandReport execute(ConcurrentMap<String, Object> requestSharedDataContext)
       throws AmbariException, InterruptedException {
 
     Map<String, String> commandParameters = getCommandParameters();
 
-    if (commandParameters == null) {
+    if (clusters == null) { // Ensure that the injected Clusters object exists...
+      throw new AmbariException("Missing payload");
+    } else if (commandParameters == null) {
       throw new AmbariException("Missing payload");
     } else if ("exception".equalsIgnoreCase(commandParameters.get(PAYLOAD_FORCE_FAIL))) {
       throw new AmbariException("Failing execution by request");

http://git-wip-us.apache.org/repos/asf/ambari/blob/a1b2d1dd/ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
index e89477a..db08460 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java
@@ -18,13 +18,18 @@
 
 package org.apache.ambari.server.serveraction;
 
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.Role;
 import org.apache.ambari.server.RoleCommand;
 import org.apache.ambari.server.actionmanager.*;
 import org.apache.ambari.server.agent.CommandReport;
-import org.apache.ambari.server.agent.ExecutionCommand;
+import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.svccomphost.ServiceComponentHostServerActionEvent;
 import org.apache.ambari.server.utils.StageUtils;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -44,6 +49,13 @@ public class ServerActionExecutorTest {
       + SERVER_HOST_NAME + "], slave_hosts=["
       + SERVER_HOST_NAME + "]}";
 
+  private static Injector injector;
+
+  @BeforeClass
+  public static void beforeClass() throws AmbariException {
+    injector = Guice.createInjector(new MockModule());
+  }
+
   /**
    * Test a normal server action
    */
@@ -57,6 +69,7 @@ public class ServerActionExecutorTest {
       }
     };
     ActionDBAccessor db = createMockActionDBAccessor(request, stages);
+    ServerActionExecutor.init(injector);
     ServerActionExecutor executor = new ServerActionExecutor(db, 10000);
 
     // Force the task to be QUEUED
@@ -90,6 +103,7 @@ public class ServerActionExecutorTest {
       }
     };
     ActionDBAccessor db = createMockActionDBAccessor(request, stages);
+    ServerActionExecutor.init(injector);
     ServerActionExecutor executor = new ServerActionExecutor(db, 10000);
 
     // Force the task to be QUEUED
@@ -123,6 +137,7 @@ public class ServerActionExecutorTest {
       }
     };
     ActionDBAccessor db = createMockActionDBAccessor(request, stages);
+    ServerActionExecutor.init(injector);
     ServerActionExecutor executor = new ServerActionExecutor(db, 10000);
 
     // Force the task to be QUEUED
@@ -155,6 +170,7 @@ public class ServerActionExecutorTest {
       }
     };
     ActionDBAccessor db = createMockActionDBAccessor(request, stages);
+    ServerActionExecutor.init(injector);
     ServerActionExecutor executor = new ServerActionExecutor(db, 10000);
 
     // Force the task to be QUEUED
@@ -168,10 +184,6 @@ public class ServerActionExecutorTest {
     assertEquals(HostRoleStatus.FAILED, getTaskStatus(s));
   }
 
-  private HostRoleStatus getTaskStatus(List<Stage> stages, int i) {
-    return getTaskStatus(stages.get(i));
-  }
-
   private HostRoleStatus getTaskStatus(Stage stage) {
     return stage.getHostRoleStatus(SERVER_HOST_NAME, "AMBARI_SERVER_ACTION");
   }
@@ -245,4 +257,11 @@ public class ServerActionExecutorTest {
 
     return stage;
   }
+
+  public static class MockModule extends AbstractModule {
+    @Override
+    protected void configure() {
+      bind(Clusters.class).toInstance(mock(Clusters.class));
+    }
+  }
 }
\ No newline at end of file