Merge "Send email notification if combined check state is updated"
diff --git a/java/com/google/gerrit/plugins/checks/ChecksStorageUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksStorageUpdate.java
new file mode 100644
index 0000000..bdf8693
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/ChecksStorageUpdate.java
@@ -0,0 +1,47 @@
+// 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;
+
+import com.google.gerrit.exceptions.DuplicateKeyException;
+import java.io.IOException;
+
+/**
+ * API for updating checks in the storage backend.
+ *
+ * <p>To be implemented by check storage backends.
+ */
+public interface ChecksStorageUpdate {
+ /**
+ * Creates a new check in the storage backend.
+ *
+ * @param key the key of the check that should be created
+ * @param checkUpdate the update describing the properties of the new check
+ * @return the newly created check
+ * @throws DuplicateKeyException thrown if a check with the given key already exists
+ * @throws IOException thrown in case of an I/O error
+ */
+ public Check createCheck(CheckKey key, CheckUpdate checkUpdate)
+ throws DuplicateKeyException, IOException;
+
+ /**
+ * Updates an existing check in the storage backend.
+ *
+ * @param key the key of the check that should be updated
+ * @param checkUpdate the update describing the check properties that should be updated
+ * @return the updated check
+ * @throws IOException thrown in case of an I/O error
+ */
+ public Check updateCheck(CheckKey key, CheckUpdate checkUpdate) throws IOException;
+}
diff --git a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
index ba58afa..f29d079 100644
--- a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
@@ -14,12 +14,127 @@
package com.google.gerrit.plugins.checks;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.DuplicateKeyException;
+import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.plugins.checks.email.CombinedCheckStateUpdatedSender;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ServerInitiated;
+import com.google.gerrit.server.UserInitiated;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
+import java.util.Optional;
-public interface ChecksUpdate {
- Check createCheck(CheckKey key, CheckUpdate checkUpdate)
- throws DuplicateKeyException, IOException;
+/**
+ * API to update checks.
+ *
+ * <p>Delegates the persistence of checks to the storage layer (see {@link ChecksStorageUpdate}).
+ *
+ * <p>This class contains additional business logic for updating checks which is independent of the
+ * used storage layer (e.g. sending email notifications).
+ */
+public class ChecksUpdate {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- Check updateCheck(CheckKey key, CheckUpdate checkUpdate) throws IOException;
+ interface Factory {
+ ChecksUpdate create(IdentifiedUser currentUser);
+
+ ChecksUpdate createWithServerIdent();
+ }
+
+ private final CombinedCheckStateCache combinedCheckStateCache;
+ private final CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory;
+ private final ChangeNotes.Factory notesFactory;
+ private final PatchSetUtil psUtil;
+ private final Optional<IdentifiedUser> currentUser;
+ private final ChecksStorageUpdate checksStorageUpdate;
+
+ @AssistedInject
+ ChecksUpdate(
+ @UserInitiated ChecksStorageUpdate checksStorageUpdate,
+ CombinedCheckStateCache combinedCheckStateCache,
+ CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
+ ChangeNotes.Factory notesFactory,
+ PatchSetUtil psUtil,
+ @Assisted IdentifiedUser currentUser) {
+ this.combinedCheckStateCache = combinedCheckStateCache;
+ this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
+ this.notesFactory = notesFactory;
+ this.psUtil = psUtil;
+ this.currentUser = Optional.of(currentUser);
+ this.checksStorageUpdate = checksStorageUpdate;
+ }
+
+ @AssistedInject
+ ChecksUpdate(
+ @ServerInitiated ChecksStorageUpdate checksStorageUpdate,
+ CombinedCheckStateCache combinedCheckStateCache,
+ CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
+ ChangeNotes.Factory notesFactory,
+ PatchSetUtil psUtil) {
+ this.combinedCheckStateCache = combinedCheckStateCache;
+ this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
+ this.notesFactory = notesFactory;
+ this.psUtil = psUtil;
+ this.currentUser = Optional.empty();
+ this.checksStorageUpdate = checksStorageUpdate;
+ }
+
+ public Check createCheck(CheckKey key, CheckUpdate checkUpdate)
+ throws DuplicateKeyException, IOException {
+ CombinedCheckState oldCombinedCheckState =
+ combinedCheckStateCache.get(key.repository(), key.patchSet());
+
+ Check check = checksStorageUpdate.createCheck(key, checkUpdate);
+
+ CombinedCheckState newCombinedCheckState =
+ combinedCheckStateCache.get(key.repository(), key.patchSet());
+ if (oldCombinedCheckState != newCombinedCheckState) {
+ sendEmail(key, newCombinedCheckState);
+ }
+
+ return check;
+ }
+
+ public Check updateCheck(CheckKey key, CheckUpdate checkUpdate) throws IOException {
+ CombinedCheckState oldCombinedCheckState =
+ combinedCheckStateCache.get(key.repository(), key.patchSet());
+
+ Check check = checksStorageUpdate.updateCheck(key, checkUpdate);
+
+ CombinedCheckState newCombinedCheckState =
+ combinedCheckStateCache.get(key.repository(), key.patchSet());
+ if (oldCombinedCheckState != newCombinedCheckState) {
+ sendEmail(key, newCombinedCheckState);
+ }
+
+ return check;
+ }
+
+ private void sendEmail(CheckKey checkKey, CombinedCheckState combinedCheckState) {
+ try {
+ CombinedCheckStateUpdatedSender sender =
+ combinedCheckStateUpdatedSenderFactory.create(
+ checkKey.repository(), checkKey.patchSet().changeId());
+
+ if (currentUser.isPresent()) {
+ sender.setFrom(currentUser.get().getAccountId());
+ }
+
+ ChangeNotes changeNotes =
+ notesFactory.create(checkKey.repository(), checkKey.patchSet().changeId());
+ PatchSet patchSet = psUtil.get(changeNotes, checkKey.patchSet());
+ sender.setPatchSet(patchSet);
+
+ sender.setCombinedCheckState(combinedCheckState);
+ sender.send();
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot email update for change %s", checkKey.patchSet().changeId());
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/checks/Module.java b/java/com/google/gerrit/plugins/checks/Module.java
index 140497b..3431ec1 100644
--- a/java/com/google/gerrit/plugins/checks/Module.java
+++ b/java/com/google/gerrit/plugins/checks/Module.java
@@ -25,8 +25,12 @@
import com.google.gerrit.plugins.checks.api.ChangeCheckAttributeFactory.GetChangeOptions;
import com.google.gerrit.plugins.checks.api.ChangeCheckAttributeFactory.QueryChangesOptions;
import com.google.gerrit.plugins.checks.db.NoteDbCheckersModule;
+import com.google.gerrit.plugins.checks.email.ChecksEmailModule;
import com.google.gerrit.plugins.checks.rules.ChecksSubmitRule;
import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.ServerInitiated;
+import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.validators.CommitValidationListener;
@@ -35,11 +39,13 @@
import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.gerrit.sshd.commands.Query;
+import com.google.inject.Provides;
public class Module extends FactoryModule {
@Override
protected void configure() {
factory(CheckJson.AssistedFactory.class);
+ factory(ChecksUpdate.Factory.class);
install(new NoteDbCheckersModule());
install(CombinedCheckStateCache.module());
@@ -74,5 +80,19 @@
install(new ApiModule());
install(new ChecksSubmitRule.Module());
+ install(new ChecksEmailModule());
+ }
+
+ @Provides
+ @ServerInitiated
+ ChecksUpdate provideServerInitiatedChecksUpdate(ChecksUpdate.Factory factory) {
+ return factory.createWithServerIdent();
+ }
+
+ @Provides
+ @UserInitiated
+ ChecksUpdate provideUserInitiatedChecksUpdate(
+ ChecksUpdate.Factory factory, IdentifiedUser currentUser) {
+ return factory.create(currentUser);
}
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersModule.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersModule.java
index 39e8d7c..efc278b 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersModule.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheckersModule.java
@@ -18,7 +18,7 @@
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.CheckersUpdate;
import com.google.gerrit.plugins.checks.Checks;
-import com.google.gerrit.plugins.checks.ChecksUpdate;
+import com.google.gerrit.plugins.checks.ChecksStorageUpdate;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.UserInitiated;
@@ -51,13 +51,13 @@
@Provides
@ServerInitiated
- ChecksUpdate provideServerInitiatedChecksUpdate(NoteDbChecksUpdate.Factory factory) {
+ ChecksStorageUpdate provideServerInitiatedChecksUpdate(NoteDbChecksUpdate.Factory factory) {
return factory.createWithServerIdent();
}
@Provides
@UserInitiated
- ChecksUpdate provideUserInitiatedChecksUpdate(
+ ChecksStorageUpdate provideUserInitiatedChecksUpdate(
NoteDbChecksUpdate.Factory factory, IdentifiedUser currentUser) {
return factory.create(currentUser);
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
index 980a1fe..467a7bd 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
@@ -29,7 +29,7 @@
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checkers;
-import com.google.gerrit.plugins.checks.ChecksUpdate;
+import com.google.gerrit.plugins.checks.ChecksStorageUpdate;
import com.google.gerrit.plugins.checks.CombinedCheckStateCache;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -56,7 +56,7 @@
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevWalk;
-public class NoteDbChecksUpdate implements ChecksUpdate {
+public class NoteDbChecksUpdate implements ChecksStorageUpdate {
interface Factory {
NoteDbChecksUpdate create(IdentifiedUser currentUser);
diff --git a/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java b/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java
new file mode 100644
index 0000000..3b4e016
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java
@@ -0,0 +1,32 @@
+// 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.email;
+
+import static com.google.inject.Scopes.SINGLETON;
+
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.mail.send.MailSoyTemplateProvider;
+
+public class ChecksEmailModule extends FactoryModule {
+ @Override
+ protected void configure() {
+ DynamicSet.bind(binder(), MailSoyTemplateProvider.class)
+ .to(ChecksMailSoyTemplateProvider.class)
+ .in(SINGLETON);
+
+ factory(CombinedCheckStateUpdatedSender.Factory.class);
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/email/ChecksMailSoyTemplateProvider.java b/java/com/google/gerrit/plugins/checks/email/ChecksMailSoyTemplateProvider.java
new file mode 100644
index 0000000..6bcfe08
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/email/ChecksMailSoyTemplateProvider.java
@@ -0,0 +1,33 @@
+// 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.email;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.server.mail.send.MailSoyTemplateProvider;
+import com.google.inject.Singleton;
+import java.util.Set;
+
+@Singleton
+public class ChecksMailSoyTemplateProvider implements MailSoyTemplateProvider {
+ @Override
+ public String getPath() {
+ return "com/google/gerrit/plugins/checks/email/";
+ }
+
+ @Override
+ public Set<String> getFileNames() {
+ return ImmutableSet.of("CombinedCheckStateUpdated.soy", "CombinedCheckStateUpdatedHtml.soy");
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
new file mode 100644
index 0000000..560e97b
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
@@ -0,0 +1,78 @@
+// 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.email;
+
+import com.google.gerrit.exceptions.EmailException;
+import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.account.ProjectWatches.NotifyType;
+import com.google.gerrit.server.mail.send.ChangeEmail;
+import com.google.gerrit.server.mail.send.EmailArguments;
+import com.google.gerrit.server.mail.send.ReplyToChangeSender;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+/** Send notice about an update of the combined check state of a change. */
+public class CombinedCheckStateUpdatedSender extends ReplyToChangeSender {
+ public interface Factory extends ReplyToChangeSender.Factory<CombinedCheckStateUpdatedSender> {
+ @Override
+ CombinedCheckStateUpdatedSender create(Project.NameKey project, Change.Id changeId);
+ }
+
+ private CombinedCheckState combinedCheckState;
+
+ @Inject
+ public CombinedCheckStateUpdatedSender(
+ EmailArguments args, @Assisted Project.NameKey project, @Assisted Change.Id changeId) {
+ super(args, "combinedCheckStateUpdate", ChangeEmail.newChangeData(args, project, changeId));
+ }
+
+ @Override
+ protected void init() throws EmailException {
+ super.init();
+
+ ccAllApprovals();
+ bccStarredBy();
+ includeWatchers(NotifyType.ALL_COMMENTS);
+ removeUsersThatIgnoredTheChange();
+ }
+
+ public void setCombinedCheckState(CombinedCheckState combinedCheckState) {
+ this.combinedCheckState = combinedCheckState;
+ }
+
+ @Override
+ protected void setupSoyContext() {
+ super.setupSoyContext();
+
+ if (combinedCheckState != null) {
+ soyContext.put("combinedCheckState", combinedCheckState.name());
+ }
+ }
+
+ @Override
+ protected void formatChange() throws EmailException {
+ appendText(textTemplate("CombinedCheckStateUpdated"));
+ if (useHtml()) {
+ appendHtml(soyHtmlTemplate("CombinedCheckStateUpdatedHtml"));
+ }
+ }
+
+ @Override
+ protected boolean supportsHtml() {
+ return true;
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
index a00c2cd..7bbf3ee 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
@@ -186,14 +186,14 @@
checksApiFactory.revision(psId).id(checkerUuid).update(checkInput);
// Incurs reload after updating check state.
- assertThat(cache.getStats()).since(start).hasHitCount(2);
+ assertThat(cache.getStats()).since(start).hasHitCount(4);
assertThat(cache.getStats()).since(start).hasMissCount(0);
assertThat(cache.getReloadCount(false) - startReloadsFalse).isEqualTo(0);
assertThat(cache.getReloadCount(true) - startReloadsTrue).isEqualTo(1);
assertThat(queryChangeCheckInfo(changeId))
.hasValue(new ChangeCheckInfo("checks", CombinedCheckState.WARNING));
- assertThat(cache.getStats()).since(start).hasHitCount(3);
+ assertThat(cache.getStats()).since(start).hasHitCount(5);
assertThat(cache.getStats()).since(start).hasMissCount(0);
assertThat(cache.getReloadCount(false) - startReloadsFalse).isEqualTo(0);
assertThat(cache.getReloadCount(true) - startReloadsTrue).isEqualTo(1);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
new file mode 100644
index 0000000..8723614
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChecksEmailIT.java
@@ -0,0 +1,241 @@
+// 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.acceptance.api;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.client.ProjectWatchInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.plugins.checks.CheckKey;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import com.google.gerrit.plugins.checks.api.ChangeCheckInfo;
+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.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.inject.Inject;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ChecksEmailIT extends AbstractCheckersTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private GroupOperations groupOperations;
+ @Inject private ProjectOperations projectOperations;
+
+ private TestAccount bot;
+ private CheckerUuid checkerUuid1;
+ private CheckerUuid checkerUuid2;
+ private TestAccount owner;
+ private TestAccount reviewer;
+ private TestAccount starrer;
+ private TestAccount watcher;
+ private PatchSet.Id patchSetId;
+
+ @Before
+ public void setup() throws Exception {
+ // Create a bot account, create a bots group and add the bot as member and allow the bots group
+ // to post checks.
+ bot = accountCreator.create("bot", "bot@test.com", "Bot");
+ AccountGroup.UUID botsAccountGroupUuid =
+ groupOperations.newGroup().name("bots").addMember(bot.id()).create();
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability("checks-administrateCheckers").group(botsAccountGroupUuid))
+ .update();
+
+ // Create two required checkers.
+ checkerUuid1 = checkerOperations.newChecker().repository(project).required().create();
+ checkerUuid2 = checkerOperations.newChecker().repository(project).required().create();
+
+ // Create a change.
+ owner = admin;
+ patchSetId = createChange().getPatchSetId();
+
+ // Add a reviewer.
+ reviewer = accountCreator.create("reviewer", "reviewer@test.com", "Reviewer");
+ gApi.changes().id(patchSetId.changeId().get()).addReviewer(reviewer.username());
+
+ // Star the change from some user.
+ starrer = accountCreator.create("starred", "starrer@test.com", "Starrer");
+ requestScopeOperations.setApiUser(starrer.id());
+ gApi.accounts().self().starChange(patchSetId.changeId().toString());
+
+ // Watch all comments of change from some user.
+ watcher = accountCreator.create("watcher", "watcher@test.com", "Watcher");
+ requestScopeOperations.setApiUser(watcher.id());
+ ProjectWatchInfo projectWatchInfo = new ProjectWatchInfo();
+ projectWatchInfo.project = project.get();
+ projectWatchInfo.filter = "*";
+ projectWatchInfo.notifyAllComments = true;
+ gApi.accounts().self().setWatchedProjects(ImmutableList.of(projectWatchInfo));
+
+ // Watch only change creations from some user --> user doesn't get notified by checks plugin.
+ TestAccount changeCreationWatcher =
+ accountCreator.create(
+ "changeCreationWatcher", "changeCreationWatcher@test.com", "Change Creation Watcher");
+ requestScopeOperations.setApiUser(changeCreationWatcher.id());
+ projectWatchInfo = new ProjectWatchInfo();
+ projectWatchInfo.project = project.get();
+ projectWatchInfo.filter = "*";
+ projectWatchInfo.notifyNewChanges = true;
+ gApi.accounts().self().setWatchedProjects(ImmutableList.of(projectWatchInfo));
+
+ // Add a reviewer that ignores the change --> user doesn't get notified by checks plugin.
+ TestAccount ignorer = accountCreator.create("ignorer", "ignorer@test.com", "Ignorer");
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.changes().id(patchSetId.changeId().get()).addReviewer(ignorer.username());
+ requestScopeOperations.setApiUser(ignorer.id());
+ gApi.changes().id(patchSetId.changeId().get()).ignore(true);
+
+ // Reset request scope to admin.
+ requestScopeOperations.setApiUser(admin.id());
+ }
+
+ @Test
+ public void combinedCheckUpdatedEmailAfterCheckCreation() throws Exception {
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ sender.clear();
+
+ // Post a new check that changes the combined check state to FAILED.
+ requestScopeOperations.setApiUser(bot.id());
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid1.get();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ // Except one email because the combined check state was updated.
+ List<Message> messages = sender.getMessages();
+ 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.FAILED);
+ assertThat(message.rcpt())
+ .containsExactly(
+ owner.getEmailAddress(),
+ reviewer.getEmailAddress(),
+ starrer.getEmailAddress(),
+ watcher.getEmailAddress());
+ }
+
+ @Test
+ public void noCombinedCheckUpdatedEmailOnCheckCreationIfCombinedCheckStateIsNotChanged()
+ throws Exception {
+ // Create a check that sets the combined check state to FAILED.
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.newCheck(checkKey).state(CheckState.FAILED).upsert();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ sender.clear();
+
+ // Post a new check that doesn't change the combined check state..
+ requestScopeOperations.setApiUser(bot.id());
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid2.get();
+ input.state = CheckState.SCHEDULED;
+ checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ // Except that no email was sent because the combined check state was not updated.
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void combinedCheckUpdatedEmailAfterCheckUpdate() throws Exception {
+ // Create a check that sets the combined check state to FAILED.
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.newCheck(checkKey).state(CheckState.FAILED).upsert();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ sender.clear();
+
+ // Update the new check so that the combined check state is changed to IN_PROGRESS.
+ requestScopeOperations.setApiUser(bot.id());
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid1.get();
+ input.state = CheckState.RUNNING;
+ checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.IN_PROGRESS);
+
+ // Except one email because the combined check state was updated.
+ List<Message> messages = sender.getMessages();
+ 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())
+ .containsExactly(
+ owner.getEmailAddress(),
+ reviewer.getEmailAddress(),
+ starrer.getEmailAddress(),
+ watcher.getEmailAddress());
+ }
+
+ @Test
+ public void noCombinedCheckUpdatedEmailOnCheckUpdateIfCombinedCheckStateIsNotChanged()
+ throws Exception {
+ // Create 2 checks that set the combined check state to FAILED.
+ CheckKey checkKey1 = CheckKey.create(project, patchSetId, checkerUuid1);
+ checkOperations.newCheck(checkKey1).state(CheckState.FAILED).upsert();
+ CheckKey checkKey2 = CheckKey.create(project, patchSetId, checkerUuid2);
+ checkOperations.newCheck(checkKey2).state(CheckState.FAILED).upsert();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ sender.clear();
+
+ // Update one of the checks in a way so that doesn't change the combined check state..
+ requestScopeOperations.setApiUser(bot.id());
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid2.get();
+ input.state = CheckState.SCHEDULED;
+ checksApiFactory.revision(patchSetId).create(input).get();
+ assertThat(getCombinedCheckState()).isEqualTo(CombinedCheckState.FAILED);
+
+ // Except that no email was sent because the combined check state was not updated.
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ private CombinedCheckState getCombinedCheckState() throws RestApiException {
+ ChangeInfo changeInfo =
+ gApi.changes()
+ .id(patchSetId.changeId().get())
+ .get(ImmutableListMultimap.of("checks--combined", "true"));
+ ImmutableList<PluginDefinedInfo> infos =
+ changeInfo.plugins.stream().filter(i -> i.name.equals("checks")).collect(toImmutableList());
+ assertThat(infos).hasSize(1);
+ assertThat(infos.get(0)).isInstanceOf(ChangeCheckInfo.class);
+ return ((ChangeCheckInfo) infos.get(0)).combinedState;
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
index 6458099..1948adb 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
@@ -41,7 +41,6 @@
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Optional;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
@@ -91,22 +90,27 @@
CheckKey.create(
Project.nameKey("non-existing"), createChange().getPatchSetId(), checkerUuid);
- IllegalStateException thrown =
- assertThrows(
- IllegalStateException.class, () -> checkOperations.newCheck(checkKey).upsert());
- assertThat(thrown.getCause(), instanceOf(RepositoryNotFoundException.class));
+ assertThrows(IllegalStateException.class, () -> checkOperations.newCheck(checkKey).upsert());
+ }
+
+ @Test
+ public void checkCannotBeCreatedForNonExistingChange() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+ CheckKey checkKey = CheckKey.create(project, PatchSet.id(Change.id(1), 1), checkerUuid);
+
+ assertThrows(IllegalStateException.class, () -> checkOperations.newCheck(checkKey).upsert());
}
@Test
public void checkCannotBeCreatedForNonExistingPatchSet() throws Exception {
+ Change.Id changeId = createChange().getChange().getId();
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- CheckKey checkKey = CheckKey.create(project, PatchSet.id(Change.id(1), 1), checkerUuid);
+ CheckKey checkKey = CheckKey.create(project, PatchSet.id(changeId, 99), checkerUuid);
IllegalStateException thrown =
assertThrows(
IllegalStateException.class, () -> checkOperations.newCheck(checkKey).upsert());
- assertThat(thrown.getCause(), instanceOf(IOException.class));
- assertThat(thrown).hasMessageThat().contains("patchset 1,1 not found");
+ assertThat(thrown).hasMessageThat().contains("patch set not found: " + changeId.get() + ",99");
}
@Test
diff --git a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
new file mode 100644
index 0000000..83d67b1
--- /dev/null
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdated.soy
@@ -0,0 +1,41 @@
+/**
+ * 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.
+ */
+
+{namespace com.google.gerrit.server.mail.template}
+
+/**
+ * The .CombinedCheckStateUpdated template will determine the contents of the email related to a
+ * change for which the combined check state was updated.
+ */
+{template .CombinedCheckStateUpdated kind="text"}
+ {@param change: ?}
+ {@param combinedCheckState: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param patchSet: ?}
+ The combined check state has been updated to {$combinedCheckState} for patch set
+ {$patchSet.patchSetId} of this change.
+ {if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
+ {\n}
+ Change subject: {$change.subject}{\n}
+ ......................................................................{\n}
+ {if $coverLetter}
+ {\n}
+ {\n}
+ {$coverLetter}
+ {\n}
+ {/if}
+{/template}
diff --git a/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
new file mode 100644
index 0000000..8e7e5ff
--- /dev/null
+++ b/resources/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedHtml.soy
@@ -0,0 +1,42 @@
+/**
+ * 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.
+ */
+
+{namespace com.google.gerrit.server.mail.template}
+
+/**
+ * The .CombinedCheckStateUpdatedHtml template will determine the contents of the email related to
+ * a change for which the combined check state was updated.
+ */
+{template .CombinedCheckStateUpdatedHtml}
+ {@param combinedCheckState: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param patchSet: ?}
+ <p>
+ The combined check state has been updated to <strong>{$combinedCheckState}</strong> for patch
+ set {$patchSet.patchSetId} of this change.
+ </p>
+
+ {if $email.changeUrl}
+ <p>
+ {call .ViewChangeButton data="all" /}
+ </p>
+ {/if}
+
+ {if $coverLetter}
+ <div style="white-space:pre-wrap">{$coverLetter}</div>
+ {/if}
+{/template}
