Cleanup classes Remove the complicated constructor SimpleSubmitRulesModule, it is error-prone and Guice might not be entirely ready at this point. Remove #getConfig, which was only used once from NoUnresolvedCommentsRule. Directly call the right method. Qualify sub-classes imports. Change-Id: Ic5a6a4875861279ae237ca6438adf5fd7694a29e

diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/SimpleSubmitRulesModule.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/SimpleSubmitRulesModule.java
index 821f794..874f6d5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/SimpleSubmitRulesModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/SimpleSubmitRulesModule.java

@@ -14,42 +14,19 @@
 
 package com.googlesource.gerrit.plugins.simplesubmitrules;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.extensions.annotations.PluginName;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.server.config.PluginConfig;
-import com.google.gerrit.server.config.PluginConfigFactory;
-import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.rules.SubmitRule;
 import com.google.inject.AbstractModule;
-import com.google.inject.Inject;
 import com.googlesource.gerrit.plugins.simplesubmitrules.config.ConfigServlet;
 import com.googlesource.gerrit.plugins.simplesubmitrules.config.ConfigTranslator;
 import com.googlesource.gerrit.plugins.simplesubmitrules.rules.NoUnresolvedCommentsRule;
 import com.googlesource.gerrit.plugins.simplesubmitrules.rules.RequireNonAuthorApprovalRule;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** Bootstraps the Simple Submit Rules plugin */
 public class SimpleSubmitRulesModule extends AbstractModule {
-  private static final Logger log = LoggerFactory.getLogger(SimpleSubmitRulesModule.class);
-
-  /** Name of this plugin, as defined in the BUILD file */
-  @VisibleForTesting protected static final String PLUGIN_NAME = "simple-submit-rules";
-
-  private static final String API_ENDPOINT = PLUGIN_NAME;
-  private final PluginConfigFactory pluginConfigFactory;
-  private final String pluginName;
-
-  @Inject
-  public SimpleSubmitRulesModule(
-      PluginConfigFactory pluginConfigFactory, @PluginName String pluginName) {
-    this.pluginConfigFactory = pluginConfigFactory;
-    this.pluginName = pluginName;
-  }
+  private static final String API_ENDPOINT = "simple-submit-rules";
 
   @Override
   protected void configure() {
@@ -67,13 +44,4 @@
     DynamicSet.bind(binder(), SubmitRule.class).to(RequireNonAuthorApprovalRule.class);
     DynamicSet.bind(binder(), SubmitRule.class).to(NoUnresolvedCommentsRule.class);
   }
-
-  public PluginConfig getConfig(ChangeData changeData) {
-    try {
-      return pluginConfigFactory.getFromProjectConfig(changeData.project(), pluginName);
-    } catch (NoSuchProjectException e) {
-      log.error("Could not load plugin configuration", e);
-      return null;
-    }
-  }
 }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java
index 468969b..bedff8d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java

@@ -15,7 +15,7 @@
 package com.googlesource.gerrit.plugins.simplesubmitrules.config;
 
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.IdentifiedUser;
@@ -59,10 +59,8 @@
   }
 
   @Override
-  public Object apply(
-      ProjectResource resource,
-      com.googlesource.gerrit.plugins.simplesubmitrules.config.SubmitConfig inConfig)
-      throws PermissionBackendException, RestApiException, IOException {
+  public Object apply(ProjectResource resource, SubmitConfig inConfig)
+      throws PermissionBackendException, AuthException, BadRequestException, IOException {
 
     permissionBackend
         .user(resource.getUser())

diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/SubmitConfig.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/SubmitConfig.java
index 20712a9..9c05ffc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/SubmitConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/SubmitConfig.java

@@ -22,8 +22,8 @@
 import java.util.Set;
 
 public class SubmitConfig {
-  public Map<String, LabelDefinition> labels = new HashMap<>();
-  public CommentsRules comments = new CommentsRules();
+  public Map<String, SubmitConfig.LabelDefinition> labels = new HashMap<>();
+  public SubmitConfig.CommentsRules comments = new SubmitConfig.CommentsRules();
 
   @Override
   public String toString() {
@@ -53,7 +53,7 @@
   }
 
   static class CommentsRules {
-    boolean blockIfUnresolvedComments = false;
+    boolean blockIfUnresolvedComments;
 
     @Override
     public String toString() {

diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRule.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRule.java
index 6712782..bd6451c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRule.java

@@ -16,16 +16,17 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.common.data.SubmitRecord.Status;
 import com.google.gerrit.common.data.SubmitRequirement;
+import com.google.gerrit.extensions.annotations.PluginName;
 import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.SubmitRuleOptions;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.rules.SubmitRule;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.googlesource.gerrit.plugins.simplesubmitrules.SimpleSubmitRulesConfig;
-import com.googlesource.gerrit.plugins.simplesubmitrules.SimpleSubmitRulesModule;
 import java.util.Collection;
 import java.util.Collections;
 import org.slf4j.Logger;
@@ -39,20 +40,31 @@
           .setType("unresolved_comments")
           .setFallbackText("Resolve all comments")
           .build();
-  private final SimpleSubmitRulesModule plugin;
+  private final PluginConfigFactory pluginConfigFactory;
+  private final String pluginName;
 
   @Inject
-  public NoUnresolvedCommentsRule(SimpleSubmitRulesModule plugin) {
-    this.plugin = plugin;
+  public NoUnresolvedCommentsRule(
+      PluginConfigFactory pluginConfigFactory, @PluginName String pluginName) {
+    this.pluginConfigFactory = pluginConfigFactory;
+    this.pluginName = pluginName;
   }
 
   @Override
   public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
-    PluginConfig config = plugin.getConfig(cd);
-    boolean blockIfUnresolvedComments =
+    PluginConfig config;
+    try {
+      config = pluginConfigFactory.getFromProjectConfig(cd.project(), pluginName);
+    } catch (NoSuchProjectException e) {
+      log.error("Error when fetching config of change {}'s project", cd.getId(), e);
+
+      return error("Error when fetching configuration");
+    }
+
+    boolean ruleEnabled =
         config.getBoolean(SimpleSubmitRulesConfig.KEY_BLOCK_IF_UNRESOLVED_COMMENTS, false);
 
-    if (!blockIfUnresolvedComments) {
+    if (!ruleEnabled) {
       return Collections.emptyList();
     }
 
@@ -62,20 +74,23 @@
     } catch (OrmException e) {
       log.error("Error when counting unresolved comments for change {}", cd.getId(), e);
 
-      SubmitRecord sr = new SubmitRecord();
-      sr.status = Status.RULE_ERROR;
-      sr.errorMessage = "Error when counting unresolved comments";
-      return ImmutableList.of(sr);
+      return error("Error when counting unresolved comments");
     }
 
     SubmitRecord sr = new SubmitRecord();
     sr.requirements = Collections.singletonList(REQUIREMENT);
-    if (unresolvedComments == null || unresolvedComments > 0) {
-      sr.status = Status.NOT_READY;
-    } else {
-      sr.status = Status.OK;
-    }
+    sr.status =
+        unresolvedComments == null || unresolvedComments > 0
+            ? SubmitRecord.Status.NOT_READY
+            : SubmitRecord.Status.OK;
 
     return ImmutableList.of(sr);
   }
+
+  private static Collection<SubmitRecord> error(String errorMessage) {
+    SubmitRecord sr = new SubmitRecord();
+    sr.status = SubmitRecord.Status.RULE_ERROR;
+    sr.errorMessage = errorMessage;
+    return ImmutableList.of(sr);
+  }
 }

diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/AbstractSimpleSubmitRulesIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/AbstractSimpleSubmitRulesIT.java
index d1f4f45..e73172f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/AbstractSimpleSubmitRulesIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/AbstractSimpleSubmitRulesIT.java

@@ -26,12 +26,11 @@
 import org.eclipse.jgit.lib.Config;
 import org.junit.Ignore;
 
-@TestPlugin(
-  name = SimpleSubmitRulesModule.PLUGIN_NAME,
-  sysModule = "com.googlesource.gerrit.plugins.simplesubmitrules.SimpleSubmitRulesModule"
-)
-@Ignore
 /** Base class used by IT tests, loads the Simple Submit Rules plugin. */
+@TestPlugin(
+    name = "my-plugin",
+    sysModule = "com.googlesource.gerrit.plugins.simplesubmitrules.SimpleSubmitRulesModule")
+@Ignore
 public abstract class AbstractSimpleSubmitRulesIT extends LightweightPluginDaemonTest {
   /** Helper method to change the project.config file using a provided consumer. */
   protected void changeProjectConfig(Consumer<Config> callback) throws Exception {

diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java
index 6b1e2ba..e83b0e9 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java

@@ -21,7 +21,7 @@
 import com.google.gerrit.common.RawInputUtil;
 import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.extensions.restapi.RawInput;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.reviewdb.client.Project;
 import com.googlesource.gerrit.plugins.simplesubmitrules.AbstractSimpleSubmitRulesIT;
 import org.junit.Before;
 import org.junit.Test;
@@ -86,7 +86,7 @@
         "application/json");
   }
 
-  private static String endpointUrl(NameKey project) {
+  private static String endpointUrl(Project.NameKey project) {
     return "/projects/" + project.get() + "/simple-submit-rules";
   }
 }

diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
index f3b95ca..332afb5 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java

@@ -18,7 +18,6 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.data.LabelType;
-import com.googlesource.gerrit.plugins.simplesubmitrules.config.SubmitConfig.LabelDefinition;
 import java.util.function.BiConsumer;
 import java.util.function.Predicate;
 import org.junit.Test;
@@ -67,7 +66,7 @@
       String copyScoreName, BiConsumer<LabelType, Boolean> functionToSet) {
 
     LabelType label = LabelType.withDefaultValues("Verified");
-    LabelDefinition labelDefinition = new SubmitConfig.LabelDefinition();
+    SubmitConfig.LabelDefinition labelDefinition = new SubmitConfig.LabelDefinition();
 
     functionToSet.accept(label, false);
     ConfigTranslator.extractLabelCopyScores(label, labelDefinition);
@@ -81,7 +80,7 @@
       String copyScoreName, BiConsumer<LabelType, Boolean> functionToSet) {
 
     LabelType label = LabelType.withDefaultValues("Verified");
-    LabelDefinition labelDefinition = new SubmitConfig.LabelDefinition();
+    SubmitConfig.LabelDefinition labelDefinition = new SubmitConfig.LabelDefinition();
 
     functionToSet.accept(label, true);
     ConfigTranslator.extractLabelCopyScores(label, labelDefinition);

diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRuleIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRuleIT.java
index 3ae48f1..8e80861 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRuleIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/NoUnresolvedCommentsRuleIT.java

@@ -22,10 +22,9 @@
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.server.project.SubmitRuleOptions;
-import com.google.inject.Inject;
+import com.google.gerrit.server.query.change.ChangeData;
 import com.googlesource.gerrit.plugins.simplesubmitrules.AbstractSimpleSubmitRulesIT;
 import com.googlesource.gerrit.plugins.simplesubmitrules.SimpleSubmitRulesConfig;
 import java.util.Collection;
@@ -35,7 +34,6 @@
 @NoHttpd
 public class NoUnresolvedCommentsRuleIT extends AbstractSimpleSubmitRulesIT {
   private static final String FILENAME = "my.file";
-  @Inject private NoUnresolvedCommentsRule rule;
 
   @Before
   public void enableRuleBeforeTest() throws Exception {
@@ -44,12 +42,11 @@
 
   @Test
   public void blocksWithUnresolvedComments() throws Exception {
-    CommentInput comment = newFileComment();
+    ReviewInput.CommentInput comment = newFileComment();
     comment.unresolved = true;
     PushOneCommit.Result r = createChangeWithComment(comment);
 
-    Collection<SubmitRecord> submitRecords =
-        rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+    Collection<SubmitRecord> submitRecords = evaluate(r.getChange(), SubmitRuleOptions.defaults());
 
     assertThat(submitRecords).hasSize(1);
     SubmitRecord result = submitRecords.iterator().next();
@@ -60,12 +57,11 @@
 
   @Test
   public void doesNotBlockWithNoComments() throws Exception {
-    CommentInput comment = newFileComment();
+    ReviewInput.CommentInput comment = newFileComment();
     comment.unresolved = false;
     PushOneCommit.Result r = createChangeWithComment(comment);
 
-    Collection<SubmitRecord> submitRecords =
-        rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+    Collection<SubmitRecord> submitRecords = evaluate(r.getChange(), SubmitRuleOptions.defaults());
 
     assertThat(submitRecords).hasSize(1);
     SubmitRecord result = submitRecords.iterator().next();
@@ -79,7 +75,7 @@
     PushOneCommit.Result change = createChange("refs/for/master");
 
     Collection<SubmitRecord> submitRecords =
-        rule.evaluate(change.getChange(), SubmitRuleOptions.defaults());
+        evaluate(change.getChange(), SubmitRuleOptions.defaults());
 
     assertThat(submitRecords).hasSize(1);
     SubmitRecord result = submitRecords.iterator().next();
@@ -90,19 +86,18 @@
 
   @Test
   public void doesNothingByDefault() throws Exception {
-    CommentInput comment = newFileComment();
+    ReviewInput.CommentInput comment = newFileComment();
     comment.unresolved = true;
 
     PushOneCommit.Result r = createChangeWithComment(comment);
 
     enableRule(false);
 
-    Collection<SubmitRecord> submitRecords =
-        rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+    Collection<SubmitRecord> submitRecords = evaluate(r.getChange(), SubmitRuleOptions.defaults());
     assertThat(submitRecords).isEmpty();
   }
 
-  private PushOneCommit.Result createChangeWithComment(CommentInput comment) throws Exception {
+  private PushOneCommit.Result createChangeWithComment(ReviewInput.CommentInput comment) throws Exception {
     PushOneCommit.Result r = createChange("My change", FILENAME, "new content");
     ReviewInput reviewInput = new ReviewInput();
     reviewInput.comments = ImmutableMap.of(comment.path, ImmutableList.of(comment));
@@ -121,8 +116,15 @@
                 newState));
   }
 
-  private static CommentInput newFileComment() {
-    CommentInput c = new CommentInput();
+  private Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+    NoUnresolvedCommentsRule rule =
+        plugin.getSysInjector().getInstance(NoUnresolvedCommentsRule.class);
+
+    return rule.evaluate(cd, options);
+  }
+
+  private static ReviewInput.CommentInput newFileComment() {
+    ReviewInput.CommentInput c = new ReviewInput.CommentInput();
     c.path = FILENAME;
     c.side = Side.REVISION;
     c.message = "nit: double  space.";

diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/RequireNonAuthorApprovalRuleTest.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/RequireNonAuthorApprovalRuleTest.java
index b1e0082..7324f55 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/RequireNonAuthorApprovalRuleTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/rules/RequireNonAuthorApprovalRuleTest.java

@@ -24,7 +24,6 @@
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.LabelId;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSet.Id;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -87,7 +86,8 @@
     return new PatchSetApproval(key, (short) value, Date.from(Instant.now()));
   }
 
-  private static PatchSetApproval.Key makeKey(Id psId, Account.Id accountId, LabelId labelId) {
+  private static PatchSetApproval.Key makeKey(
+      PatchSet.Id psId, Account.Id accountId, LabelId labelId) {
     return new PatchSetApproval.Key(psId, accountId, labelId);
   }