Better API?

3 files changed, 64 insertions, 36 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java
index 0375d99550a..dfd64fd951b 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java
@@ -1,11 +1,17 @@
 package com.yahoo.config.application.api;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
-import java.util.Optional;
-import java.util.stream.Collectors;
+import java.util.Set;
+
+import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toMap;
 
 /**
  * Configuration of notifications for deployment jobs.
@@ -23,41 +29,50 @@ public class Notifications {
     private static final Notifications none = new Notifications(Collections.emptyMap(), Collections.emptyMap());
     public static Notifications none() { return none; }
 
-    private final Map<String, When> staticEmails;
-    private final Map<Role, When> roleEmails;
+    private final Map<When, List<String>> emailAddresses;
+    private final Map<When, List<Role>> emailRoles;
 
-    private Notifications(Map<String, When> staticEmails, Map<Role, When> roleEmails) {
-        this.staticEmails = ImmutableMap.copyOf(staticEmails);
-        this.roleEmails = ImmutableMap.copyOf(roleEmails);
+    private Notifications(Map<When, List<String>> emailAddresses, Map<When, List<Role>> emailRoles) {
+        this.emailAddresses = emailAddresses;
+        this.emailRoles = emailRoles;
     }
 
     /**
      * Returns a new Notifications as specified by the given String input.
      *
-     * @param when Optional string name of the default condition for sending notifications; defaults to failingCommit.
-     * @param staticEmails Map from email addresses to optional overrides for when to send to these.
-     * @param roleEmails Map from email roles to optional overrides for when to send to these.
+     * @param emailAddressesByWhen What email addresses to notify, indexed by when to notify them.
+     * @param emailRolesByWhen What roles to infer email addresses for, and notify, indexed by when to notify them.
      * @return The Notifications as specified.
      */
-    public static Notifications of(Optional<String> when, Map<String, Optional<String>> staticEmails, Map<String, Optional<String>> roleEmails) {
-        if (staticEmails.isEmpty() && roleEmails.isEmpty())
+    public static Notifications of(Map<When, List<String>> emailAddressesByWhen, Map<When, List<Role>> emailRolesByWhen) {
+        if (emailAddressesByWhen.isEmpty() && emailRolesByWhen.isEmpty())
             return none;
 
-        When defaultWhen = when.map(When::fromValue).orElse(When.failingCommit);
-        return new Notifications(staticEmails.entrySet().stream()
-                                             .collect(Collectors.toMap(entry -> entry.getKey(),
-                                                                       entry -> entry.getValue().map(When::fromValue).orElse(defaultWhen))),
-                                 roleEmails.entrySet().stream()
-                                           .collect(Collectors.toMap(entry -> Role.fromValue(entry.getKey()),
-                                                                     entry -> entry.getValue().map(When::fromValue).orElse(defaultWhen))));
+        ImmutableMap.Builder<When, List<String>> emailAddresses = ImmutableMap.builder();
+        emailAddressesByWhen.forEach((when, addresses) -> emailAddresses.put(when, ImmutableList.copyOf(addresses)));
+
+        ImmutableMap.Builder<When, List<Role>> emailRoles = ImmutableMap.builder();
+        emailRolesByWhen.forEach((when, roles) -> emailRoles.put(when, ImmutableList.copyOf(roles)));
+
+        return new Notifications(emailAddresses.build(), emailRoles.build());
     }
 
-    public Map<String, When> staticEmails() {
-        return staticEmails;
+    /** Returns all email addresses to notify for the given condition. */
+    public Set<String> emailAddressesFor(When when) {
+        ImmutableSet.Builder<String> addresses = ImmutableSet.builder();
+        addresses.addAll(emailAddresses.getOrDefault(when, emptyList()));
+        for (When include : when.includes)
+            addresses.addAll(emailAddressesFor(include));
+        return addresses.build();
     }
 
-    public Map<Role, When> roleEmails() {
-        return roleEmails;
+    /** Returns all roles for which email notification is to be sent for the given condition. */
+    public Set<Role> emailRolesFor(When when) {
+        ImmutableSet.Builder<Role> roles = ImmutableSet.builder();
+        roles.addAll(emailRoles.getOrDefault(when, emptyList()));
+        for (When include : when.includes)
+            roles.addAll(emailRolesFor(include));
+        return roles.build();
     }
 
 
@@ -89,7 +104,13 @@ public class Notifications {
         failing,
 
         /** Send notifications whenever a job fails while deploying a new commit. */
-        failingCommit;
+        failingCommit(failing);
+
+        private final List<When> includes;
+
+        When(When... includes) {
+            this.includes = Arrays.asList(includes);
+        }
 
         public static String toValue(When when) {
             switch (when) {
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
index 140e4dafe32..970abd971af 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
@@ -7,6 +7,8 @@ import com.yahoo.config.application.api.DeploymentSpec.Delay;
 import com.yahoo.config.application.api.DeploymentSpec.ParallelZones;
 import com.yahoo.config.application.api.DeploymentSpec.Step;
 import com.yahoo.config.application.api.Notifications;
+import com.yahoo.config.application.api.Notifications.Role;
+import com.yahoo.config.application.api.Notifications.When;
 import com.yahoo.config.application.api.TimeWindow;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
@@ -121,20 +123,25 @@ public class DeploymentSpecXmlReader {
         if (notificationsElement == null)
             return Notifications.none();
 
-        Optional<String> when = stringAttribute("when", notificationsElement);
-        Map<String, Optional<String>> staticEmails = new HashMap<>();
-        Map<String, Optional<String>> roleEmails = new HashMap<>();
+        When defaultWhen = stringAttribute("when", notificationsElement).map(When::fromValue).orElse(When.failingCommit);
+        Map<When, List<String>> emailAddresses = new HashMap<>();
+        Map<When, List<Role>> emailRoles = new HashMap<>();
+        for (When when : When.values()) {
+            emailAddresses.put(when, new ArrayList<>());
+            emailRoles.putIfAbsent(when, new ArrayList<>());
+        }
 
         for (Element emailElement : XML.getChildren(notificationsElement, "email")) {
             Optional<String> addressAttribute = stringAttribute("address", emailElement);
-            Optional<String> roleAttribute = stringAttribute("role", emailElement);
+            Optional<Role> roleAttribute = stringAttribute("role", emailElement).map(Role::fromValue);
+            When when = stringAttribute("when", emailElement).map(When::fromValue).orElse(defaultWhen);
             if (addressAttribute.isPresent() == roleAttribute.isPresent())
                 throw new IllegalArgumentException("Exactly one of 'role' and 'address' must be present in 'email' elements.");
 
-            addressAttribute.ifPresent(address -> staticEmails.put(address, stringAttribute("when", emailElement)));
-            roleAttribute.ifPresent(role -> roleEmails.put(role, stringAttribute("when", emailElement)));
+            addressAttribute.ifPresent(address -> emailAddresses.get(when).add(address));
+            roleAttribute.ifPresent(role -> emailRoles.get(when).add(role));
         }
-        return Notifications.of(when, staticEmails, roleEmails);
+        return Notifications.of(emailAddresses, emailRoles);
     }
 
     /** Imposes some constraints on tag order which are not expressible in the schema */
diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
index b2ecf5368e9..b40feb3081a 100644
--- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
+++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.config.application.api;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import org.junit.Test;
@@ -463,11 +464,10 @@ public class DeploymentSpecTest {
                                                      "    <email address=\"jane@dev\"/>\n" +
                                                      "  </notifications>\n" +
                                                      "</deployment>");
-        assertEquals(ImmutableMap.of(author, failing),
-                     spec.notifications().roleEmails());
-        assertEquals(ImmutableMap.of("john@dev", failingCommit,
-                                     "jane@dev", failing),
-                     spec.notifications().staticEmails());
+        assertEquals(ImmutableSet.of(author), spec.notifications().emailRolesFor(failing));
+        assertEquals(ImmutableSet.of(author), spec.notifications().emailRolesFor(failingCommit));
+        assertEquals(ImmutableSet.of("john@dev", "jane@dev"), spec.notifications().emailAddressesFor(failingCommit));
+        assertEquals(ImmutableSet.of("jane@dev"), spec.notifications().emailAddressesFor(failing));
     }
 
 }