AMBARI-10590 - Expose A Way To Register Custom Alert Dispatchers (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java

@@ -67,6 +67,8 @@ import org.apache.ambari.server.controller.internal.RepositoryVersionResourcePro
 import org.apache.ambari.server.controller.internal.ServiceResourceProvider;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
 import org.apache.ambari.server.controller.utilities.DatabaseChecker;
+import org.apache.ambari.server.notifications.DispatchFactory;
+import org.apache.ambari.server.notifications.NotificationDispatcher;
 import org.apache.ambari.server.orm.DBAccessor;
 import org.apache.ambari.server.orm.DBAccessorImpl;
 import org.apache.ambari.server.orm.PersistenceType;
@@ -117,6 +119,7 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
 import org.springframework.core.type.filter.AnnotationTypeFilter;
+import org.springframework.core.type.filter.AssignableTypeFilter;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.crypto.password.StandardPasswordEncoder;
 import org.springframework.util.ClassUtils;
@@ -286,6 +289,7 @@ public class ControllerModule extends AbstractModule {
         setTargetBeanName("springSecurityFilterChain");
       }
     });
+
     bind(Gson.class).annotatedWith(Names.named("prettyGson")).toInstance(prettyGson);
 
     install(buildJpaPersistModule());
@@ -311,8 +315,8 @@ public class ControllerModule extends AbstractModule {
     bindConstant().annotatedWith(Names.named("executionCommandCacheSize")).
         to(configuration.getExecutionCommandsCacheSize());
 
-    bind(AmbariManagementController.class)
-        .to(AmbariManagementControllerImpl.class);
+    bind(AmbariManagementController.class).to(
+        AmbariManagementControllerImpl.class);
     bind(AbstractRootServiceResponseFactory.class).to(RootServiceResponseFactory.class);
     bind(ExecutionScheduler.class).to(ExecutionSchedulerImpl.class);
     bind(DBAccessor.class).to(DBAccessorImpl.class);
@@ -322,6 +326,7 @@ public class ControllerModule extends AbstractModule {
     requestStaticInjection(DatabaseChecker.class);
 
     bindByAnnotation(null);
+    bindNotificationDispatchers();
   }
 
 
@@ -499,4 +504,51 @@ public class ControllerModule extends AbstractModule {
 
     return beanDefinitions;
   }
+
+  /**
+   * Searches for all instances of {@link NotificationDispatcher} on the
+   * classpath and registers each as a singleton with the
+   * {@link DispatchFactory}.
+   */
+  private void bindNotificationDispatchers() {
+    ClassPathScanningCandidateComponentProvider scanner =
+        new ClassPathScanningCandidateComponentProvider(false);
+
+    // make the factory a singleton
+    DispatchFactory dispatchFactory = DispatchFactory.getInstance();
+    bind(DispatchFactory.class).toInstance(dispatchFactory);
+
+    // match all implementations of the dispatcher interface
+    AssignableTypeFilter filter = new AssignableTypeFilter(
+        NotificationDispatcher.class);
+
+    scanner.addIncludeFilter(filter);
+
+    Set<BeanDefinition> beanDefinitions = scanner.findCandidateComponents(
+        "org.apache.ambari.server.notifications.dispatchers");
+
+    // no dispatchers is a problem
+    if (null == beanDefinitions || beanDefinitions.size() == 0) {
+      LOG.error("No instances of {} found to register", NotificationDispatcher.class);
+      return;
+    }
+
+    // for every discovered dispatcher, singleton-ize them and register with
+    // the dispatch factory
+    for (BeanDefinition beanDefinition : beanDefinitions) {
+      String className = beanDefinition.getBeanClassName();
+      Class<?> clazz = ClassUtils.resolveClassName(className,
+          ClassUtils.getDefaultClassLoader());
+
+      try {
+        NotificationDispatcher dispatcher = (NotificationDispatcher) clazz.newInstance();
+        dispatchFactory.register(dispatcher.getType(), dispatcher);
+
+        LOG.info("Binding and registering notification dispatcher {}", clazz);
+      } catch (Exception exception) {
+        LOG.error("Unable to bind and register notification dispatcher {}",
+            clazz, exception);
+      }
+    }
+  }
 }

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java

@@ -42,6 +42,7 @@ import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException;
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.spi.UnsupportedPropertyException;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
+import org.apache.ambari.server.notifications.TargetConfigurationResult;
 import org.apache.ambari.server.notifications.DispatchFactory;
 import org.apache.ambari.server.notifications.NotificationDispatcher;
 import org.apache.ambari.server.orm.dao.AlertDispatchDAO;
@@ -506,8 +507,8 @@ public class AlertTargetResourceProvider extends
     if (dispatcher == null) {
       throw new IllegalArgumentException("Dispatcher for given notification type doesn't exist");
     }
-    NotificationDispatcher.ConfigValidationResult validationResult = dispatcher.validateTargetConfig(properties);
-    if (validationResult.getStatus() == NotificationDispatcher.ConfigValidationResult.Status.INVALID) {
+    TargetConfigurationResult validationResult = dispatcher.validateTargetConfig(properties);
+    if (validationResult.getStatus() == TargetConfigurationResult.Status.INVALID) {
       throw new IllegalArgumentException(validationResult.getMessage());
     }
   }

ambari-server/src/main/java/org/apache/ambari/server/notifications/DispatchFactory.java

@@ -20,12 +20,7 @@ package org.apache.ambari.server.notifications;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.apache.ambari.server.notifications.dispatchers.EmailDispatcher;
-
-import com.google.inject.Inject;
-import com.google.inject.Injector;
 import com.google.inject.Singleton;
-import org.apache.ambari.server.notifications.dispatchers.SNMPDispatcher;
 
 /**
  * The {@link DispatchFactory} is used to provide singleton instances of
@@ -34,6 +29,11 @@ import org.apache.ambari.server.notifications.dispatchers.SNMPDispatcher;
 @Singleton
 public class DispatchFactory {
 
+  /**
+   * Singleton.
+   */
+  private static final DispatchFactory s_instance = new DispatchFactory();
+
   /**
    * Mapping of dispatch type to dispatcher singleton.
    */
@@ -43,12 +43,16 @@ public class DispatchFactory {
    * Constructor.
    *
    */
-  @Inject
-  public DispatchFactory(Injector injector) {
-    EmailDispatcher emailDispatcher = injector.getInstance(EmailDispatcher.class);
-    SNMPDispatcher snmpDispatcher = injector.getInstance(SNMPDispatcher.class);
-    m_dispatchers.put(emailDispatcher.getType(), emailDispatcher);
-    m_dispatchers.put(snmpDispatcher.getType(), snmpDispatcher);
+  private DispatchFactory() {
+  }
+
+  /**
+   * Gets the single instance of this factory.
+   *
+   * @return
+   */
+  public static DispatchFactory getInstance() {
+    return s_instance;
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java

@@ -27,7 +27,7 @@ import java.util.Map;
 public class Notification {
 
   /**
-   *
+   * A short summary of the notification.
    */
   public String Subject;
 

ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java

@@ -59,36 +59,5 @@ public interface NotificationDispatcher {
    * @param properties alert target properties
    * @return ConfigValidationResult with validation status and message
    */
-  public ConfigValidationResult validateTargetConfig(Map<String, Object> properties);
-
-  public static class ConfigValidationResult {
-
-    public enum Status {
-      VALID, INVALID
-    }
-
-    private String message;
-    private Status status;
-
-    private ConfigValidationResult(Status status, String message) {
-      this.message = message;
-      this.status = status;
-    }
-
-    public String getMessage() {
-      return message;
-    }
-
-    public Status getStatus() {
-      return status;
-    }
-
-    public static ConfigValidationResult valid() {
-      return new ConfigValidationResult(Status.VALID, "Configuration is valid");
-    }
-
-    public static ConfigValidationResult invalid(String message) {
-      return new ConfigValidationResult(Status.INVALID, message);
-    }
-  }
+  public TargetConfigurationResult validateTargetConfig(Map<String, Object> properties);
 }

ambari-server/src/main/java/org/apache/ambari/server/notifications/TargetConfigurationResult.java

@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.notifications;
+
+import org.apache.ambari.server.state.alert.AlertTarget;
+
+/**
+ * The {@link TargetConfigurationResult} is used to validate if an
+ * {@link AlertTarget} 's configuration is valid and the dispatcher for its type
+ * can successfully deliver a {@link Notification}.
+ */
+public class TargetConfigurationResult {
+
+  public enum Status {
+    VALID, INVALID
+  }
+
+  private String message;
+  private Status status;
+
+  private TargetConfigurationResult(Status status, String message) {
+    this.message = message;
+    this.status = status;
+  }
+
+  public String getMessage() {
+    return message;
+  }
+
+  public Status getStatus() {
+    return status;
+  }
+
+  public static TargetConfigurationResult valid() {
+    return new TargetConfigurationResult(Status.VALID, "Configuration is valid");
+  }
+
+  public static TargetConfigurationResult invalid(String message) {
+    return new TargetConfigurationResult(Status.INVALID, message);
+  }
+}

ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java

@@ -33,6 +33,7 @@ import javax.mail.Transport;
 import javax.mail.internet.InternetAddress;
 import javax.mail.internet.MimeMessage;
 
+import org.apache.ambari.server.notifications.TargetConfigurationResult;
 import org.apache.ambari.server.notifications.DispatchCredentials;
 import org.apache.ambari.server.notifications.Notification;
 import org.apache.ambari.server.notifications.NotificationDispatcher;
@@ -166,19 +167,19 @@ public class EmailDispatcher implements NotificationDispatcher {
    * {@inheritDoc}
    */
   @Override
-  public ConfigValidationResult validateTargetConfig(Map<String, Object> properties) {
+  public TargetConfigurationResult validateTargetConfig(Map<String, Object> properties) {
     try {
       Transport transport = getMailTransport(properties);
       transport.connect();
       transport.close();
     } catch(AuthenticationFailedException e) {
       LOG.debug("Invalid credentials. Authentication failure.", e);
-      return ConfigValidationResult.invalid("Invalid credentials. Authentication failure: " + e.getMessage());
+      return TargetConfigurationResult.invalid("Invalid credentials. Authentication failure: " + e.getMessage());
     } catch(MessagingException e) {
       LOG.debug("Invalid config.", e);
-      return ConfigValidationResult.invalid("Invalid config: " + e.getMessage());
+      return TargetConfigurationResult.invalid("Invalid config: " + e.getMessage());
     }
-    return ConfigValidationResult.valid();
+    return TargetConfigurationResult.valid();
   }
 
   protected Transport getMailTransport(Map<String, Object> properties) throws NoSuchProviderException {

ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java

@@ -22,6 +22,7 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 
+import org.apache.ambari.server.notifications.TargetConfigurationResult;
 import org.apache.ambari.server.notifications.Notification;
 import org.apache.ambari.server.notifications.NotificationDispatcher;
 import org.apache.ambari.server.notifications.Recipient;
@@ -124,7 +125,7 @@ public class SNMPDispatcher implements NotificationDispatcher {
    * {@inheritDoc}
    */
   @Override
-  public ConfigValidationResult validateTargetConfig(Map<String, Object> properties) {
+  public TargetConfigurationResult validateTargetConfig(Map<String, Object> properties) {
     Map<String, String> stringValuesConfig = new HashMap<String, String>(properties.size());
     for (Map.Entry<String, Object> propertyEntry : properties.entrySet()) {
       stringValuesConfig.put(propertyEntry.getKey(), propertyEntry.getValue().toString());
@@ -155,9 +156,9 @@ public class SNMPDispatcher implements NotificationDispatcher {
           break;
       }
     } catch (InvalidSnmpConfigurationException ex) {
-      return ConfigValidationResult.invalid(ex.getMessage());
+      return TargetConfigurationResult.invalid(ex.getMessage());
     }
-    return ConfigValidationResult.valid();
+    return TargetConfigurationResult.valid();
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/state/NotificationState.java

@@ -39,6 +39,11 @@ public enum NotificationState {
   /**
    * The notification was processed successfully.
    */
-  DELIVERED
+  DELIVERED,
 
+  /**
+   * The notification was picked up and processed by the dispatcher, but there
+   * is no information on whether the delivery was successful.
+   */
+  DISPATCHED;
 }

ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java

@@ -290,6 +290,10 @@ public class AlertNoticeDispatchService extends AbstractScheduledService {
         aggregateMap.put(target, notices);
       }
 
+      // at this point, notices have been processed but not yet delivered
+      notice.setNotifyState(NotificationState.DISPATCHED);
+      notice = m_dao.merge(notice);
+
       notices.add(notice);
     }
 

ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog210.java

@@ -522,24 +522,30 @@ public class UpgradeCatalog210 extends AbstractUpgradeCatalog {
     // stack for each cluster defined
     String INSERT_STACK_ID_TEMPLATE = "UPDATE {0} SET {1} = {2} WHERE cluster_id = {3}";
     ResultSet resultSet = dbAccessor.executeSelect("SELECT * FROM clusters");
-    while (resultSet.next()) {
-      long clusterId = resultSet.getLong("cluster_id");
-      String stackJson = resultSet.getString(DESIRED_STACK_VERSION_COLUMN_NAME);
-      StackId stackId = gson.fromJson(stackJson, StackId.class);
+    try {
+      while (resultSet.next()) {
+        long clusterId = resultSet.getLong("cluster_id");
+        String stackJson = resultSet.getString(DESIRED_STACK_VERSION_COLUMN_NAME);
+        StackId stackId = gson.fromJson(stackJson, StackId.class);
 
-      StackEntity stackEntity = stackDAO.find(stackId.getStackName(),
-          stackId.getStackVersion());
+        StackEntity stackEntity = stackDAO.find(stackId.getStackName(),
+            stackId.getStackVersion());
 
-      String clusterConfigSQL = MessageFormat.format(INSERT_STACK_ID_TEMPLATE,
-          "clusterconfig", STACK_ID_COLUMN_NAME, stackEntity.getStackId(),
-          clusterId);
+        String clusterConfigSQL = MessageFormat.format(
+            INSERT_STACK_ID_TEMPLATE, "clusterconfig", STACK_ID_COLUMN_NAME,
+            stackEntity.getStackId(), clusterId);
 
-      String serviceConfigSQL = MessageFormat.format(INSERT_STACK_ID_TEMPLATE,
-          "serviceconfig", STACK_ID_COLUMN_NAME, stackEntity.getStackId(),
-          clusterId);
+        String serviceConfigSQL = MessageFormat.format(
+            INSERT_STACK_ID_TEMPLATE, "serviceconfig", STACK_ID_COLUMN_NAME,
+            stackEntity.getStackId(), clusterId);
 
-      dbAccessor.executeQuery(clusterConfigSQL);
-      dbAccessor.executeQuery(serviceConfigSQL);
+        dbAccessor.executeQuery(clusterConfigSQL);
+        dbAccessor.executeQuery(serviceConfigSQL);
+      }
+    } finally {
+      if (null != resultSet) {
+        resultSet.close();
+      }
     }
   }
 

ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java

@@ -0,0 +1,73 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.notifications;
+
+import java.io.File;
+import java.util.Properties;
+
+import org.apache.ambari.server.configuration.Configuration;
+import org.apache.ambari.server.controller.ControllerModule;
+import org.apache.ambari.server.notifications.dispatchers.EmailDispatcher;
+import org.apache.ambari.server.notifications.dispatchers.SNMPDispatcher;
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+
+/**
+ * Tests {@link DispatchFactory}.
+ */
+public class DispatchFactoryTest {
+
+  /**
+   * Verify that know {@link NotificationDispatcher}s are registered via the
+   * Guice {@link Module} and that they singletons.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testDispatchFactoryRegistration() throws Exception {
+    String sourceResourceDirectory = "src" + File.separator + "test"
+        + File.separator + "resources";
+
+    Properties properties = new Properties();
+    properties.setProperty(Configuration.SERVER_PERSISTENCE_TYPE_KEY,"in-memory");
+    properties.setProperty(Configuration.OS_VERSION_KEY, "centos6");
+    properties.setProperty(Configuration.SHARED_RESOURCES_DIR_KEY,sourceResourceDirectory);
+
+    Injector injector = Guice.createInjector(new ControllerModule(properties));
+    DispatchFactory dispatchFactory = injector.getInstance(DispatchFactory.class);
+    DispatchFactory dispatchFactory2 = injector.getInstance(DispatchFactory.class);
+
+    // verify singleton
+    Assert.assertEquals(dispatchFactory, dispatchFactory2);
+
+    EmailDispatcher emailDispatcher = injector.getInstance(EmailDispatcher.class);
+    emailDispatcher = (EmailDispatcher) dispatchFactory.getDispatcher(emailDispatcher.getType());
+
+    Assert.assertNotNull(emailDispatcher);
+
+    SNMPDispatcher snmpDispatcher = injector.getInstance(SNMPDispatcher.class);
+    snmpDispatcher = (SNMPDispatcher) dispatchFactory.getDispatcher(snmpDispatcher.getType());
+
+    Assert.assertNotNull(snmpDispatcher);
+  }
+
+}

ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java

@@ -55,7 +55,7 @@ public class MockDispatcher implements NotificationDispatcher {
   }
 
   @Override
-  public ConfigValidationResult validateTargetConfig(Map<String, Object> properties) {
+  public TargetConfigurationResult validateTargetConfig(Map<String, Object> properties) {
     return null;
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java

@@ -125,8 +125,8 @@ public class EmailDispatcherTest {
 
     EasyMock.replay(dispatcher, mockedTransport);
 
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    Assert.assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    Assert.assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -143,8 +143,8 @@ public class EmailDispatcherTest {
 
     EasyMock.replay(dispatcher, mockedTransport);
 
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    Assert.assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    Assert.assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -159,8 +159,8 @@ public class EmailDispatcherTest {
 
     EasyMock.replay(dispatcher, mockedTransport);
 
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    Assert.assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    Assert.assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   /**

ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java

@@ -17,6 +17,7 @@
  */
 package org.apache.ambari.server.notifications.dispatchers;
 
+import org.apache.ambari.server.notifications.TargetConfigurationResult;
 import org.apache.ambari.server.notifications.DispatchCallback;
 import org.apache.ambari.server.notifications.Notification;
 import org.apache.ambari.server.notifications.NotificationDispatcher;
@@ -379,8 +380,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.TRAP_OID_PROPERTY, "1.3.6.1.6.3.1.1.5.4");
     properties.put(SNMPDispatcher.COMMUNITY_PROPERTY, "public");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -393,8 +394,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SNMP_VERSION_PROPERTY, "SNMPv4");
     properties.put(SNMPDispatcher.COMMUNITY_PROPERTY, "public");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -406,8 +407,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SNMP_VERSION_PROPERTY, "SNMPv1");
     properties.put(SNMPDispatcher.COMMUNITY_PROPERTY, "public");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -420,8 +421,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.TRAP_OID_PROPERTY, "1.3.6.1.6.3.1.1.5.4");
     properties.put(SNMPDispatcher.COMMUNITY_PROPERTY, "public");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -433,8 +434,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.TRAP_OID_PROPERTY, "1.3.6.1.6.3.1.1.5.4");
     properties.put(SNMPDispatcher.SNMP_VERSION_PROPERTY, "SNMPv2c");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -450,8 +451,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_PRIV_PASSPHRASE_PROPERTY, "PASSPHRASE2");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "INCORRECT");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -465,8 +466,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_USERNAME_PROPERTY, "USER");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "NOAUTH_NOPRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -481,8 +482,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_AUTH_PASSPHRASE_PROPERTY, "PASSPHRASE1");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "AUTH_NOPRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -496,8 +497,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_USERNAME_PROPERTY, "USER");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "AUTH_NOPRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -513,8 +514,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_PRIV_PASSPHRASE_PROPERTY, "PASSPHRASE2");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "AUTH_PRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.VALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.VALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -528,8 +529,8 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_USERNAME_PROPERTY, "USER");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "AUTH_PRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 
   @Test
@@ -544,7 +545,7 @@ public class SNMPDispatcherTest {
     properties.put(SNMPDispatcher.SECURITY_AUTH_PASSPHRASE_PROPERTY, "PASSPHRASE1");
     properties.put(SNMPDispatcher.SECURITY_LEVEL_PROPERTY, "AUTH_PRIV");
     NotificationDispatcher dispatcher = new SNMPDispatcher();
-    NotificationDispatcher.ConfigValidationResult configValidationResult = dispatcher.validateTargetConfig(properties);
-    assertEquals(NotificationDispatcher.ConfigValidationResult.Status.INVALID, configValidationResult.getStatus());
+    TargetConfigurationResult configValidationResult = dispatcher.validateTargetConfig(properties);
+    assertEquals(TargetConfigurationResult.Status.INVALID, configValidationResult.getStatus());
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java

@@ -18,10 +18,26 @@
 
 package org.apache.ambari.server.serveraction;
 
-import com.google.inject.AbstractModule;
-import com.google.inject.Guice;
-import com.google.inject.Inject;
-import com.google.inject.Injector;
+import static org.easymock.EasyMock.anyBoolean;
+import static org.easymock.EasyMock.anyInt;
+import static org.easymock.EasyMock.anyLong;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.ambari.server.Role;
 import org.apache.ambari.server.RoleCommand;
 import org.apache.ambari.server.actionmanager.ActionDBAccessor;
@@ -32,8 +48,8 @@ import org.apache.ambari.server.actionmanager.RequestStatus;
 import org.apache.ambari.server.actionmanager.Stage;
 import org.apache.ambari.server.actionmanager.StageFactory;
 import org.apache.ambari.server.agent.CommandReport;
-import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.serveraction.upgrades.ManualStageAction;
+import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.svccomphost.ServiceComponentHostServerActionEvent;
 import org.apache.ambari.server.utils.StageUtils;
 import org.easymock.IAnswer;
@@ -43,21 +59,10 @@ import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
-import java.util.*;
-
-import static org.easymock.EasyMock.*;
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-
-import static org.easymock.EasyMock.anyInt;
-import static org.easymock.EasyMock.anyBoolean;
-import static org.easymock.EasyMock.anyLong;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.expect;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
 
 // TODO, fix this test later.
 @Ignore
@@ -113,7 +118,7 @@ public class ServerActionExecutorTest {
     final Request request = createMockRequest();
     stageFactory = createNiceMock(StageFactory.class);
 
-    final Stage stage = stageFactory.createNew((long) 1, "/tmp", "cluster1", (long) 978, "context", CLUSTER_HOST_INFO,
+    final Stage stage = stageFactory.createNew(1, "/tmp", "cluster1", 978, "context", CLUSTER_HOST_INFO,
         "{\"host_param\":\"param_value\"}", "{\"stage_param\":\"param_value\"}");
 
     stage.addServerActionCommand(ManualStageAction.class.getName(),

ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java

@@ -37,6 +37,7 @@ import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.notifications.DispatchFactory;
 import org.apache.ambari.server.notifications.Notification;
 import org.apache.ambari.server.notifications.NotificationDispatcher;
+import org.apache.ambari.server.notifications.TargetConfigurationResult;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.orm.dao.AlertDispatchDAO;
 import org.apache.ambari.server.orm.entities.AlertDefinitionEntity;
@@ -206,9 +207,12 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
   @Test
   public void testDigestDispatch() throws Exception {
     MockEmailDispatcher dispatcher = new MockEmailDispatcher();
+    List<AlertNoticeEntity> notices = getSingleMockNotice(dispatcher.getType());
+    AlertNoticeEntity notice = notices.get(0);
 
-    EasyMock.expect(m_dao.findPendingNotices()).andReturn(getSingleEmailMockNotice()).once();
+    EasyMock.expect(m_dao.findPendingNotices()).andReturn(notices).once();
     EasyMock.expect(m_dispatchFactory.getDispatcher("EMAIL")).andReturn(dispatcher).once();
+    EasyMock.expect(m_dao.merge(notice)).andReturn(notice).atLeastOnce();
 
     EasyMock.replay(m_dao, m_dispatchFactory);
 
@@ -239,7 +243,11 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
   public void testSingleDispatch() throws Exception {
     MockSnmpDispatcher dispatcher = new MockSnmpDispatcher();
 
+    List<AlertNoticeEntity> notices = getSingleMockNotice(dispatcher.getType());
+    AlertNoticeEntity notice = notices.get(0);
+
     EasyMock.expect(m_dao.findPendingNotices()).andReturn(getSnmpMockNotices()).once();
+    EasyMock.expect(m_dao.merge(notice)).andReturn(notice).atLeastOnce();
     EasyMock.expect(m_dispatchFactory.getDispatcher("SNMP")).andReturn(
         dispatcher).atLeastOnce();
 
@@ -268,16 +276,15 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
   @Test
   public void testFailedDispatch() throws Exception {
     MockEmailDispatcher dispatcher = new MockEmailDispatcher();
-    List<AlertNoticeEntity> notices = getSingleEmailMockNotice();
+    List<AlertNoticeEntity> notices = getSingleMockNotice(dispatcher.getType());
     AlertNoticeEntity notice = notices.get(0);
 
     // these expectations happen b/c we need to mark the notice as FAILED
     EasyMock.expect(m_dao.findPendingNotices()).andReturn(notices).once();
+    EasyMock.expect(m_dao.merge(notice)).andReturn(notice).once();
     EasyMock.expect(m_dao.findNoticeByUuid(ALERT_NOTICE_UUID_1)).andReturn(notice).once();
-    EasyMock.expect(m_dao.merge(getSingleEmailMockNotice().get(0))).andReturn(notice).once();
-
-    EasyMock.expect(m_dispatchFactory.getDispatcher("EMAIL")).andReturn(
-        dispatcher).once();
+    EasyMock.expect(m_dao.merge(notice)).andReturn(notice).once();
+    EasyMock.expect(m_dispatchFactory.getDispatcher(dispatcher.getType())).andReturn(dispatcher).once();
 
     EasyMock.replay(m_dao, m_dispatchFactory);
 
@@ -294,27 +301,71 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
     assertNull(notification);
   }
 
+  /**
+   * Tests that when a dispatcher doesn't call back, the
+   * {@link AlertNoticeEntity} will be put from
+   * {@link NotificationState#PENDING} to {@link NotificationState#DISPATCHED}.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testDispatcherWithoutCallbacks() throws Exception {
+    MockNoCallbackDispatcher dispatcher = new MockNoCallbackDispatcher();
+    List<AlertNoticeEntity> notices = getSingleMockNotice(dispatcher.getType());
+    AlertNoticeEntity notice = notices.get(0);
+
+    // these expectations happen b/c we need to mark the notice as FAILED
+    EasyMock.expect(m_dao.findPendingNotices()).andReturn(notices).once();
+    EasyMock.expect(m_dao.merge(notice)).andReturn(notice).atLeastOnce();
+    EasyMock.expect(m_dispatchFactory.getDispatcher(dispatcher.getType())).andReturn(dispatcher).once();
+
+    EasyMock.replay(m_dao, m_dispatchFactory);
+
+    // do NOT startup the service which will force a template NPE
+    AlertNoticeDispatchService service = m_injector.getInstance(AlertNoticeDispatchService.class);
+    service.startUp();
+
+    // service trigger with mock executor that blocks
+    service.setExecutor(new MockExecutor());
+    service.runOneIteration();
+
+    EasyMock.verify(m_dao, m_dispatchFactory);
+
+    Notification notification = dispatcher.getNotification();
+    assertNotNull(notification);
+
+    // the most important part of this test; ensure that notices that are
+    // processed but have no callbacks are in the DISPATCHED state
+    assertEquals(NotificationState.DISPATCHED, notice.getNotifyState());
+  }
+
   /**
    * Gets a single PENDING notice.
    *
    * @return
    */
-  private List<AlertNoticeEntity> getSingleEmailMockNotice() {
+  private List<AlertNoticeEntity> getSingleMockNotice(String notificationType) {
+    AlertDefinitionEntity definition = new AlertDefinitionEntity();
+    definition.setDefinitionId(1L);
+    definition.setDefinitionName("alert-definition-1");
+    definition.setLabel("Alert Definition 1");
+
     AlertHistoryEntity history = new AlertHistoryEntity();
+    history.setAlertDefinition(definition);
     history.setServiceName("HDFS");
     history.setClusterId(1L);
-    history.setAlertDefinition(null);
     history.setAlertLabel("Label");
     history.setAlertState(AlertState.OK);
     history.setAlertText(ALERT_UNIQUE_TEXT);
     history.setAlertTimestamp(System.currentTimeMillis());
 
+
     AlertTargetEntity target = new AlertTargetEntity();
     target.setTargetId(1L);
     target.setAlertStates(EnumSet.allOf(AlertState.class));
     target.setTargetName("Alert Target");
     target.setDescription("Mock Target");
-    target.setNotificationType("EMAIL");
+    target.setNotificationType(notificationType);
 
     String properties = "{ \"foo\" : \"bar\" }";
     target.setProperties(properties);
@@ -421,7 +472,7 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
     }
 
     @Override
-    public ConfigValidationResult validateTargetConfig(Map<String, Object> properties) {
+    public TargetConfigurationResult validateTargetConfig(Map<String, Object> properties) {
       return null;
     }
 
@@ -467,10 +518,53 @@ public class AlertNoticeDispatchServiceTest extends AlertNoticeDispatchService {
     }
 
     @Override
-    public ConfigValidationResult validateTargetConfig(
+    public TargetConfigurationResult validateTargetConfig(
+        Map<String, Object> properties) {
+      return null;
+    }
+  }
+
+  /**
+   * A mock dispatcher that captures the {@link Notification}.
+   */
+  private static final class MockNoCallbackDispatcher implements
+      NotificationDispatcher {
+
+    private Notification m_notificaiton;
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public String getType() {
+      return "NO_CALLBACK";
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public boolean isDigestSupported() {
+      return false;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void dispatch(Notification notification) {
+      m_notificaiton = notification;
+    }
+
+    @Override
+    public TargetConfigurationResult validateTargetConfig(
         Map<String, Object> properties) {
       return null;
     }
+
+    public Notification getNotification() {
+      return m_notificaiton;
+    }
   }
 
   /**