Add properties to specify where an issue got associated Associations in the commit message's subject get a "association = subject" property. Accordingly for the body and the footer of a commit message. Change-Id: Ia594d7204995f1cd303b8ae533b85500f304f851
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java index 8030d78..017fbb9 100644 --- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java +++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
@@ -12,6 +12,7 @@ import com.googlesource.gerrit.plugins.hooks.its.ItsName; +import org.apache.commons.lang.StringUtils; import org.eclipse.jgit.lib.Config; import org.slf4j.Logger; @@ -71,19 +72,24 @@ } /** - * Adds an issue to the map returned by {@link #getIssueIds(String, String)}. + * Helper funcion for {@link #getIssueIds(String, String)}. + * <p> + * Adds a text's issues for a given occurrence to the map returned by + * {@link #getIssueIds(String, String)}. * - * @param issue The issue to add. - * @param map The map that the issue should get added to. - * @param occurrence The occurrence the issue get added at in {@code map}. + * @param text The text to extract issues from. + * @param occurrence The occurrence the issues get added at in {@code map}. + * @param map The map that the issues should get added to. */ - private void addIssueOccurrence(String issue, Map<String,Set<String>> map, String occurrence) { - Set<String> occurrences = map.get(issue); - if (occurrences == null) { - occurrences = Sets.newLinkedHashSet(); - map.put(issue, occurrences); + private void addIssuesOccurrence(String text, String occurrence, Map<String,Set<String>> map) { + for (String issue : getIssueIds(text)) { + Set<String> occurrences = map.get(issue); + if (occurrences == null) { + occurrences = Sets.newLinkedHashSet(); + map.put(issue, occurrences); + } + occurrences.add(occurrence); } - occurrences.add(occurrence); } /** @@ -93,15 +99,61 @@ * @param commitId The commit id to fetch issues for. * @return A mapping, whose keys are issue ids and whose values is a set of * places where the issue occurs. Each issue occurs at least in - * "somewhere". + * "somewhere". Issues from the first line get tagged with an occurrence + * "subject". Issues in the last block get tagged with "footer". Issues + * occurring between "subject" and "footer" get tagged with "body". */ public Map<String,Set<String>> getIssueIds(String projectName, String commitId) { Map<String,Set<String>> ret = Maps.newHashMap(); String commitMessage = commitMessageFetcher.fetchGuarded(projectName, commitId); - for (String issue : getIssueIds(commitMessage)) { - addIssueOccurrence(issue, ret, "somewhere"); + + addIssuesOccurrence(commitMessage, "somewhere", ret); + + String[] lines = commitMessage.split("\n"); + if (lines.length > 0) { + // Parsing for "subject" + addIssuesOccurrence(lines[0], "subject", ret); + + // Determining footer line numbers + int currentLine = lines.length-1; + while (currentLine >=0 && lines[currentLine].isEmpty()) { + currentLine--; + } + int footerEnd = currentLine + 1; + while (currentLine >=0 && !lines[currentLine].isEmpty()) { + currentLine--; + } + int footerStart = currentLine + 1; + + if (footerStart == 0) { + // The first block of non-blank lines is not considered a footer, so + // we adjust that. + footerStart = -1; + } + + // Parsing for "body", and "footer" + String body = null; + String footer = null; + if (footerStart == -1) { + // No footer could be found. So all lines after the first one (that's + // the subject) is the body. + //body = String[] templateParameters = + // Arrays.copyOfRange(allParameters, 1, allParameters.length); + if (lines.length > 0) { + body = StringUtils.join(lines, "\n", 1, lines.length); + } + } else { + body = StringUtils.join(lines, "\n", 1, footerStart - 1); + footer = StringUtils.join(lines, "\n", footerStart, footerEnd); + } + if (body != null) { + addIssuesOccurrence(body, "body", ret); + } + if (footer != null) { + addIssuesOccurrence(footer, "footer", ret); + } } return ret; }
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java index 33705a3..323205f 100644 --- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java +++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
@@ -193,10 +193,13 @@ "1234567891123456789212345678931234567894"); Map<String,Set<String>> expected = Maps.newHashMap(); - expected.put("42", Sets.newHashSet("somewhere")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); assertEquals("Extracted issues do not match", expected, actual); assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); } public void testIssueIdsCommitMultipleIssues() { @@ -216,11 +219,14 @@ "1234567891123456789212345678931234567894"); Map<String,Set<String>> expected = Maps.newHashMap(); - expected.put("42", Sets.newHashSet("somewhere")); - expected.put("4711", Sets.newHashSet("somewhere")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("4711", Sets.newHashSet("somewhere", "subject")); assertEquals("Extracted issues do not match", expected, actual); assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); } public void testIssueIdsCommitMultipleIssuesMultipleTimes() { @@ -240,11 +246,395 @@ "1234567891123456789212345678931234567894"); Map<String,Set<String>> expected = Maps.newHashMap(); - expected.put("42", Sets.newHashSet("somewhere")); - expected.put("4711", Sets.newHashSet("somewhere")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("4711", Sets.newHashSet("somewhere", "subject")); assertEquals("Extracted issues do not match", expected, actual); assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitSingleIssueBody() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject does not reference a bug\n" + + "Body references bug#42\n" + + "\n" + + "Footer: does not reference a bug\n" + + "Change-Id: I1234567891123456789212345678931234567894"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("42", Sets.newHashSet("somewhere", "body")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitSingleIssueFooter() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject does not reference a bug\n" + + "Body does not reference a bug\n" + + "\n" + + "Footer: references bug#42\n" + + "Change-Id: I1234567891123456789212345678931234567894"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("42", Sets.newHashSet("somewhere", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentParts() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n" + + "\n" + + "Bug: bug#4711 in footer\n" + + "Change-Id: I1234567891123456789212345678931234567894"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + expected.put("4711", Sets.newHashSet("somewhere", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsEmptySubject() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n" + + "\n" + + "Bug: bug#4711 in footer\n" + + "Change-Id: I1234567891123456789212345678931234567894"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + expected.put("4711", Sets.newHashSet("somewhere", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsLinePastFooter() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n" + + "\n" + + "Bug: bug#4711 in footer\n" + + "Change-Id: I1234567891123456789212345678931234567894\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + expected.put("4711", Sets.newHashSet("somewhere", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsLinesPastFooter() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n" + + "\n" + + "Bug: bug#4711 in footer\n" + + "Change-Id: I1234567891123456789212345678931234567894\n" + + "\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + expected.put("4711", Sets.newHashSet("somewhere", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsNoFooter() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176."); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsNoFooterTrailingLine() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitDifferentPartsNoFooterTrailingLines() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject references bug#42.\n" + + "Body references bug#16.\n" + + "Body also references bug#176.\n" + + "\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "body")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitEmpty() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn(""); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitBlankLine() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn("\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitBlankLines() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn("\n\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitMoreBlankLines() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn("\n\n\n"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + } + + public void testIssueIdsCommitMixed() { + expect(serverConfig.getString("commentLink", "ItsTestName", "match")) + .andReturn("bug#(\\d+)").atLeastOnce(); + + expect(commitMessageFetcher.fetchGuarded("testProject", + "1234567891123456789212345678931234567894")).andReturn( + "Subject bug#42, bug#1984, and bug#16\n" + + "\n" + + "bug#4711 in body,\n" + + "along with bug#1984, and bug#5150.\n" + + "bug#4711 in body again, along with bug#16\n" + + "\n" + + "Bug: bug#176, bug#1984, and bug#5150\n" + + "Change-Id: I1234567891123456789212345678931234567894"); + + replayMocks(); + + IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class); + Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject", + "1234567891123456789212345678931234567894"); + + Map<String,Set<String>> expected = Maps.newHashMap(); + expected.put("16", Sets.newHashSet("somewhere", "subject", "body")); + expected.put("42", Sets.newHashSet("somewhere", "subject")); + expected.put("176", Sets.newHashSet("somewhere", "footer")); + expected.put("1984", Sets.newHashSet("somewhere", "subject", "body", + "footer")); + expected.put("4711", Sets.newHashSet("somewhere", "body")); + expected.put("5150", Sets.newHashSet("somewhere", "body", "footer")); + assertEquals("Extracted issues do not match", expected, actual); + + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); + assertLogMessageContains("Matching"); } @Override