fix(notifications): remove notification type enum (spinnaker#821)

This is needed for a notification listener extension point so that notification types can be added by a plugin.
pdk27 · Mar 25, 2020 · 9e27efc · 9e27efc
1 parent 5856d63
commit 9e27efc
Show file tree

Hide file tree

Showing 17 changed files with 43 additions and 66 deletions.
diff --git a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/api/Notification.groovy b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/api/Notification.groovy
@@ -21,7 +21,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo
 import groovy.transform.Canonical
 
 class Notification {
-  Type notificationType
+  String notificationType
   Collection<String> to
   Collection<String> cc
   String templateGroup
@@ -41,19 +41,6 @@ class Notification {
     String user
   }
 
-  static enum Type {
-    BEARYCHAT,
-    EMAIL,
-    GITHUB_STATUS,
-    GOOGLECHAT,
-    HIPCHAT, // DEPRECATED/UNUSED; support was removed in commit d175913ab
-    JIRA,
-    PAGER_DUTY,
-    PUBSUB,
-    SMS,
-    SLACK
-  }
-
   static enum Severity {
     NORMAL,
     HIGH

diff --git a/.../src/main/groovy/com/netflix/spinnaker/echo/bearychat/BearychatNotificationService.groovy b/.../src/main/groovy/com/netflix/spinnaker/echo/bearychat/BearychatNotificationService.groovy
@@ -28,7 +28,6 @@ import org.springframework.stereotype.Component
 @Component
 @ConditionalOnProperty('bearychat.enabled')
 class BearychatNotificationService implements NotificationService {
-  private static Notification.Type TYPE = Notification.Type.BEARYCHAT
 
   @Autowired
   BearychatService bearychatService
@@ -40,8 +39,8 @@ class BearychatNotificationService implements NotificationService {
   String token
 
   @Override
-  boolean supportsType(Notification.Type type) {
-    return type == TYPE
+  boolean supportsType(String type) {
+    return "BEARYCHAT".equals(type.toUpperCase())
   }
 
   @Override

diff --git a/...ications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy b/...ications/src/main/groovy/com/netflix/spinnaker/echo/email/EmailNotificationService.groovy
@@ -37,7 +37,6 @@ import javax.mail.internet.MimeMessage
 @Component
 @ConditionalOnProperty('mail.enabled')
 class EmailNotificationService implements NotificationService {
-  private static Notification.Type TYPE = Notification.Type.EMAIL
 
   @Autowired
   JavaMailSender javaMailSender
@@ -49,8 +48,8 @@ class EmailNotificationService implements NotificationService {
   String from
 
   @Override
-  boolean supportsType(Notification.Type type) {
-    return type == TYPE
+  boolean supportsType(String type) {
+    return "EMAIL".equals(type.toUpperCase())
   }
 
   @Override

diff --git a/.../src/main/groovy/com/netflix/spinnaker/echo/googlechat/GoogleChatNotificationService.java b/.../src/main/groovy/com/netflix/spinnaker/echo/googlechat/GoogleChatNotificationService.java
@@ -28,15 +28,14 @@
 @Component
 @ConditionalOnProperty("googlechat.enabled")
 class GoogleChatNotificationService implements NotificationService {
-  private static Notification.Type TYPE = Notification.Type.GOOGLECHAT;
 
   @Autowired GoogleChatService chat;
 
   @Autowired NotificationTemplateEngine notificationTemplateEngine;
 
   @Override
-  public boolean supportsType(Notification.Type type) {
-    return type == TYPE;
+  public boolean supportsType(String type) {
+    return "GOOGLECHAT".equals(type.toUpperCase());
   }
 
   @Override

diff --git a/...otifications/src/main/groovy/com/netflix/spinnaker/echo/jira/JiraNotificationService.java b/...otifications/src/main/groovy/com/netflix/spinnaker/echo/jira/JiraNotificationService.java
@@ -57,8 +57,8 @@ public JiraNotificationService(JiraService jiraService, ObjectMapper objectMappe
   }
 
   @Override
-  public boolean supportsType(Notification.Type type) {
-    return type == Notification.Type.JIRA;
+  public boolean supportsType(String type) {
+    return "JIRA".equals(type.toUpperCase());
   }
 
   @Override

diff --git a/...ovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy b/...ovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy
@@ -109,7 +109,7 @@ class InteractiveNotificationCallbackHandler {
 
   private InteractiveNotificationService getNotificationService(String source) {
     return notificationServices.find { it ->
-      it.supportsType(Notification.Type.valueOf(source.toUpperCase()))
+      it.supportsType(source.toUpperCase())
     }
   }
 

diff --git a/...ations/src/main/groovy/com/netflix/spinnaker/echo/notification/NotificationService.groovy b/...ations/src/main/groovy/com/netflix/spinnaker/echo/notification/NotificationService.groovy
@@ -23,7 +23,7 @@ import org.springframework.http.RequestEntity
 import org.springframework.http.ResponseEntity
 
 interface NotificationService {
-  boolean supportsType(Notification.Type type)
+  boolean supportsType(String type)
   EchoResponse handle(Notification notification)
 }
 

diff --git a/...src/main/groovy/com/netflix/spinnaker/echo/notification/NotificationTemplateEngine.groovy b/...src/main/groovy/com/netflix/spinnaker/echo/notification/NotificationTemplateEngine.groovy
@@ -76,7 +76,7 @@ class NotificationTemplateEngine {
 
     @PackageScope
     @VisibleForTesting
-    static Template determineTemplate(Configuration configuration, String templateGroup, Type type, Notification.Type notificationType) {
+    static Template determineTemplate(Configuration configuration, String templateGroup, Type type, String notificationType) {
         def specificTemplate = specificTemplate(templateGroup, type, notificationType)
         def genericTemplate = genericTemplate(templateGroup, type)
 
@@ -89,8 +89,8 @@ class NotificationTemplateEngine {
 
     @PackageScope
     @VisibleForTesting
-    static String specificTemplate(String templateGroup, Type type, Notification.Type notificationType) {
-        return "${templateGroup}/${type.toString().toLowerCase()}-${notificationType.toString().toLowerCase()}.ftl"
+    static String specificTemplate(String templateGroup, Type type, String notificationType) {
+        return "${templateGroup}/${type.toString().toLowerCase()}-${notificationType.toLowerCase()}.ftl"
     }
 
     @PackageScope

diff --git a/.../src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy b/.../src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy
@@ -40,7 +40,6 @@ import static net.logstash.logback.argument.StructuredArguments.kv
 @Component
 @ConditionalOnProperty('pager-duty.enabled')
 class PagerDutyNotificationService implements NotificationService {
-  private static Notification.Type TYPE = Notification.Type.PAGER_DUTY
 
   @Autowired
   ObjectMapper mapper
@@ -55,8 +54,8 @@ class PagerDutyNotificationService implements NotificationService {
   Front50Service front50Service
 
   @Override
-  boolean supportsType(Notification.Type type) {
-    return type == TYPE
+  boolean supportsType(String type) {
+    return "PAGER_DUTY".equals(type.toUpperCase())
   }
 
   @Override

diff --git a/...ications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy b/...ications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy
@@ -30,7 +30,6 @@ import org.springframework.stereotype.Component
 @Component
 @ConditionalOnProperty('slack.enabled')
 class SlackNotificationService implements NotificationService {
-  protected static Notification.Type TYPE = Notification.Type.SLACK
 
   protected SlackService slack
   protected NotificationTemplateEngine notificationTemplateEngine
@@ -44,8 +43,8 @@ class SlackNotificationService implements NotificationService {
   }
 
   @Override
-  boolean supportsType(Notification.Type type) {
-    return type == TYPE
+  boolean supportsType(String type) {
+    return "SLACK".equals(type.toUpperCase())
   }
 
   @Override

diff --git a/...ations/src/main/groovy/com/netflix/spinnaker/echo/twilio/TwilioNotificationService.groovy b/...ations/src/main/groovy/com/netflix/spinnaker/echo/twilio/TwilioNotificationService.groovy
@@ -28,7 +28,6 @@ import org.springframework.stereotype.Component
 @Component
 @ConditionalOnProperty('twilio.enabled')
 class TwilioNotificationService implements NotificationService {
-  private static Notification.Type TYPE = Notification.Type.SMS
 
   @Autowired
   TwilioService twilioService
@@ -43,8 +42,8 @@ class TwilioNotificationService implements NotificationService {
   String from
 
   @Override
-  boolean supportsType(Notification.Type type) {
-    return type == TYPE
+  boolean supportsType(String type) {
+    return "SMS".equals(type.toUpperCase())
   }
 
   @Override

diff --git a/...src/test/groovy/com/netflix/spinnaker/echo/notification/ManualJudgmentTemplateTest.groovy b/...src/test/groovy/com/netflix/spinnaker/echo/notification/ManualJudgmentTemplateTest.groovy
@@ -94,7 +94,7 @@ class ManualJudgmentTemplateTest extends Specification {
     Notification buildFullNotification() {
         Notification notification = new Notification()
         notification.templateGroup = "manualJudgment"
-        notification.notificationType = Notification.Type.SLACK
+        notification.notificationType = "SLACK"
         notification.source = new Notification.Source()
         notification.source.application = "testapp"
         notification.source.executionId = "exec-id"

diff --git a/...src/test/groovy/com/netflix/spinnaker/echo/notification/NotificationControllerSpec.groovy b/...src/test/groovy/com/netflix/spinnaker/echo/notification/NotificationControllerSpec.groovy
@@ -61,9 +61,9 @@ class NotificationControllerSpec extends Specification {
   void 'creating a notification delegates to the appropriate service'() {
     given:
     Notification notification = new Notification()
-    notification.notificationType = Notification.Type.SLACK
+    notification.notificationType = "SLACK"
 
-    notificationService.supportsType(Notification.Type.SLACK) >> true
+    notificationService.supportsType("SLACK") >> true
 
     when:
     notificationController.create(notification)
@@ -75,7 +75,7 @@ class NotificationControllerSpec extends Specification {
   void 'interactive notifications are only delegated to interactive notification services'() {
     given:
     Notification notification = new Notification()
-    notification.notificationType = Notification.Type.SLACK
+    notification.notificationType = "SLACK"
     notification.interactiveActions = new Notification.InteractiveActions(
       callbackServiceId: "test",
       callbackMessageId: "test",
@@ -87,8 +87,8 @@ class NotificationControllerSpec extends Specification {
       ]
     )
 
-    notificationService.supportsType(Notification.Type.SLACK) >> true
-    interactiveNotificationService.supportsType(Notification.Type.SLACK) >> true
+    notificationService.supportsType("SLACK") >> true
+    interactiveNotificationService.supportsType("SLACK") >> true
 
     when:
     notificationController.create(notification)
@@ -101,10 +101,10 @@ class NotificationControllerSpec extends Specification {
   void 'only interactive notifications are delegated to interactive notification services'() {
     given:
     Notification nonInteractiveNotification = new Notification()
-    nonInteractiveNotification.notificationType = Notification.Type.PAGER_DUTY
+    nonInteractiveNotification.notificationType = "PAGER_DUTY"
 
-    notificationService.supportsType(Notification.Type.PAGER_DUTY) >> true
-    interactiveNotificationService.supportsType(Notification.Type.SLACK) >> true
+    notificationService.supportsType("PAGER_DUTY") >> true
+    interactiveNotificationService.supportsType("SLACK") >> true
 
     when:
     notificationController.create(nonInteractiveNotification)
@@ -123,7 +123,7 @@ class NotificationControllerSpec extends Specification {
     callbackObject.serviceId = "test"
     callbackObject.user = "john.doe"
 
-    interactiveNotificationService.supportsType(Notification.Type.SLACK) >> true
+    interactiveNotificationService.supportsType("SLACK") >> true
     spinnakerService.notificationCallback(*_) >> { mockResponse() }
 
     when:
@@ -143,7 +143,7 @@ class NotificationControllerSpec extends Specification {
     callbackObject.serviceId = "unknown"
     callbackObject.user = "john.doe"
 
-    interactiveNotificationService.supportsType(Notification.Type.SLACK) >> true
+    interactiveNotificationService.supportsType("SLACK") >> true
     interactiveNotificationService.parseInteractionCallback(request) >> callbackObject
 
     when:
@@ -164,7 +164,7 @@ class NotificationControllerSpec extends Specification {
     callbackObject.serviceId = "test"
     callbackObject.user = "john.doe"
 
-    interactiveNotificationService.supportsType(Notification.Type.SLACK) >> true
+    interactiveNotificationService.supportsType("SLACK") >> true
     spinnakerService.notificationCallback(*_) >> { mockResponse() }
 
     when:

diff --git a/...test/groovy/com/netflix/spinnaker/echo/notification/NotificationTemplateEngineSpec.groovy b/...test/groovy/com/netflix/spinnaker/echo/notification/NotificationTemplateEngineSpec.groovy
@@ -49,8 +49,8 @@ class NotificationTemplateEngineSpec extends Specification {
     specificTemplate = Stub(Template)
     genericTemplate = Stub(Template)
     templateGroup | type      | notificationType        | specificTemplateExists
-    "group"       | Type.BODY | Notification.Type.EMAIL | true
-    "group"       | Type.BODY | Notification.Type.EMAIL | false
+    "group"       | Type.BODY | "EMAIL"                 | true
+    "group"       | Type.BODY | "EMAIL"                 | false
   }
 
   @Unroll
@@ -74,14 +74,14 @@ class NotificationTemplateEngineSpec extends Specification {
 
     where:
     templateGroup | type          | notificationType        | content
-    null          | Type.BODY     | Notification.Type.EMAIL | "test"
-    null          | Type.SUBJECT  | Notification.Type.EMAIL | "test"
+    null          | Type.BODY     | "EMAIL"                 | "test"
+    null          | Type.SUBJECT  | "EMAIL"                 | "test"
   }
 
   void "Using a MARKDOWN formatter should leave the original markdown untouched"() {
     given:
     def notification = new Notification(
-      notificationType: Notification.Type.EMAIL,
+      notificationType: "EMAIL",
       to: ["test@netflix.com"],
       severity: Notification.Severity.NORMAL,
       additionalContext: [

diff --git a/...st/groovy/com/netflix/spinnaker/echo/slack/SlackInteractiveNotificationServiceSpec.groovy b/...st/groovy/com/netflix/spinnaker/echo/slack/SlackInteractiveNotificationServiceSpec.groovy
@@ -16,7 +16,6 @@
 
 package com.netflix.spinnaker.echo.slack
 
-import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.echo.api.Notification
 import com.netflix.spinnaker.echo.jackson.EchoObjectMapper
 import com.netflix.spinnaker.echo.notification.NotificationTemplateEngine
@@ -41,7 +40,7 @@ class SlackInteractiveNotificationServiceSpec extends Specification {
 
   def "supports the SLACK notification type"() {
     when:
-    boolean result = service.supportsType(Notification.Type.SLACK)
+    boolean result = service.supportsType("SLACK")
 
     then:
     result == true

diff --git a/...ions/src/test/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationServiceSpec.groovy b/...ions/src/test/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationServiceSpec.groovy
@@ -32,7 +32,7 @@ class SlackNotificationServiceSpec extends Specification {
 
   def "supports the SLACK notification type"() {
     when:
-    boolean result = service.supportsType(Notification.Type.SLACK)
+    boolean result = service.supportsType("SLACK")
 
     then:
     result == true
@@ -41,7 +41,7 @@ class SlackNotificationServiceSpec extends Specification {
   def "handling a notification causes a Slack message to be sent to each channel in the to field"() {
     given:
     Notification notification = new Notification()
-    notification.notificationType = Notification.Type.SLACK
+    notification.notificationType = "SLACK"
     notification.to = [ "channel1", "channel2" ]
     notification.severity = Notification.Severity.NORMAL
     notification.additionalContext["body"] = "text"

diff --git a/.../java/com/netflix/spinnaker/echo/pubsub/google/GooglePubsubNotificationEventListener.java b/.../java/com/netflix/spinnaker/echo/pubsub/google/GooglePubsubNotificationEventListener.java
@@ -18,7 +18,6 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.netflix.spinnaker.echo.api.Notification;
-import com.netflix.spinnaker.echo.api.Notification.Type;
 import com.netflix.spinnaker.echo.api.events.Event;
 import com.netflix.spinnaker.echo.config.GooglePubsubProperties.Content;
 import com.netflix.spinnaker.echo.controller.EchoResponse;
@@ -42,8 +41,6 @@
 public class GooglePubsubNotificationEventListener extends AbstractEventNotificationAgent
     implements NotificationService {
 
-  private static Type TYPE = Type.PUBSUB;
-
   @Autowired private PubsubPublishers publishers;
 
   /**
@@ -91,13 +88,13 @@ public EchoResponse.Void handle(Notification notification) {
   }
 
   @Override
-  public boolean supportsType(Type type) {
-    return type == TYPE;
+  public boolean supportsType(String type) {
+    return getNotificationType().toUpperCase().equals(type.toUpperCase());
   }
 
   @Override
   public String getNotificationType() {
-    return TYPE.toString().toLowerCase();
+    return "pubsub";
   }
 
   private void publishNotification(GooglePubsubPublisher p, Notification notification) {