Allow to specify which group of the comment link holds the issue id
This configuration option is useful when reusing the same comment
link for different its plugins.
Change-Id: I16786b96c388deca5559f57d1d7760a5c8472410
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 2be5be8..cdca3e8 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
@@ -147,6 +147,10 @@
/**
* Gets the regular expression used to identify issue ids.
+ * <p>
+ * The index of the group that holds the issue id is
+ * {@link #getIssuePatternGroupIndex()}.
+ *
* @return the regular expression, or {@code null}, if there is no pattern
* to match issue ids.
*/
@@ -161,6 +165,24 @@
}
/**
+ * Gets the index of the group in the issue pattern that holds the issue id.
+ * <p>
+ * The corresponding issue pattern is {@link #getIssuePattern()}
+ *
+ * @return the group index for {@link #getIssuePattern()} that holds the
+ * issue id. The group index is guaranteed to be a valid group index.
+ */
+ public int getIssuePatternGroupIndex() {
+ Pattern pattern = getIssuePattern();
+ int groupCount = pattern.matcher("").groupCount();
+ int index = gerritConfig.getInt(pluginName, "commentlinkGroupIndex", 1);
+ if (index < 0 || index > groupCount) {
+ index = (groupCount == 0 ? 0 : 1);
+ }
+ return index;
+ }
+
+ /**
* Gets how necessary it is to associate commits with issues
* @return policy on how necessary association with issues is
*/
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
index 4894492..45892d2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
@@ -51,8 +51,8 @@
Set<String> issues = Sets.newHashSet();
Matcher matcher = pattern.matcher(haystack);
+ int groupIdx = itsConfig.getIssuePatternGroupIndex();
while (matcher.find()) {
- int groupIdx = Math.min(matcher.groupCount(), 1);
String issueId = matcher.group(groupIdx);
if (!Strings.isNullOrEmpty(issueId)) {
issues.add(issueId);
diff --git a/src/main/resources/Documentation/config-common.md b/src/main/resources/Documentation/config-common.md
index 26da3ca..a111aba 100644
--- a/src/main/resources/Documentation/config-common.md
+++ b/src/main/resources/Documentation/config-common.md
@@ -17,8 +17,8 @@
[commentlink][upstream-comment-link-doc]s of
([per default][common-config-commentlink]) name "`@PLUGIN@`".
-The first group of `commentlink.@PLUGIN@.match` is considered the
-issue id.
+The ([per default][common-config-commentlinkGroupIndex]) first group of
+`commentlink.@PLUGIN@.match` is considered the issue id.
So for example having
@@ -148,6 +148,7 @@
-----------------------------------------------------------------------
[common-config-commentlink]: #common-config-commentlink
+[common-config-commentlinkGroupIndex]: #common-config-commentlinkGroupIndex
<a name="common-config-commentlink">`@PLUGIN@.commentlink`
: The name of the comment link to use to extract issue ids.
@@ -159,6 +160,13 @@
Default is `@PLUGIN@`
+<a name="common-config-commentlinkGroupIndex">`@PLUGIN@.commentlinkGroupIndex`
+: The group index within `@PLUGIN@.commentlink` that holds the issue id.
+
+ Default is `1`, if there are are groups within the regular expression for
+ the `@PLUGIN@.commentlink` comment link, and the default is `0`, if there
+ are no such groups.
+
[Back to @PLUGIN@ documentation index][index]
[index]: index.html
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 0417c55..88b99fd 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
@@ -465,6 +465,102 @@
}
+ public void testPatternGroupIndexGroupDefault() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("(foo)(bar)(baz)").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(1).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 1, itsConfig.getIssuePatternGroupIndex());
+ }
+
+ public void testPatternGroupIndexGroupDefaultGroupless() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("foo").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(1).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 0, itsConfig.getIssuePatternGroupIndex());
+ }
+
+ public void testPatternGroupIndexGroup1() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("(foo)(bar)(baz)").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(1).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 1, itsConfig.getIssuePatternGroupIndex());
+ }
+
+ public void testPatternGroupIndexGroup3() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("(foo)(bar)(baz)").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(3).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 3, itsConfig.getIssuePatternGroupIndex());
+ }
+
+ public void testPatternGroupIndexGroupTooHigh() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("(foo)(bar)(baz)").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(5).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 1, itsConfig.getIssuePatternGroupIndex());
+ }
+
+ public void testPatternGroupIndexGroupTooHighGroupless() {
+ ItsConfig itsConfig = createItsConfig();
+
+ expect(serverConfig.getString("ItsTestName", null, "commentlink"))
+ .andReturn(null).atLeastOnce();
+ expect(serverConfig.getString("commentlink", "ItsTestName", "match"))
+ .andReturn("foo").atLeastOnce();
+ expect(serverConfig.getInt("ItsTestName", "commentlinkGroupIndex", 1))
+ .andReturn(5).atLeastOnce();
+
+ replayMocks();
+
+ assertEquals("Expected and actual group index do not match",
+ 0, itsConfig.getIssuePatternGroupIndex());
+ }
+
public void testItsAssociationPolicyOptional() {
ItsConfig itsConfig = createItsConfig();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
index f20e955..7a1530d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
@@ -65,6 +65,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -79,6 +80,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(X*)(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -93,6 +95,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -108,6 +111,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -123,6 +127,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#\\d+"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(0).atLeastOnce();
replayMocks();
@@ -133,11 +138,12 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsMultiGroupMatch() {
+ public void testIssueIdsMultiGroupMatchGroup1() {
IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
expect(itsConfig.getIssuePattern())
.andReturn(Pattern.compile("bug#(\\d)(\\d+)")).atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -148,11 +154,28 @@
assertLogMessageContains("Matching");
}
+ public void testIssueIdsMultiGroupMatchGroup2() {
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+
+ expect(itsConfig.getIssuePattern())
+ .andReturn(Pattern.compile("bug#(\\d)(\\d+)")).atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(2).atLeastOnce();
+
+ replayMocks();
+
+ String ret[] = issueExtractor.getIssueIds("Foo bug#4711 bar");
+ assertEquals("Number of found ids do not match", 1, ret.length);
+ assertEquals("Found issue id does not match", "711", ret[0]);
+
+ assertLogMessageContains("Matching");
+ }
+
public void testIssueIdsMulipleMatches() {
IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -171,6 +194,7 @@
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
replayMocks();
@@ -188,6 +212,7 @@
public void testIssueIdsCommitSingleIssue() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -215,6 +240,7 @@
public void testIssueIdsCommitMultipleIssues() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -243,6 +269,7 @@
public void testIssueIdsCommitMultipleIssuesMultipleTimes() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -271,6 +298,7 @@
public void testIssueIdsCommitSingleIssueBody() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -301,6 +329,7 @@
public void testIssueIdsCommitSingleIssueFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -332,6 +361,7 @@
public void testIssueIdsCommitMultipleIssuesFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -372,6 +402,7 @@
public void testIssueIdsCommitDifferentParts() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -406,6 +437,7 @@
public void testIssueIdsCommitDifferentPartsEmptySubject() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -439,6 +471,7 @@
public void testIssueIdsCommitDifferentPartsLinePastFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -473,6 +506,7 @@
public void testIssueIdsCommitDifferentPartsLinesPastFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -508,6 +542,7 @@
public void testIssueIdsCommitDifferentPartsNoFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -535,6 +570,7 @@
public void testIssueIdsCommitDifferentPartsNoFooterTrailingLine() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -562,6 +598,7 @@
public void testIssueIdsCommitDifferentPartsNoFooterTrailingLines() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -590,6 +627,7 @@
public void testIssueIdsCommitEmpty() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn("");
@@ -611,6 +649,7 @@
public void testIssueIdsCommitBlankLine() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn("\n");
@@ -630,6 +669,7 @@
public void testIssueIdsCommitBlankLines() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn("\n\n");
@@ -649,6 +689,7 @@
public void testIssueIdsCommitMoreBlankLines() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn("\n\n\n");
@@ -668,6 +709,7 @@
public void testIssueIdsCommitMixed() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn(
@@ -708,6 +750,7 @@
public void testIssueIdsCommitWAddedEmptyFirst() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
expect(commitMessageFetcher.fetchGuarded("testProject",
"1234567891123456789212345678931234567894")).andReturn("");
@@ -730,6 +773,7 @@
public void testIssueIdsCommitWAddedSingleSubjectIssueFirst() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -766,6 +810,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -828,6 +873,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -889,6 +935,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -951,6 +998,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -1015,6 +1063,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
@@ -1083,6 +1132,7 @@
throws OrmException {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
+ expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
Change.Id changeId = createMock(Change.Id.class);
