Update notifications syntax

8 files changed, 155 insertions, 67 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
index 7f63abe72c3..bdc9e41c85d 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
@@ -2,6 +2,7 @@
 package com.yahoo.config.application.api;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
@@ -17,6 +18,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -458,34 +460,101 @@ public class DeploymentSpec {
     /**
      * Configuration of notifications for deployment jobs.
      *
-     * Supports a list of email recipients, and a flag for whether to send to the commit author.
+     * Supports a list of email addresses, and a list of roles for which email addresses are known.
+     * The currently supported roles are:
+     * <ul>
+     *     <li><strong>author</strong>: the author of the git commit of a given application package. </li>
+     * </ul>
      */
     public static class Notifications {
 
-        private static final Notifications none = new Notifications(Collections.emptyList(), false);
+        private static final Notifications none = new Notifications(Collections.emptyMap(), Collections.emptyMap());
         public static Notifications none() { return none; }
 
-        private final List<String> staticEmails;
-        private final boolean includeAuthor;
+        private final Map<String, When> staticEmails;
+        private final Map<Role, When> roleEmails;
 
-        private Notifications(List<String> staticEmails, boolean includeAuthor) {
-            this.staticEmails = ImmutableList.copyOf(staticEmails);
-            this.includeAuthor = includeAuthor;
+        private Notifications(Map<String, When> staticEmails, Map<Role, When> roleEmails) {
+            this.staticEmails = ImmutableMap.copyOf(staticEmails);
+            this.roleEmails = ImmutableMap.copyOf(roleEmails);
         }
 
-        public static Notifications of(List<String> staticEmails, boolean includeAuthor) {
-            if (staticEmails.isEmpty() && ! includeAuthor)
+        /**
+         * 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.
+         * @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())
                 return none;
 
-            return new Notifications(staticEmails, includeAuthor);
+            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))));
         }
 
-        public List<String> staticEmails() {
+        public Map<String, When> staticEmails() {
             return staticEmails;
         }
 
-        public boolean includeAuthor() {
-            return includeAuthor;
+        public Map<Role, When> roleEmails() {
+            return roleEmails;
+        }
+
+
+        public enum Role {
+
+            /** Author of the last commit of an application package. */
+            author;
+
+            public static String toValue(Role role) {
+                switch (role) {
+                    case author: return "author";
+                    default: throw new IllegalArgumentException("Unexpected constant '" + role.name() + "'.");
+                }
+            }
+
+            public static Role fromValue(String value) {
+                switch (value) {
+                    case "author": return author;
+                    default: throw new IllegalArgumentException("Unknown value '" + value + "'.");
+                }
+            }
+
+        }
+
+
+        public enum When {
+
+            /** Send notifications whenever a job fails. */
+            failing,
+
+            /** Send notifications whenever a job fails while deploying a new commit. */
+            failingCommit;
+
+            public static String toValue(When when) {
+                switch (when) {
+                    case failing: return "failing";
+                    case failingCommit: return "failing-commit";
+                    default: throw new IllegalArgumentException("Unexpected constant '" + when.name() + "'.");
+                }
+            }
+
+            public static When fromValue(String value) {
+                switch (value) {
+                    case "failing": return failing;
+                    case "failing-commit": return failingCommit;
+                    default: throw new IllegalArgumentException("Unknown value '" + value + "'.");
+                }
+            }
+
         }
 
     }
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 04f8594d18e..8ee1d05df21 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
@@ -21,7 +21,9 @@ import java.time.Duration;
 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 java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -118,11 +120,20 @@ public class DeploymentSpecXmlReader {
         if (notificationsElement == null)
             return DeploymentSpec.Notifications.none();
 
-        List<String> staticEmails = XML.getChildren(notificationsElement, "email").stream()
-                .map(XML::getValue)
-                .collect(Collectors.toList());
-        boolean includeAuthor = XML.getChild(notificationsElement, "author") != null;
-        return DeploymentSpec.Notifications.of(staticEmails, includeAuthor);
+        Optional<String> when = stringAttribute("when", notificationsElement);
+        Map<String, Optional<String>> staticEmails = new HashMap<>();
+        Map<String, Optional<String>> roleEmails = new HashMap<>();
+
+        for (Element emailElement : XML.getChildren(notificationsElement, "email")) {
+            Optional<String> addressAttribute = stringAttribute("address", emailElement);
+            Optional<String> roleAttribute = stringAttribute("role", emailElement);
+            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)));
+        }
+        return DeploymentSpec.Notifications.of(when, staticEmails, roleEmails);
     }
 
     /** 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 ae1d8bd6d9b..e29ce7f6c39 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
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.application.api;
 
+import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import org.junit.Test;
@@ -11,6 +12,9 @@ import java.time.ZoneId;
 import java.util.Arrays;
 import java.util.Optional;
 
+import static com.yahoo.config.application.api.DeploymentSpec.Notifications.Role.author;
+import static com.yahoo.config.application.api.DeploymentSpec.Notifications.When.failing;
+import static com.yahoo.config.application.api.DeploymentSpec.Notifications.When.failingCommit;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -454,14 +458,16 @@ public class DeploymentSpecTest {
     @Test
     public void someNotifications() {
         DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" +
-                                                     "  <notifications>\n" +
-                                                     "    <author />\n" +
-                                                     "    <email>john@dev</email>\n" +
-                                                     "    <email>jane@dev</email>\n" +
+                                                     "  <notifications when=\"failing\">\n" +
+                                                     "    <email role=\"author\"/>\n" +
+                                                     "    <email address=\"john@dev\" when=\"failing-commit\"/>\n" +
+                                                     "    <email address=\"jane@dev\"/>\n" +
                                                      "  </notifications>\n" +
                                                      "</deployment>");
-        assertTrue(spec.notifications().includeAuthor());
-        assertEquals(Arrays.asList("john@dev", "jane@dev"),
+        assertEquals(ImmutableMap.of(author, failing),
+                     spec.notifications().roleEmails());
+        assertEquals(ImmutableMap.of("john@dev", failingCommit,
+                                     "jane@dev", failing),
                      spec.notifications().staticEmails());
     }
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java
index f833ff9babe..7d1369da864 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java
@@ -4,7 +4,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 
-import java.util.List;
+import java.util.Collection;
 
 /**
  * Used to create mails for different kinds of deployment failure.
@@ -19,38 +19,38 @@ public class DeploymentFailureMails {
         this.registry = registry;
     }
 
-    public Mail outOfCapacity(RunId id, List<String> recipients) {
+    public Mail outOfCapacity(RunId id, Collection<String> recipients) {
         return mail(id, recipients, " due to lack of capacity",
                     "as the zone does not have enough free capacity to " +
                     "accomodate the deployment. Please contact the Vespa team to request more!");
     }
 
-    public Mail deploymentFailure(RunId id, List<String> recipients) {
+    public Mail deploymentFailure(RunId id, Collection<String> recipients) {
         return mail(id, recipients, " deployment",
                     "and any previous deployment in the zone is unaffected. " +
                     "This is usually due to an invalid application configuration, or because " +
                     "of timeouts waiting for other deployments of the same application to finish.");
     }
 
-    public Mail installationFailure(RunId id, List<String> recipients) {
+    public Mail installationFailure(RunId id, Collection<String> recipients) {
         return mail(id, recipients, "installation",
                     "as nodes were not able to start the new Java containers. " +
                     "This is very often due to a misconfiguration of the components of an " +
                     "application, where one or more of these can not be instantiated.");
     }
 
-    public Mail testFailure(RunId id, List<String> recipients) {
+    public Mail testFailure(RunId id, Collection<String> recipients) {
         return mail(id, recipients, "tests",
                     "as one or more verification tests against the deployment failed.");
     }
 
-    public Mail systemError(RunId id, List<String> recipients) {
+    public Mail systemError(RunId id, Collection<String> recipients) {
         return mail(id, recipients, "due to system error",
                     "as something in the framework went wrong. Such errors are " +
                     "usually transient. Please contact the Vespa team if the problem persists!");
     }
 
-    private Mail mail(RunId id, List<String> recipients, String summaryDetail, String messageDetail) {
+    private Mail mail(RunId id, Collection<String> recipients, String summaryDetail, String messageDetail) {
         return new Mail(recipients,
                         String.format("Vespa application %s: %s failing %s",
                                       id.application(),
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java
index a3f492da21f..38b9cd4db5a 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java
@@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.organization;
 
 import com.google.common.collect.ImmutableList;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 
@@ -12,11 +13,11 @@ import java.util.Objects;
  */
 public class Mail {
 
-    private final List<String> recipients;
+    private final Collection<String> recipients;
     private final String subject;
     private final String message;
 
-    public Mail(List<String> recipients, String subject, String message) {
+    public Mail(Collection<String> recipients, String subject, String message) {
         if (recipients.isEmpty())
             throw new IllegalArgumentException("Empty recipient list is not allowed.");
         recipients.forEach(Objects::requireNonNull);
@@ -25,7 +26,7 @@ public class Mail {
         this.message = Objects.requireNonNull(message);
     }
 
-    public List<String> recipients() { return recipients; }
+    public Collection<String> recipients() { return recipients; }
     public String subject() { return subject; }
     public String message() { return message; }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 56c041a1e26..2463d8d8298 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -29,7 +29,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentFailureMails;
-import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
@@ -46,7 +45,7 @@ import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 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 java.util.Optional;
@@ -436,12 +435,8 @@ public class InternalStepRunner implements StepRunner {
                                                    run.hasFailed() ? Optional.of(DeploymentJobs.JobError.unknown) : Optional.empty());
                 controller.applications().deploymentTrigger().notifyOfCompletion(report);
 
-                Application application = controller.applications().require(id.application());
-                boolean newCommit = application.change().application()
-                                               .map(run.versions().targetApplication()::equals)
-                                               .orElse(false);
-                if (run.hasFailed() && newCommit)
-                    sendNotification(run, application.deploymentSpec().notifications(), logger);
+                if (run.hasFailed())
+                    sendNotification(run, logger);
             });
         }
         catch (IllegalStateException e) {
@@ -451,23 +446,33 @@ public class InternalStepRunner implements StepRunner {
     }
 
     /** Sends a mail with a notification of a failed run, if one should be sent. */
-    private void sendNotification(Run run, Notifications notifications, DualLogger logger) {
+    private void sendNotification(Run run, DualLogger logger) {
+        Application application = controller.applications().require(run.id().application());
+        Notifications notifications = application.deploymentSpec().notifications();
         if (notifications != Notifications.none()) {
-            try {
-                List<String> recipients = new ArrayList<>(notifications.staticEmails());
-                if (notifications.includeAuthor())
-                    run.versions().targetApplication().authorEmail().ifPresent(recipients::add);
+            boolean newCommit = application.change().application()
+                                           .map(run.versions().targetApplication()::equals)
+                                           .orElse(false);
+
+            Map<String, Notifications.When> recipients = new HashMap<>(notifications.staticEmails());
+            if (notifications.roleEmails().containsKey(Notifications.Role.author))
+                run.versions().targetApplication().authorEmail()
+                   .ifPresent(address -> recipients.put(address, notifications.roleEmails().get(Notifications.Role.author)));
 
+            if ( ! newCommit)
+                recipients.values().removeIf(Notifications.When.failingCommit::equals);
+
+            try {
                 if (run.status() == outOfCapacity && run.id().type().isProduction())
-                    controller.mailer().send(mails.outOfCapacity(run.id(), recipients));
+                    controller.mailer().send(mails.outOfCapacity(run.id(), recipients.keySet()));
                 if (run.status() == deploymentFailed)
-                    controller.mailer().send(mails.deploymentFailure(run.id(), recipients));
+                    controller.mailer().send(mails.deploymentFailure(run.id(), recipients.keySet()));
                 if (run.status() == installationFailed)
-                    controller.mailer().send(mails.installationFailure(run.id(), recipients));
+                    controller.mailer().send(mails.installationFailure(run.id(), recipients.keySet()));
                 if (run.status() == testFailure)
-                    controller.mailer().send(mails.testFailure(run.id(), recipients));
+                    controller.mailer().send(mails.testFailure(run.id(), recipients.keySet()));
                 if (run.status() == error)
-                    controller.mailer().send(mails.systemError(run.id(), recipients));
+                    controller.mailer().send(mails.systemError(run.id(), recipients.keySet()));
             }
             catch (RuntimeException e) {
                 logger.log(INFO, "Exception trying to send mail for " + run.id(), e);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
index 09c8b4ac62c..0499a92d05f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
@@ -20,6 +20,7 @@ import java.util.Date;
 import java.util.List;
 import java.util.Optional;
 import java.util.StringJoiner;
+import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
@@ -33,6 +34,9 @@ public class ApplicationPackageBuilder {
     private final StringBuilder environmentBody = new StringBuilder();
     private final StringBuilder validationOverridesBody = new StringBuilder();
     private final StringBuilder blockChange = new StringBuilder();
+    private final StringJoiner notifications = new StringJoiner("/>\n  <email ",
+                                                                "<notifications>\n  <email ",
+                                                                "/>\n</notifications>\n").setEmptyValue("");
 
     private Optional<Integer> majorVersion = Optional.empty();
     private String upgradePolicy = null;
@@ -40,7 +44,6 @@ public class ApplicationPackageBuilder {
     private String globalServiceId = null;
     private String athenzIdentityAttributes = null;
     private String searchDefinition = "search test { }";
-    private DeploymentSpec.Notifications notifications = DeploymentSpec.Notifications.none();
 
     public ApplicationPackageBuilder majorVersion(int majorVersion) {
         this.majorVersion = Optional.of(majorVersion);
@@ -110,13 +113,13 @@ public class ApplicationPackageBuilder {
         return this;
     }
 
-    public ApplicationPackageBuilder notifyAuthor(boolean notify) {
-        this.notifications = DeploymentSpec.Notifications.of(this.notifications.staticEmails(), notify);
+    public ApplicationPackageBuilder emailRole(String role) {
+        this.notifications.add("role=\"" + role + "\"");
         return this;
     }
 
-    public ApplicationPackageBuilder notifyEmails(List<String> emails) {
-        this.notifications = DeploymentSpec.Notifications.of(emails, this.notifications.includeAuthor());
+    public ApplicationPackageBuilder emailAddress(String address) {
+        this.notifications.add("address=\"" + address + "\"");
         return this;
     }
 
@@ -139,14 +142,7 @@ public class ApplicationPackageBuilder {
             xml.append(upgradePolicy);
             xml.append("'/>\n");
         }
-        if (notifications != DeploymentSpec.Notifications.none()) {
-            xml.append("<notifications>\n");
-            if (notifications.includeAuthor())
-                xml.append("  <author />\n");
-            for (String email : notifications.staticEmails())
-                xml.append("  <email>").append(email).append("</email>\n");
-            xml.append("</notifications>\n");
-        }
+        xml.append(notifications);
         xml.append(blockChange);
         xml.append("  <");
         xml.append(environment.value());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index 523c43a485b..ebda3482a50 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -43,8 +43,8 @@ public class InternalDeploymentTester {
             .upgradePolicy("default")
             .region("us-central-1")
             .parallel("us-west-1", "us-east-3")
-            .notifyAuthor(true)
-            .notifyEmails(Collections.singletonList("b@a"))
+            .emailRole("author")
+            .emailAddress("b@a")
             .build();
     public static final ApplicationId appId = ApplicationId.from("tenant", "application", "default");
     public static final TesterId testerId = TesterId.of(appId);