Support notify settings for rerunning checks
This way bots rerunning checks can control who is notified when the
combined check state changes due to rerunning a check.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ib9c42e5707c3a10e73e7930c5bbefd7ac065f0ab
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckApi.java b/java/com/google/gerrit/plugins/checks/api/CheckApi.java
index 14acb41..ef42bda 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckApi.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckApi.java
@@ -27,7 +27,12 @@
CheckInfo update(CheckInput input) throws RestApiException;
/** Reruns the check and returns the {@link CheckInfo} for the updated check. */
- CheckInfo rerun() throws RestApiException;
+ default CheckInfo rerun() throws RestApiException {
+ return rerun(new RerunInput());
+ }
+
+ /** Reruns the check and returns the {@link CheckInfo} for the updated check. */
+ CheckInfo rerun(RerunInput input) throws RestApiException;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -44,7 +49,7 @@
}
@Override
- public CheckInfo rerun() throws RestApiException {
+ public CheckInfo rerun(RerunInput input) throws RestApiException {
throw new NotImplementedException();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java b/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
index 3be0a74..856eff0 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
@@ -64,9 +64,9 @@
}
@Override
- public CheckInfo rerun() throws RestApiException {
+ public CheckInfo rerun(RerunInput input) throws RestApiException {
try {
- return rerunCheck.apply(checkResource, null).value();
+ return rerunCheck.apply(checkResource, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot rerun check", e);
}
diff --git a/java/com/google/gerrit/plugins/checks/api/RerunCheck.java b/java/com/google/gerrit/plugins/checks/api/RerunCheck.java
index 487742f..273e0f8 100644
--- a/java/com/google/gerrit/plugins/checks/api/RerunCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/RerunCheck.java
@@ -14,7 +14,6 @@
package com.google.gerrit.plugins.checks.api;
-import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -42,7 +41,7 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
-public class RerunCheck implements RestModifyView<CheckResource, Input> {
+public class RerunCheck implements RestModifyView<CheckResource, RerunInput> {
private final Provider<CurrentUser> self;
private final Checks checks;
private final Provider<ChecksUpdate> checksUpdate;
@@ -64,7 +63,7 @@
}
@Override
- public Response<CheckInfo> apply(CheckResource checkResource, Input input)
+ public Response<CheckInfo> apply(CheckResource checkResource, RerunInput input)
throws RestApiException, IOException, PermissionBackendException, ConfigInvalidException {
if (!self.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
@@ -72,6 +71,9 @@
if (checkResource.getRevisionResource().getEdit().isPresent()) {
throw new ResourceConflictException("checks are not supported on a change edit");
}
+ if (input == null) {
+ input = new RerunInput();
+ }
CheckKey key =
CheckKey.create(
checkResource.getRevisionResource().getProject(),
@@ -103,7 +105,8 @@
.unsetStarted()
.setMessage("")
.setUrl("");
- updatedCheck = checksUpdate.get().updateCheck(key, builder.build(), null, null);
+ updatedCheck =
+ checksUpdate.get().updateCheck(key, builder.build(), input.notify, input.notifyDetails);
}
return Response.ok(checkJsonFactory.noOptions().format(updatedCheck));
}
diff --git a/java/com/google/gerrit/plugins/checks/api/RerunInput.java b/java/com/google/gerrit/plugins/checks/api/RerunInput.java
new file mode 100644
index 0000000..95b6dbe
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/api/RerunInput.java
@@ -0,0 +1,56 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed 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 com.google.gerrit.plugins.checks.api;
+
+import com.google.common.base.MoreObjects;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.NotifyInfo;
+import com.google.gerrit.extensions.api.changes.RecipientType;
+import java.util.Map;
+import java.util.Objects;
+
+public class RerunInput {
+ /**
+ * Whom to send email notifications to when the combined check state changes due to rerunning this
+ * check.
+ */
+ @Nullable public NotifyHandling notify;
+ /** Additional information about whom to notify regardless of the {@link #notify} setting. */
+ @Nullable public Map<RecipientType, NotifyInfo> notifyDetails;
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof RerunInput)) {
+ return false;
+ }
+ RerunInput other = (RerunInput) o;
+ return Objects.equals(other.notify, notify)
+ && Objects.equals(other.notifyDetails, notifyDetails);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(notify, notifyDetails);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("notify", notify)
+ .add("notifyDetails", notifyDetails)
+ .toString();
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
index 922754d..9667978 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -40,6 +40,7 @@
import com.google.gerrit.plugins.checks.api.CheckInput;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.plugins.checks.api.RerunInput;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.testing.FakeEmailSender.Message;
@@ -351,6 +352,69 @@
assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
}
+ @Test
+ public void rerunCheckRespectsNotifySettings() throws Exception {
+ testNotifySettingsForRerunCheck(NotifyHandling.ALL, owner, reviewer, starrer, watcher);
+ testNotifySettingsForRerunCheck(NotifyHandling.OWNER, owner);
+ testNotifySettingsForRerunCheck(NotifyHandling.OWNER_REVIEWERS, owner, reviewer);
+ testNotifySettingsForRerunCheck(NotifyHandling.NONE);
+
+ testNotifySettingsForRerunCheck(
+ ImmutableSet.of(user), NotifyHandling.ALL, user, owner, reviewer, starrer, watcher);
+ testNotifySettingsForRerunCheck(ImmutableSet.of(user), NotifyHandling.OWNER, user, owner);
+ testNotifySettingsForRerunCheck(
+ ImmutableSet.of(user), NotifyHandling.OWNER_REVIEWERS, user, owner, reviewer);
+ testNotifySettingsForRerunCheck(ImmutableSet.of(user), NotifyHandling.NONE, user);
+ }
+
+ private void testNotifySettingsForRerunCheck(
+ NotifyHandling notify, TestAccount... expectedRecipients) throws RestApiException {
+ testNotifySettingsForPostCheck(ImmutableSet.of(), notify, expectedRecipients);
+ }
+
+ private void testNotifySettingsForRerunCheck(
+ Set<TestAccount> accountsToNotify, NotifyHandling notify, TestAccount... expectedRecipients)
+ throws RestApiException {
+ // Create a check that sets the combined check state to FAILED.
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.check(checkKey).forUpdate().state(CheckState.FAILED).upsert();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ sender.clear();
+
+ // Post a new check that changes the combined check state to FAILED.
+ requestScopeOperations.setApiUser(bot.id());
+ RerunInput rerunInput = new RerunInput();
+ if (!accountsToNotify.isEmpty()) {
+ rerunInput.notifyDetails =
+ ImmutableMap.of(
+ RecipientType.TO,
+ new NotifyInfo(
+ accountsToNotify.stream().map(TestAccount::username).collect(toImmutableList())));
+ }
+ rerunInput.notify = notify;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun(rerunInput);
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ List<Message> messages = sender.getMessages();
+ if (expectedRecipients.length == 0) {
+ assertThat(messages).isEmpty();
+ } else {
+ assertThat(messages).hasSize(1);
+
+ Message message = messages.get(0);
+ assertThat(message.from().getName()).isEqualTo(bot.fullName() + " (Code Review)");
+ assertThat(message.body())
+ .contains(
+ "The combined check state has been updated to " + CombinedCheckState.IN_PROGRESS);
+ assertThat(message.rcpt())
+ .containsExactlyElementsIn(
+ Arrays.stream(expectedRecipients)
+ .map(TestAccount::getEmailAddress)
+ .collect(toImmutableList()));
+ }
+ }
+
private CombinedCheckState getCombinedCheckState() throws RestApiException {
ChangeInfo changeInfo =
gApi.changes()
diff --git a/resources/Documentation/rest-api-checks.md b/resources/Documentation/rest-api-checks.md
index 94346d5..bbab437 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -164,6 +164,8 @@
Reruns a check. As response the [CheckInfo](#check-info) entity is returned that
describes the created check.
+Notification options may be specified as [RerunInput](#rerun-input) entity in
+the request body.
This REST endpoint supports rerunning a check. It also resets all relevant check
fields such as `message`, `url`, `started` and `finished`.
@@ -205,6 +207,14 @@
| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to posting this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `ALL`.
| `notifyDetails`| optional | Additional information about whom to notify when the combined check state changes due to posting this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity.
+### <a id="rerun-input"> RerunInput
+The `RerunInput` entity contains information for rerunning a check.
+
+| Field Name | | Description |
+| --------------- | -------- | ----------- |
+| `notify` | optional | Notify handling that defines to whom email notifications should be sent when the combined check state changes due to rerunning this check. Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. If not set, the default is `ALL`.
+| `notifyDetails`| optional | Additional information about whom to notify when the combined check state changes due to rerunning this check as a map of recipient type to [NotifyInfo](../../../Documentation/rest-api-changes.html#notify-info) entity.
+
### <a id="check-state"> CheckState (enum)
The `CheckState` enum can have the following values: `NOT_STARTED`, `FAILED`,
`SCHEDULED`, `RUNNING`, `SUCCESSFUL` and `NOT_RELEVANT`.
