When extracting issues, only consider non-empty strings as issue ids
For groups that could potentially be empty, we considered empty
matches as issue ids. This could lead to counterintuitive
results. Hence, we make sure to only consider non-empty strings.
Change-Id: I2329cf28cd22f2f1b050de93e9ae5df27dcd27a5
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 c4a6864..4894492 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
@@ -1,5 +1,6 @@
package com.googlesource.gerrit.plugins.hooks.util;
+import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -52,7 +53,10 @@
while (matcher.find()) {
int groupIdx = Math.min(matcher.groupCount(), 1);
- issues.add(matcher.group(groupIdx));
+ String issueId = matcher.group(groupIdx);
+ if (!Strings.isNullOrEmpty(issueId)) {
+ issues.add(issueId);
+ }
}
return issues.toArray(new String[issues.size()]);
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 a6c42c7..f20e955 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
@@ -74,6 +74,20 @@
assertLogMessageContains("Matching");
}
+ public void testIssueIdsEmptyGroup() {
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+
+ expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(X*)(\\d+)"))
+ .atLeastOnce();
+
+ replayMocks();
+
+ String ret[] = issueExtractor.getIssueIds("bug#4711");
+ assertEquals("Number of found ids do not match", 0, ret.length);
+
+ assertLogMessageContains("Matching");
+ }
+
public void testIssueIdsFullMatch() {
IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);