Merge branch 'stable-2.14' into HEAD
The test 'test_published_draft_is_autosubmitted' is failing because
the drafts are now 'private' and merged. There is no more need to
'publish' them.
The test should be replaced by a test on the 'wip'
new feature but currently no event is propagated to the EventListener
when pushing '%ready'.
Change-Id: I9622ba642d736ecffd480584525a1f142bb8c14c
diff --git a/README.md b/README.md
index a96c3c5..372bba1 100644
--- a/README.md
+++ b/README.md
@@ -1,7 +1,7 @@
Autosubmitter
=============
-[](https://gerrit-ci.gerritforge.com/view/Plugins-stable-2.14/job/plugin-automerge-plugin-gh-bazel-stable-2.14/)
+[](https://gerrit-ci.gerritforge.com/job/plugin-autosubmitter-bazel-master/)
A [gerrit](https://www.gerritcodereview.com) plugin that takes care of
automatically merging reviews when all approvals are present.
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
index 18471b3..98d67b3 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
@@ -1,23 +1,26 @@
package com.criteo.gerrit.plugins.automerge;
+import static com.google.gerrit.server.permissions.ChangePermission.READ;
+
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.ChangeStatus;
-import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountByEmailCache;
+import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.GetRelated;
import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
import com.google.gerrit.server.change.GetRelated.RelatedInfo;
-import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
-import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
@@ -34,12 +37,8 @@
private static final Logger log = LoggerFactory.getLogger(AtomicityHelper.class);
- @Inject private AccountByEmailCache byEmailCache;
-
@Inject ChangeData.Factory changeDataFactory;
- @Inject private ChangeControl.GenericFactory changeFactory;
-
@Inject private ChangesCollection collection;
@Inject AutomergeConfig config;
@@ -50,12 +49,16 @@
@Inject GetRelated getRelated;
- @Inject MergeUtil.Factory mergeUtilFactory;
-
- @Inject Provider<PostReview> reviewer;
-
@Inject Submit submitter;
+ @Inject Emails emails;
+
+ @Inject ChangeNotes.Factory changeNotesFactory;
+
+ @Inject PermissionBackend permissionBackend;
+
+ @Inject SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
+
/**
* Check if the current patchset of the specified change has dependent unmerged changes.
*
@@ -105,7 +108,7 @@
* Check if a change is an atomic change or not. A change is atomic if it has the atomic topic
* prefix.
*
- * @param change a ChangeAttribute instance
+ * @param change a Change instance
* @return true or false
*/
public boolean isAtomicReview(final Change change) {
@@ -131,7 +134,10 @@
new com.google.gerrit.reviewdb.client.Change.Id(change));
// For draft reviews, the patchSet must be set to avoid an NPE.
final List<SubmitRecord> cansubmit =
- new SubmitRuleEvaluator(changeData).setPatchSet(changeData.currentPatchSet()).evaluate();
+ submitRuleEvaluatorFactory
+ .create(changeData)
+ .setPatchSet(changeData.currentPatchSet())
+ .evaluate();
log.debug(String.format("Checking if change %d is submitable.", change));
for (SubmitRecord submit : cansubmit) {
if (submit.status != SubmitRecord.Status.OK) {
@@ -143,43 +149,37 @@
return true;
}
- /**
- * Merge a review.
- *
- * @param info
- * @throws RestApiException
- * @throws NoSuchChangeException
- * @throws OrmException
- * @throws IOException
- */
- public void mergeReview(String project, int changeNumber)
- throws RestApiException, NoSuchChangeException, OrmException, IOException {
- final SubmitInput input = new SubmitInput();
- input.waitForMerge = true;
- final RevisionResource r = getRevisionResource(project, changeNumber);
- submitter.apply(r, input);
+ /** Merge a review. */
+ public void mergeReview(String project, int number) throws Exception {
+ submitter.apply(getRevisionResource(project, number), new SubmitInput());
}
public RevisionResource getRevisionResource(String project, int changeNumber)
- throws NoSuchChangeException, OrmException {
- ChangeControl ctl =
- changeFactory.validateFor(
- db.get(), new com.google.gerrit.reviewdb.client.Change.Id(changeNumber), getBotUser());
- ChangeData changeData =
- changeDataFactory.create(
- db.get(),
- new Project.NameKey(project),
- new com.google.gerrit.reviewdb.client.Change.Id(changeNumber));
- RevisionResource r = new RevisionResource(collection.parse(ctl), changeData.currentPatchSet());
- return r;
+ throws OrmException {
+ com.google.gerrit.reviewdb.client.Change.Id changeId =
+ new com.google.gerrit.reviewdb.client.Change.Id(changeNumber);
+ ChangeNotes notes = changeNotesFactory.createChecked(changeId);
+ try {
+ permissionBackend.user(getBotUser()).change(notes).database(db).check(READ);
+ ChangeData changeData =
+ changeDataFactory.create(db.get(), new Project.NameKey(project), changeId);
+ RevisionResource r =
+ new RevisionResource(collection.parse(changeId), changeData.currentPatchSet());
+ return r;
+ } catch (ResourceNotFoundException | AuthException | PermissionBackendException e) {
+ throw new NoSuchChangeException(changeId);
+ }
}
private IdentifiedUser getBotUser() {
- final Set<Account.Id> ids = byEmailCache.get(config.getBotEmail());
- if (ids.isEmpty()) {
- throw new RuntimeException("No user found with email: " + config.getBotEmail());
+ try {
+ Set<Account.Id> ids = emails.getAccountFor(config.getBotEmail());
+ if (ids.isEmpty()) {
+ throw new RuntimeException("No user found with email: " + config.getBotEmail());
+ }
+ return factory.create(ids.iterator().next());
+ } catch (IOException | OrmException e) {
+ throw new RuntimeException("Unable to get account with email: " + config.getBotEmail(), e);
}
- final IdentifiedUser bot = factory.create(ids.iterator().next());
- return bot;
}
}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
index 4696b2d..864de7d 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -17,6 +17,7 @@
import com.google.common.collect.Lists;
import com.google.gerrit.common.EventListener;
import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.events.LifecycleListener;
@@ -36,13 +37,9 @@
import com.google.gerrit.server.events.ReviewerDeletedEvent;
import com.google.gerrit.server.events.TopicChangedEvent;
import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.update.UpdateException;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.io.IOException;
import java.util.EnumSet;
import java.util.List;
import org.slf4j.Logger;
@@ -98,7 +95,7 @@
}
try {
autoSubmitIfMergeable(change);
- } catch (OrmException | RestApiException | IOException | UpdateException e) {
+ } catch (Exception e) {
log.error("An exception occured while trying to merge change #" + change.number, e);
}
}
@@ -112,7 +109,7 @@
try {
checkReviewExists(change.number);
autoSubmitIfMergeable(Change.from(change));
- } catch (RestApiException | OrmException | UpdateException | IOException e) {
+ } catch (Exception e) {
log.error("An exception occured while trying to atomic merge a change.", e);
throw new RuntimeException(e);
}
@@ -137,7 +134,7 @@
+ refName
+ ": Submitting ...");
autoSubmitIfMergeable(Change.from(submittable));
- } catch (UpdateException | RestApiException | OrmException | IOException e) {
+ } catch (Exception e) {
log.error(
"Cannot autosubmit change "
+ submittable._number
@@ -154,8 +151,7 @@
}
}
- private void autoSubmitIfMergeable(Change change)
- throws OrmException, RestApiException, NoSuchChangeException, IOException, UpdateException {
+ private void autoSubmitIfMergeable(Change change) throws Exception {
if (atomicityHelper.isSubmittable(change.project, change.number)) {
if (atomicityHelper.isAtomicReview(change)) {
attemptToMergeAtomic(change);
@@ -184,14 +180,19 @@
return false;
}
- private void attemptToMergeAtomic(Change change)
- throws RestApiException, OrmException, NoSuchChangeException, IOException, UpdateException {
+ private void attemptToMergeAtomic(Change change) throws Exception {
final List<ChangeInfo> related = Lists.newArrayList();
- related.addAll(
- api.changes()
- .query("status: open AND topic: " + change.topic)
- .withOption(ListChangesOption.CURRENT_REVISION)
- .get());
+ if (atomicityHelper.isAtomicReview(change)) {
+ related.addAll(
+ api.changes()
+ .query("status: open AND topic: " + change.topic)
+ .withOption(ListChangesOption.CURRENT_REVISION)
+ .get());
+ } else {
+ ChangeApi changeApi = api.changes().id(change.project, change.branch, change.id);
+ related.add(changeApi.get(EnumSet.of(ListChangesOption.CURRENT_REVISION)));
+ }
+
for (final ChangeInfo info : related) {
if (!atomicityHelper.isSubmittable(info.project, info._number)) {
log.info(
@@ -239,7 +240,7 @@
reviewUpdater.commentOnReview(
change.project, change.number, config.atomicReviewsSameRepo.getContent());
}
- } catch (RestApiException | IOException | OrmException | UpdateException e) {
+ } catch (Exception e) {
throw new RuntimeException(e);
}
}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java b/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java
index 3216cca..8728bed 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java
@@ -12,19 +12,29 @@
public class Change {
public final String project;
public final int number;
+ public final String id;
public final String topic;
+ public final String branch;
- private Change(String project, int number, String topic) {
+ private Change(String project, int number, String id, String topic, String branch) {
this.project = project;
this.number = number;
+ this.id = id;
this.topic = topic;
+ this.branch = branch;
}
public static Change from(ChangeAttribute changeAttribute) {
- return new Change(changeAttribute.project, changeAttribute.number, changeAttribute.topic);
+ return new Change(
+ changeAttribute.project,
+ changeAttribute.number,
+ changeAttribute.id,
+ changeAttribute.topic,
+ changeAttribute.branch);
}
public static Change from(ChangeInfo changeInfo) {
- return new Change(changeInfo.project, changeInfo._number, changeInfo.topic);
+ return new Change(
+ changeInfo.project, changeInfo._number, changeInfo.id, changeInfo.topic, changeInfo.branch);
}
}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
index d5f982f..690ebc0 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
@@ -1,14 +1,10 @@
package com.criteo.gerrit.plugins.automerge;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.update.UpdateException;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.io.IOException;
public class ReviewUpdater {
/**
@@ -22,8 +18,7 @@
@Inject private AtomicityHelper atomicityHelper;
- public void commentOnReview(String project, int number, String comment)
- throws RestApiException, OrmException, IOException, UpdateException {
+ public void commentOnReview(String project, int number, String comment) throws Exception {
ReviewInput reviewInput = createComment(comment);
applyComment(project, number, reviewInput);
}
@@ -32,8 +27,7 @@
return new ReviewInput().message(commentsPrefix + comment);
}
- private void applyComment(String project, int number, ReviewInput comment)
- throws RestApiException, OrmException, IOException, UpdateException {
+ private void applyComment(String project, int number, ReviewInput comment) throws Exception {
RevisionResource r = atomicityHelper.getRevisionResource(project, number);
reviewer.get().apply(r, comment);
}
