Fix Guice binding
Bind correct classes to allow guice to instantiate class attributes.
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 f48e000..a2f7d33 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
@@ -21,12 +21,17 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import java.io.IOException;
import java.util.List;
import java.util.Set;
public class AtomicityHelper {
+ private final static Logger log = LoggerFactory.getLogger(AtomicityHelper.class);
+
@Inject
private AccountByEmailCache byEmailCache;
@@ -39,6 +44,7 @@
@Inject
private ChangesCollection collection;
+ @Inject
AutomergeConfig config;
@Inject
@@ -59,10 +65,6 @@
@Inject
Submit submitter;
- public AtomicityHelper(final AutomergeConfig config) {
- this.config = config;
- }
-
/**
* Check if the current patchset of the specified change has dependent
* unmerged changes.
@@ -78,10 +80,9 @@
final IdentifiedUser bot = factory.create(ids.iterator().next());
final ChangeControl ctl = changeFactory.controlFor(new Change.Id(number), bot);
final ChangeData changeData = changeDataFactory.create(db.get(), new Change.Id(number));
-
final RevisionResource r = new RevisionResource(collection.parse(ctl), changeData.currentPatchSet());
final RelatedInfo related = getRelated.apply(r);
-
+ log.debug(String.format("Checking for related changes on review %d", number));
return related.changes.size() > 0;
}
@@ -93,7 +94,9 @@
* @return true or false
*/
public boolean isAtomicReview(final ChangeAttribute change) {
- return change.topic != null && change.topic.startsWith(config.getTopicPrefix());
+ final boolean atomic = change.topic != null && change.topic.startsWith(config.getTopicPrefix());
+ log.debug(String.format("Checking if change %s is an atomic change: %b", change.number, atomic));
+ return atomic;
}
/**
@@ -106,12 +109,14 @@
public boolean isSubmittable(final int change) throws OrmException {
final ChangeData changeData = changeDataFactory.create(db.get(), new Change.Id(change));
final List<SubmitRecord> cansubmit = changeData.changeControl().canSubmit(db.get(), changeData.currentPatchSet());
-
+ log.debug(String.format("Checking if change %d is submitable.", change));
for (final SubmitRecord submit : cansubmit) {
if (submit.status != SubmitRecord.Status.OK) {
+ log.debug(String.format("Change %d is not submitable", change));
return false;
}
}
+ log.debug(String.format("Change %d is submitable", change));
return true;
}
}
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 102b71d..73aa030 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -35,7 +35,6 @@
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.config.GerritServerConfig;
import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.events.ChangeEvent;
import com.google.gerrit.server.events.CommentAddedEvent;
@@ -50,7 +49,8 @@
import com.google.inject.Provider;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.EnumSet;
@@ -63,9 +63,13 @@
*/
public class AutomaticMerger implements ChangeListener, LifecycleListener {
+ private final static Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
+
@Inject
private GerritApi api;
- private final AtomicityHelper atomicityHelper;
+
+ @Inject
+ private AtomicityHelper atomicityHelper;
@Inject
private AccountByEmailCache byEmailCache;
@@ -79,7 +83,8 @@
@Inject
private ChangesCollection collection;
- private final AutomergeConfig config;
+ @Inject
+ private AutomergeConfig config;
@Inject
Provider<ReviewDb> db;
@@ -95,24 +100,22 @@
@Inject
Provider<PostReview> reviewer;
- private final ReviewUpdater reviewUpdater;
+
+ @Inject
+ private ReviewUpdater reviewUpdater;
+
@Inject
Submit submitter;
- @Inject
- public AutomaticMerger(@GerritServerConfig final Config gerritConfig) {
- config = new AutomergeConfig(gerritConfig);
- reviewUpdater = new ReviewUpdater(config);
- atomicityHelper = new AtomicityHelper(config);
- }
-
@Override
synchronized public void onChangeEvent(final ChangeEvent event) {
+ log.info("A change has been recieved");
try {
if (event instanceof TopicChangedEvent) {
final TopicChangedEvent newComment = (TopicChangedEvent) event;
final ChangeAttribute change = newComment.change;
final int reviewNumber = Integer.parseInt(change.number);
+ log.info(String.format("Change on review %d is a topic change.", reviewNumber));
try {
api.changes().id(reviewNumber).get(EnumSet.of(ListChangesOption.CURRENT_REVISION));
} catch (final RestApiException e1) {
@@ -121,8 +124,11 @@
if (atomicityHelper.isAtomicReview(change)) {
if (atomicityHelper.hasDependentReview(reviewNumber)) {
+ log.info(String.format("Setting -2 on change %d, other atomic changes exists on the same repository.",
+ reviewNumber));
reviewUpdater.setMinusTwo(reviewNumber, 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);
}
}
@@ -167,6 +173,7 @@
final String topic = change.topic;
try {
if (atomicityHelper.isSubmittable(Integer.parseInt(newComment.change.number))) {
+ log.info(String.format("Change %d is submittable. Will try to merge all related changes.", reviewNumber));
final List<ChangeInfo> related = Lists.newArrayList();
if (topic != null) {
related.addAll(api.changes().query("status: open AND topic: " + topic)
@@ -189,6 +196,7 @@
}
}
if (mergeable) {
+ log.debug(String.format("Change %d is mergeable", reviewNumber));
for (final ChangeInfo info : related) {
final SubmitInput input = new SubmitInput();
input.waitForMerge = true;
@@ -228,6 +236,7 @@
@Override
public void start() {
+ log.info("Starting automatic merger plugin.");
}
@Override
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 9b84fdc..135c8df 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
@@ -4,7 +4,6 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
-import org.apache.commons.io.FileUtils;
import org.eclipse.jgit.lib.Config;
import java.io.File;
@@ -25,28 +24,31 @@
public static final String getDefaultBotEmail() {
return defaultBotEmail;
-
}
public static final String getDefaultTopicPrefix() {
return defaultTopicPrefix;
}
- private final Config config;
+ private String botEmail;
+ private final File templatesPath;
+ private String topicPrefix;
@Inject
- private SitePaths sitePaths;
+ public AutomergeConfig(@GerritServerConfig final Config config, final SitePaths paths) {
+ botEmail = config.getString(AUTOMERGE_SECTION, null, BOT_EMAIL_KEY);
+ if (botEmail == null) {
+ botEmail = defaultBotEmail;
+ }
+ topicPrefix = config.getString(AUTOMERGE_SECTION, null, TOPIC_PREFIX_KEY);
+ if (topicPrefix == null) {
+ topicPrefix = defaultTopicPrefix;
+ }
-
- public AutomergeConfig(@GerritServerConfig final Config config) {
- this.config = config;
+ templatesPath = paths.etc_dir;
}
public final String getBotEmail() {
- final String botEmail = config.getString(AUTOMERGE_SECTION, null, BOT_EMAIL_KEY);
- if (botEmail == null) {
- return defaultBotEmail;
- }
return botEmail;
}
@@ -57,21 +59,11 @@
* @return a string containing a comment.
* @throws IOException
*/
- public final String getCommentFromFile(final String filename) {
- final File commentFile = new File(sitePaths.etc_dir, filename);
- try {
- return FileUtils.readFileToString(commentFile);
- } catch (final IOException exc) {
- return String.format("Cannot find %s file in gerrit etc dir. Please check your gerrit configuration", filename);
- }
+ public final String getTemplatesPath() {
+ return templatesPath.getPath();
}
public final String getTopicPrefix() {
- final String topicPrefix = config.getString(AUTOMERGE_SECTION, null, TOPIC_PREFIX_KEY);
- if (topicPrefix == null) {
- return defaultTopicPrefix;
- }
return topicPrefix;
}
-
}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/Module.java b/src/main/java/com/criteo/gerrit/plugins/automerge/Module.java
index 8ae8977..32da151 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/Module.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/Module.java
@@ -4,7 +4,7 @@
// 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
+// 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,
@@ -14,9 +14,13 @@
package com.criteo.gerrit.plugins.automerge;
-import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.gerrit.common.ChangeListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
/**
* Main automerge Guice module.
@@ -25,8 +29,22 @@
*/
public class Module extends AbstractModule {
+ protected static final Supplier<Injector> injector = Suppliers.memoize(new Supplier<Injector>() {
+ @Override
+ public Injector get() {
+ return Guice.createInjector(new Module());
+ }
+ });
+
+ public static <T> T getInstance(final Class<T> type) {
+ return injector.get().getInstance(type);
+ }
+
@Override
protected void configure() {
- DynamicSet.bind(binder(), LifecycleListener.class).to(AutomaticMerger.class);
+ DynamicSet.bind(binder(), ChangeListener.class).to(AutomaticMerger.class);
+ bind(AutomergeConfig.class).asEagerSingleton();
+ bind(AtomicityHelper.class);
+ bind(ReviewUpdater.class);
}
}
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 12138f1..9f40f2c 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
@@ -1,5 +1,7 @@
package com.criteo.gerrit.plugins.automerge;
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -19,10 +21,16 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
import java.io.IOException;
import java.util.Set;
public class ReviewUpdater {
+ private final static Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
+
@Inject
private AccountByEmailCache byEmailCache;
@@ -35,7 +43,9 @@
@Inject
private ChangesCollection collection;
- final AutomergeConfig config;
+ @Inject
+ AutomergeConfig config;
+
@Inject
Provider<ReviewDb> db;
@@ -45,14 +55,10 @@
@Inject
Provider<PostReview> reviewer;
- public ReviewUpdater(final AutomergeConfig config) {
- this.config = config;
- }
-
public void commentOnReview(final int number, final String commentTemplate) throws NoSuchChangeException,
OrmException, AuthException, BadRequestException, UnprocessableEntityException, IOException {
final ReviewInput message = new ReviewInput();
- message.message = config.getCommentFromFile(commentTemplate);
+ message.message = getCommentFromFile(commentTemplate);
final Set<Account.Id> ids = byEmailCache.get(config.getBotEmail());
final IdentifiedUser bot = factory.create(ids.iterator().next());
final ChangeControl ctl = changeFactory.controlFor(new Change.Id(number), bot);
@@ -62,10 +68,22 @@
reviewer.get().apply(r, message);
}
+ private final String getCommentFromFile(final String filename) {
+ try {
+ return Files.toString(new File(config.getTemplatesPath(), filename), Charsets.UTF_8);
+
+ } catch (final IOException exc) {
+ final String errmsg =
+ String.format("Cannot find %s file in gerrit etc dir. Please check your gerrit configuration", filename);
+ log.error(errmsg);
+ return errmsg;
+ }
+ }
+
public void setMinusTwo(final int number, final String commentTemplate) throws NoSuchChangeException, OrmException,
AuthException, BadRequestException, UnprocessableEntityException, IOException {
final ReviewInput message = new ReviewInput();
- message.message = config.getCommentFromFile(commentTemplate);
+ message.message = getCommentFromFile(commentTemplate);
message.label("Code-Review", -2);
final Set<Account.Id> ids = byEmailCache.get(config.getBotEmail());
final IdentifiedUser bot = factory.create(ids.iterator().next());
