Merge branch 'master' into stable-2.14
diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index ddabe77..0000000
--- a/.travis.yml
+++ /dev/null
@@ -1,61 +0,0 @@
-# Copyright 2014 Criteo
-#
-# 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.
-
-language: java
-
-sudo: false
-
-addons:
- apt:
- pakages:
- - automake
- - autoconf
- - autogen
-
-install:
- - cd ../..
- - mkdir -p build
- - cd build
- - git clone https://gerrit.googlesource.com/buck
- - cd buck
- - ant
- - export PATH=$PATH:$TRAVIS_BUILD_DIR/../../build/buck/bin
- - cd ..
- - git clone -b stable-2.11 https://gerrit.googlesource.com/gerrit
- - cd gerrit
- - ln -s $TRAVIS_BUILD_DIR plugins
- - travis_retry tools/download_all.py > download.log
-
-script:
- - buck clean
- - buck build gerrit > buck.log
- - buck build plugins/automerge-plugin:automerge-plugin >> buck.log
-
-after_failure:
- - cat buck.log
-
-before_deploy:
- - cd $TRAVIS_BUILD_DIR
- - git fetch --tags
-
-deploy:
- provider: releases
- skip_cleanup: true
- api_key:
- secure: Hze64QC7F5SSOVoufSIPtRWb+t6Pzzvx2kJgLtPJYUj87CKOXYzVHVqHuWsa50DPfed9knDnm5nOJawZAxleXG5VxSvBERvSyUNhjNUbDkKson1/TcCl12YM5FcyM2ocsLYe/mc59V3h9LZR2mHDqVj/incxwAIxV/MJIgu5PBg=
- file: ../../build/gerrit/buck-out/gen/plugins/automerge-plugin/automerge-plugin.jar
- on:
- repo: criteo/automerge-plugin
- tags: true
- all_branches: true
diff --git a/BUCK b/BUCK
deleted file mode 100644
index 9a96483..0000000
--- a/BUCK
+++ /dev/null
@@ -1,26 +0,0 @@
-# Copyright 2014 Criteo
-#
-# 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.
-
-include_defs('//bucklets/gerrit_plugin.bucklet')
-
-gerrit_plugin(
- name = 'automerge-plugin',
- srcs = glob(['src/main/java/**/*.java']),
- manifest_entries = [
- 'Gerrit-PluginName: automerge',
- 'Gerrit-Module: com.criteo.gerrit.plugins.automerge.AutomergeModule',
- 'Implementation-Title: Automerge plugin',
- 'Implementation-URL: https://github.com/criteo/automerge-plugin'
- ]
-)
diff --git a/BUILD b/BUILD
new file mode 100644
index 0000000..30b996a
--- /dev/null
+++ b/BUILD
@@ -0,0 +1,11 @@
+load("//tools/bzl:plugin.bzl", "gerrit_plugin")
+
+gerrit_plugin(
+ name = "automerge-plugin",
+ srcs = glob(["src/main/java/**/*.java"]),
+ manifest_entries = [
+ "Gerrit-PluginName: automerge-plugin",
+ "Gerrit-Module: com.criteo.gerrit.plugins.automerge.AutomergeModule",
+ ],
+ resources = glob(["src/main/resources/*"]),
+)
diff --git a/README.md b/README.md
index cace79e..0be924e 100644
--- a/README.md
+++ b/README.md
@@ -1,9 +1,9 @@
automerge-plugin
================
-[](https://travis-ci.org/criteo/automerge-plugin)
+[](https://gerrit-ci.gerritforge.com/view/Plugins-stable-2.14/job/plugin-automerge-plugin-gh-bazel-stable-2.14/)
-A [gerrit](https://code.google.com/p/gerrit/) plugin that takes care of
+A [gerrit](https://www.gerritcodereview.com) plugin that takes care of
automatically merging reviews when all approvals are present.
Also, it introduces the concept of cross-repository reviews.
@@ -11,4 +11,4 @@
in different gerrit repositories. They will be merged at the same time,
when all approvals for all reviews are present, and all reviews are mergeable.
-Requires Gerrit 2.11 or later.
+Requires Gerrit 2.14 or later.
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 e65d35e..00604e4 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
@@ -6,6 +6,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+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;
@@ -73,15 +74,16 @@
* Check if the current patchset of the specified change has dependent
* unmerged changes.
*
+ * @param project
* @param number
* @return true or false
* @throws IOException
* @throws NoSuchChangeException
* @throws OrmException
*/
- public boolean hasDependentReview(final int number) throws IOException, NoSuchChangeException, OrmException {
- final RevisionResource r = getRevisionResource(number);
- final RelatedInfo related = getRelated.apply(r);
+ public boolean hasDependentReview(String project, int number) throws IOException, NoSuchChangeException, OrmException {
+ RevisionResource r = getRevisionResource(project, number);
+ RelatedInfo related = getRelated.apply(r);
log.debug(String.format("Checking for related changes on review %d", number));
return related.changes.size() > 0;
}
@@ -102,16 +104,17 @@
/**
* Check if a change is submitable.
*
+ * @param project a change project
* @param change a change number
* @return true or false
* @throws OrmException
*/
- public boolean isSubmittable(final int change) throws OrmException {
- final ChangeData changeData = changeDataFactory.create(db.get(), new Change.Id(change));
+ public boolean isSubmittable(String project, int change) throws OrmException {
+ ChangeData changeData = changeDataFactory.create(db.get(), new Project.NameKey(project), new 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();
log.debug(String.format("Checking if change %d is submitable.", change));
- for (final SubmitRecord submit : cansubmit) {
+ for (SubmitRecord submit : cansubmit) {
if (submit.status != SubmitRecord.Status.OK) {
log.debug(String.format("Change %d is not submitable", change));
return false;
@@ -133,14 +136,14 @@
public void mergeReview(ChangeInfo info) throws RestApiException, NoSuchChangeException, OrmException, IOException {
final SubmitInput input = new SubmitInput();
input.waitForMerge = true;
- final RevisionResource r = getRevisionResource(info._number);
+ final RevisionResource r = getRevisionResource(info.project, info._number);
submitter.apply(r, input);
}
- public RevisionResource getRevisionResource(int changeNumber) throws NoSuchChangeException, OrmException {
- final ChangeControl ctl = changeFactory.controlFor(new Change.Id(changeNumber), getBotUser());
- final ChangeData changeData = changeDataFactory.create(db.get(), new Change.Id(changeNumber));
- final RevisionResource r = new RevisionResource(collection.parse(ctl), changeData.currentPatchSet());
+ public RevisionResource getRevisionResource(String project, int changeNumber) throws NoSuchChangeException, OrmException {
+ ChangeControl ctl = changeFactory.validateFor(db.get(), new Change.Id(changeNumber), getBotUser());
+ ChangeData changeData = changeDataFactory.create(db.get(), new Project.NameKey(project), new Change.Id(changeNumber));
+ RevisionResource r = new RevisionResource(collection.parse(ctl), changeData.currentPatchSet());
return r;
}
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 b27e7cf..21bf51d 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.change.GetRelated;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.Submit;
+import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ApprovalAttribute;
import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.events.CommentAddedEvent;
@@ -33,6 +34,7 @@
import com.google.gerrit.server.events.PatchSetCreatedEvent;
import com.google.gerrit.server.events.TopicChangedEvent;
import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -98,17 +100,19 @@
}
private void onTopicChanged(final TopicChangedEvent event) {
- if (!atomicityHelper.isAtomicReview(event.change)) {
+ ChangeAttribute change = event.change.get();
+ if (!atomicityHelper.isAtomicReview(change)) {
return;
}
- processNewAtomicPatchSet(event.change);
+ processNewAtomicPatchSet(change);
}
private void onPatchSetCreated(final PatchSetCreatedEvent event) {
- if (!atomicityHelper.isAtomicReview(event.change)) {
+ ChangeAttribute change = event.change.get();
+ if (!atomicityHelper.isAtomicReview(change)) {
return;
}
- processNewAtomicPatchSet(event.change);
+ processNewAtomicPatchSet(change);
}
private void onCommendAdded(final CommentAddedEvent newComment) {
@@ -116,15 +120,14 @@
return;
}
- final ChangeAttribute change = newComment.change;
- final int reviewNumber = Integer.parseInt(change.number);
+ ChangeAttribute change = newComment.change.get();
try {
- checkReviewExists(reviewNumber);
- if (atomicityHelper.isSubmittable(reviewNumber)) {
- log.info(String.format("Change %d is submittable. Will try to merge all related changes.", reviewNumber));
+ checkReviewExists(change.number);
+ if (atomicityHelper.isSubmittable(change.project, change.number)) {
+ log.info(String.format("Change %d is submittable. Will try to merge all related changes.", change.number));
attemptToMerge(change);
}
- } catch (final RestApiException | NoSuchChangeException | OrmException | IOException e) {
+ } catch (RestApiException | OrmException | UpdateException | IOException e) {
log.error("An exception occured while trying to atomic merge a change.", e);
throw new RuntimeException(e);
}
@@ -138,11 +141,13 @@
* @return a boolean
*/
private boolean shouldProcessCommentEvent(CommentAddedEvent comment) {
- if (!config.getBotEmail().equals(comment.author.email)) {
+ AccountAttribute account = comment.author.get();
+ if (!config.getBotEmail().equals(account.email)) {
return true;
}
- if (comment.approvals != null) {
- for (ApprovalAttribute approval : comment.approvals) {
+ ApprovalAttribute[] approvals = comment.approvals.get();
+ if (approvals != null) {
+ for (ApprovalAttribute approval : approvals) {
// See ReviewUpdate#setMinusOne
if (!("Code-Review".equals(approval.type) && "-1".equals(approval.value))) {
return true;
@@ -152,7 +157,7 @@
return false;
}
- private void attemptToMerge(ChangeAttribute change) throws RestApiException, OrmException, NoSuchChangeException, IOException {
+ private void attemptToMerge(ChangeAttribute change) throws RestApiException, OrmException, NoSuchChangeException, IOException, UpdateException {
final List<ChangeInfo> related = Lists.newArrayList();
if (atomicityHelper.isAtomicReview(change)) {
related.addAll(api.changes().query("status: open AND topic: " + change.topic)
@@ -167,37 +172,35 @@
if (!info.mergeable) {
mergeable = false;
}
- if (!atomicityHelper.isSubmittable(info._number)) {
+ if (!atomicityHelper.isSubmittable(info.project, info._number)) {
submittable = false;
}
}
- final int reviewNumber = Integer.parseInt(change.number);
if (submittable) {
if (mergeable) {
- log.debug(String.format("Change %d is mergeable", reviewNumber));
+ log.debug(String.format("Change %d is mergeable", change.number));
for (final ChangeInfo info : related) {
atomicityHelper.mergeReview(info);
}
} else {
- reviewUpdater.commentOnReview(reviewNumber, AutomergeConfig.CANT_MERGE_COMMENT_FILE);
+ reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.CANT_MERGE_COMMENT_FILE);
}
}
}
- private void processNewAtomicPatchSet(final ChangeAttribute change) {
- final int reviewNumber = Integer.parseInt(change.number);
+ private void processNewAtomicPatchSet(ChangeAttribute change) {
try {
- checkReviewExists(reviewNumber);
- if (atomicityHelper.hasDependentReview(reviewNumber)) {
+ checkReviewExists(change.number);
+ if (atomicityHelper.hasDependentReview(change.project, change.number)) {
log.info(String.format("Warn the user by setting -1 on change %d, as other atomic changes exists on the same repository.",
- reviewNumber));
- reviewUpdater.setMinusOne(reviewNumber, AutomergeConfig.ATOMIC_REVIEWS_SAME_REPO_FILE);
+ change.number));
+ reviewUpdater.setMinusOne(change.project, change.number, AutomergeConfig.ATOMIC_REVIEWS_SAME_REPO_FILE);
} else {
- log.info(String.format("Detected atomic review on change %d.", reviewNumber));
- reviewUpdater.commentOnReview(reviewNumber, AutomergeConfig.ATOMIC_REVIEW_DETECTED_FILE);
+ log.info(String.format("Detected atomic review on change %d.", change.number));
+ reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.ATOMIC_REVIEW_DETECTED_FILE);
}
- } catch (RestApiException | IOException | NoSuchChangeException | OrmException e) {
+ } catch (RestApiException | IOException | OrmException | UpdateException e) {
throw new RuntimeException(e);
}
}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
index 135c8df..40d0811 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
@@ -45,7 +45,7 @@
topicPrefix = defaultTopicPrefix;
}
- templatesPath = paths.etc_dir;
+ templatesPath = paths.etc_dir.toFile();
}
public final String getBotEmail() {
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 32c05fe..9ee5e0e 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
@@ -10,6 +10,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -40,14 +41,14 @@
@Inject
private AtomicityHelper atomicityHelper;
- public void commentOnReview(final int number, final String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException {
- final ReviewInput comment = createComment(commentTemplate);
- applyComment(number, comment);
+ public void commentOnReview(String project, int number, String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
+ ReviewInput comment = createComment(commentTemplate);
+ applyComment(project, number, comment);
}
- public void setMinusOne(final int number, final String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException {
- final ReviewInput message = createComment(commentTemplate).label("Code-Review", -1);
- applyComment(number, message);
+ public void setMinusOne(String project, int number, String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
+ ReviewInput message = createComment(commentTemplate).label("Code-Review", -1);
+ applyComment(project, number, message);
}
private ReviewInput createComment(final String commentTemplate) {
@@ -65,8 +66,8 @@
}
}
- private void applyComment(final int number, ReviewInput comment) throws RestApiException, OrmException, IOException, NoSuchChangeException {
- final RevisionResource r = atomicityHelper.getRevisionResource(number);
+ private void applyComment(String project, int number, ReviewInput comment) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
+ RevisionResource r = atomicityHelper.getRevisionResource(project, number);
reviewer.get().apply(r, comment);
}
}
diff --git a/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java b/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
index 3ff50ee..49535db 100644
--- a/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
+++ b/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
@@ -8,16 +8,17 @@
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
-import java.io.File;
import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.nio.file.Paths;
public class AutomergeConfigTest {
@Test
- public void testGetDefaultConfig() {
+ public void testGetDefaultConfig() throws IOException {
final Config conf = new Config();
try {
- final SitePaths paths = new SitePaths(new File("."));
+ final SitePaths paths = new SitePaths(Paths.get("."));
final AutomergeConfig amconf = new AutomergeConfig(conf, paths);
@@ -29,10 +30,10 @@
}
@Test
- public void testGetValues() {
+ public void testGetValues() throws IOException {
final Config conf = new Config();
try {
- final SitePaths paths = new SitePaths(new File("."));
+ final SitePaths paths = new SitePaths(Paths.get("."));
conf.setString(AutomergeConfig.AUTOMERGE_SECTION, null, AutomergeConfig.BOT_EMAIL_KEY, "Foo@bar.com");
conf.setString(AutomergeConfig.AUTOMERGE_SECTION, null, AutomergeConfig.TOPIC_PREFIX_KEY, "fake_topic_prefix");
