Move getting the issue association policy into the ItsConfig class "Getting the issue association policy" is a config thing. Now that we have a ItsConfig class, it should live there. Hence, we move it there. Change-Id: Ic0fb8304e0863e5bcee8e635c5407704dd9bc45d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfig.java index e3fc124..7f107bf 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfig.java
@@ -32,6 +32,8 @@ import com.google.gerrit.server.project.RefPatternMatcher; import com.google.inject.Inject; +import com.googlesource.gerrit.plugins.hooks.validation.ItsAssociationPolicy; + import java.util.regex.Pattern; import org.eclipse.jgit.lib.Config; @@ -140,4 +142,13 @@ } return ret; } + + /** + * Gets how necessary it is to associate commits with issues + * @return policy on how necessary association with issues is + */ + public ItsAssociationPolicy getItsAssociationPolicy() { + return gerritConfig.getEnum("commentLink", pluginName, "association", + ItsAssociationPolicy.OPTIONAL); + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateComment.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateComment.java index 8dd24c2..190f061 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateComment.java +++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateComment.java
@@ -16,7 +16,6 @@ import com.google.common.collect.Lists; import com.google.gerrit.extensions.annotations.PluginName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidationListener; @@ -27,7 +26,6 @@ import com.googlesource.gerrit.plugins.hooks.its.ItsFacade; import com.googlesource.gerrit.plugins.hooks.util.IssueExtractor; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.ReceiveCommand; import org.slf4j.Logger; @@ -45,10 +43,6 @@ @Inject private ItsFacade client; - @Inject - @GerritServerConfig - private Config gerritConfig; - @Inject @PluginName private String pluginName; @@ -60,7 +54,7 @@ private List<CommitValidationMessage> validCommit(ReceiveCommand cmd, RevCommit commit) throws CommitValidationException { List<CommitValidationMessage> ret = Lists.newArrayList(); - ItsAssociationPolicy associationPolicy = getItsAssociationPolicy(); + ItsAssociationPolicy associationPolicy = itsConfig.getItsAssociationPolicy(); switch (associationPolicy) { case MANDATORY: @@ -135,16 +129,11 @@ return ret; } - private ItsAssociationPolicy getItsAssociationPolicy() { - return gerritConfig.getEnum("commentLink", pluginName, "association", - ItsAssociationPolicy.OPTIONAL); - } - private CommitValidationMessage commitValidationFailure( String synopsis, String details) throws CommitValidationException { CommitValidationMessage ret = new CommitValidationMessage(synopsis + "\n" + details, false); - if (getItsAssociationPolicy() == ItsAssociationPolicy.MANDATORY) { + if (itsConfig.getItsAssociationPolicy() == ItsAssociationPolicy.MANDATORY) { throw new CommitValidationException(synopsis, Collections.singletonList(ret)); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfigTest.java index ebf36b2..39b0455 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/hooks/its/ItsConfigTest.java
@@ -30,6 +30,7 @@ import com.google.inject.Injector; import com.googlesource.gerrit.plugins.hooks.testutil.LoggingMockingTestCase; +import com.googlesource.gerrit.plugins.hooks.validation.ItsAssociationPolicy; import org.eclipse.jgit.lib.Config; @@ -431,6 +432,48 @@ "TestPattern", itsConfig.getIssuePattern().pattern()); } + public void testItsAssociationPolicyOptional() { + ItsConfig itsConfig = createItsConfig(); + + expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", + ItsAssociationPolicy.OPTIONAL)) + .andReturn(ItsAssociationPolicy.OPTIONAL) + .atLeastOnce(); + + replayMocks(); + + assertEquals("Expected and generated associated policy do not match", + ItsAssociationPolicy.OPTIONAL, itsConfig.getItsAssociationPolicy()); + } + + public void testItsAssociationPolicySuggested() { + ItsConfig itsConfig = createItsConfig(); + + expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", + ItsAssociationPolicy.OPTIONAL)) + .andReturn(ItsAssociationPolicy.SUGGESTED) + .atLeastOnce(); + + replayMocks(); + + assertEquals("Expected and generated associated policy do not match", + ItsAssociationPolicy.SUGGESTED, itsConfig.getItsAssociationPolicy()); + } + + public void testItsAssociationPolicyMandatory() { + ItsConfig itsConfig = createItsConfig(); + + expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", + ItsAssociationPolicy.OPTIONAL)) + .andReturn(ItsAssociationPolicy.MANDATORY) + .atLeastOnce(); + + replayMocks(); + + assertEquals("Expected and generated associated policy do not match", + ItsAssociationPolicy.MANDATORY, itsConfig.getItsAssociationPolicy()); + } + private ItsConfig createItsConfig() { return injector.getInstance(ItsConfig.class); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateCommentTest.java b/src/test/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateCommentTest.java index 2358ced..fd679a6 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateCommentTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/hooks/validation/ItsValidateCommentTest.java
@@ -45,7 +45,6 @@ @PrepareForTest({RevCommit.class}) public class ItsValidateCommentTest extends LoggingMockingTestCase { private Injector injector; - private Config serverConfig; private IssueExtractor issueExtractor; private ItsFacade itsFacade; private ItsConfig itsConfig; @@ -59,9 +58,10 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.OPTIONAL).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.OPTIONAL).atLeastOnce(); + replayMocks(); ret = ivc.onCommitReceived(event); @@ -76,11 +76,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); - expect(serverConfig.getString("commentLink", "ItsTestName", "match")) - .andReturn("TestPattern").anyTimes(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("TestMessage").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -103,9 +101,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("TestMessage").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -132,9 +130,8 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -157,9 +154,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -182,9 +179,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -211,9 +208,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711").atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); expect(commit.getName()).andReturn("TestCommit").anyTimes(); @@ -240,9 +237,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -267,9 +264,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -294,9 +291,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -327,9 +324,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -358,9 +355,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -391,9 +388,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.MANDATORY).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -422,9 +419,9 @@ RevCommit commit = createMock(RevCommit.class); CommitReceivedEvent event = new CommitReceivedEvent(command, project, null, commit, null); - expect(serverConfig.getEnum("commentLink", "ItsTestName", "association", - ItsAssociationPolicy.OPTIONAL)).andReturn( - ItsAssociationPolicy.SUGGESTED).atLeastOnce(); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.SUGGESTED).atLeastOnce(); expect(commit.getFullMessage()).andReturn("bug#4711, bug#42") .atLeastOnce(); expect(commit.getId()).andReturn(commit).anyTimes(); @@ -491,10 +488,6 @@ bind(String.class).annotatedWith(PluginName.class) .toInstance("ItsTestName"); - serverConfig = createMock(Config.class); - bind(Config.class).annotatedWith(GerritServerConfig.class) - .toInstance(serverConfig); - issueExtractor = createMock(IssueExtractor.class); bind(IssueExtractor.class).toInstance(issueExtractor);