Merge "When adding issue occurrences, mark them additionally as 'added@'"
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 017fbb9..57f51da 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
@@ -7,7 +7,10 @@
 
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 import com.googlesource.gerrit.plugins.hooks.its.ItsName;
@@ -25,13 +28,16 @@
   private final Config gerritConfig;
   private final String itsName;
   private final CommitMessageFetcher commitMessageFetcher;
+  private final ReviewDb db;
 
   @Inject
   IssueExtractor(@GerritServerConfig Config gerritConfig,
-      @ItsName String itsName, CommitMessageFetcher commitMessageFetcher) {
+      @ItsName String itsName, CommitMessageFetcher commitMessageFetcher,
+      ReviewDb db) {
     this.gerritConfig = gerritConfig;
     this.itsName = itsName;
     this.commitMessageFetcher = commitMessageFetcher;
+    this.db = db;
   }
 
   /**
@@ -157,4 +163,62 @@
     }
     return ret;
   }
+
+  /**
+   * Gets issues for a commit with new issue occurrences marked as "added".
+   * <p>
+   * Fetches the patch set's immediate ancestor and compares issue occurrences
+   * between them. Any new occurrence gets marked as "added." So if for
+   * example in patch sets 1, and 2 issue 23 occurs in the subject, while in
+   * patch set the issue occurs in the body, then patch set 2 has occurrences
+   * "somewhere", and "subject" for issue 23. Patch set 3 has occurrences
+   * "somewhere", "body", and "body-added" for issue 23.
+   *
+   * @param projectName The project to fetch {@code commitId} from.
+   * @param commitId The commit id to fetch issues for.
+   * @param patchSetId The patch set for the {@code commitId}. If it is null,
+   *    no occurrence can be marked as "-added".
+   * @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". 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, PatchSet.Id patchSetId) {
+    Map<String,Set<String>> current = getIssueIds(projectName, commitId);
+    if (patchSetId != null) {
+      Map<String,Set<String>> previous = Maps.newHashMap();
+      if (patchSetId.get() != 1) {
+        PatchSet.Id previousPatchSetId = new PatchSet.Id(
+            patchSetId.getParentKey(), patchSetId.get() - 1);
+        try {
+          PatchSet previousPatchSet = db.patchSets().get(previousPatchSetId);
+          if (previousPatchSet != null) {
+            previous = getIssueIds(projectName,
+                previousPatchSet.getRevision().get());
+          }
+        } catch (OrmException e) {
+          // previous is still empty to indicate that there was no previous
+          // accessible patch set. We treat every occurrence as added.
+        }
+      }
+
+      for (String issue : current.keySet()) {
+        Set<String> currentOccurrences = current.get(issue);
+        Set<String> previousOccurrences = previous.get(issue);
+        Set<String> newOccurrences;
+        if (previousOccurrences == null || previousOccurrences.isEmpty()) {
+          newOccurrences = Sets.newHashSet(currentOccurrences);
+        } else {
+          newOccurrences = Sets.newHashSet(currentOccurrences);
+          newOccurrences.removeAll(previousOccurrences);
+        }
+        for (String occurrence : newOccurrences) {
+          currentOccurrences.add( "added@" + occurrence);
+        }
+      }
+    }
+    return current;
+  }
 }
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
index 895ec33..e6f9968 100644
--- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
+++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
@@ -18,6 +18,8 @@
 import java.util.Set;
 
 import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.events.ChangeAbandonedEvent;
 import com.google.gerrit.server.events.ChangeEvent;
 import com.google.gerrit.server.events.ChangeMergedEvent;
@@ -47,6 +49,22 @@
     this.propertyAttributeExtractor = propertyAttributeExtractor;
   }
 
+  /**
+   * creates a patch id for change id string and patchset id string.
+   * @param changeId String representation of the patch sets {@code Change.Id@}
+   * @param patchId String representation of the patch sets {@code Patchset.Id@}
+   * @return PatchSet.Id for the specified patch set. If the String to int
+   *    conversion fails for any of the parameters, null is returned.
+   */
+  private PatchSet.Id newPatchSetId(String changeId, String patchId) {
+    try {
+        return new PatchSet.Id(new Change.Id(Integer.parseInt(changeId)),
+            Integer.parseInt(patchId));
+    } catch (NumberFormatException e) {
+      return null;
+    }
+  }
+
   private Map<String,Set<String>> extractFrom(ChangeAbandonedEvent event,
       Set<Property> common) {
     common.add(propertyFactory.create("event-type", event.type));
@@ -54,8 +72,10 @@
     common.addAll(propertyAttributeExtractor.extractFrom(event.abandoner, "abandoner"));
     common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
     common.add(propertyFactory.create("reason", event.reason));
+    PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+        event.patchSet.number);
     return issueExtractor.getIssueIds(event.change.project,
-        event.patchSet.revision);
+        event.patchSet.revision, patchSetId);
   }
 
   private Map<String,Set<String>> extractFrom(ChangeMergedEvent event,
@@ -64,8 +84,10 @@
     common.addAll(propertyAttributeExtractor.extractFrom(event.change));
     common.addAll(propertyAttributeExtractor.extractFrom(event.submitter, "submitter"));
     common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
+    PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+        event.patchSet.number);
     return issueExtractor.getIssueIds(event.change.project,
-        event.patchSet.revision);
+        event.patchSet.revision, patchSetId);
   }
 
   private Map<String,Set<String>> extractFrom(ChangeRestoredEvent event,
@@ -75,8 +97,10 @@
     common.addAll(propertyAttributeExtractor.extractFrom(event.restorer, "restorer"));
     common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
     common.add(propertyFactory.create("reason", event.reason));
+    PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+        event.patchSet.number);
     return issueExtractor.getIssueIds(event.change.project,
-        event.patchSet.revision);
+        event.patchSet.revision, patchSetId);
   }
 
   private Map<String,Set<String>> extractFrom(RefUpdatedEvent event,
@@ -94,8 +118,10 @@
     common.addAll(propertyAttributeExtractor.extractFrom(event.change));
     common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
     common.addAll(propertyAttributeExtractor.extractFrom(event.uploader, "uploader"));
+    PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+        event.patchSet.number);
     return issueExtractor.getIssueIds(event.change.project,
-        event.patchSet.revision);
+        event.patchSet.revision, patchSetId);
   }
 
   private Map<String,Set<String>> extractFrom(CommentAddedEvent event,
@@ -106,8 +132,10 @@
     common.addAll(propertyAttributeExtractor.extractFrom(event.author, "commenter"));
     common.add(propertyFactory.create("comment", event.comment));
     //TODO approvals
+    PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+        event.patchSet.number);
     return issueExtractor.getIssueIds(event.change.project,
-        event.patchSet.revision);
+        event.patchSet.revision, patchSetId);
   }
 
   /**
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 323205f..53d69db 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
@@ -21,20 +21,32 @@
 import java.util.Set;
 
 import org.eclipse.jgit.lib.Config;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.server.PatchSetAccess;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.FactoryModule;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.googlesource.gerrit.plugins.hooks.its.ItsName;
 import com.googlesource.gerrit.plugins.hooks.testutil.LoggingMockingTestCase;
 
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({PatchSet.class,RevId.class})
 public class IssueExtractorTest extends LoggingMockingTestCase {
   private Injector injector;
   private Config serverConfig;
   private CommitMessageFetcher commitMessageFetcher;
+  private ReviewDb db;
 
   public void testPatternNullMatch() {
     IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
@@ -637,6 +649,431 @@
     assertLogMessageContains("Matching");
   }
 
+  public void testIssueIdsCommitWAddedEmptyFirst() {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn("");
+
+    replayMocks();
+
+    PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(4), 1);
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", patchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSingleSubjectIssueFirst() {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(1).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "subject",
+        "added@somewhere", "added@subject"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSingleSubjectIssueSecondEmpty()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "subject\n" +
+            "\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "subject",
+        "added@somewhere", "added@subject"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSingleSubjectIssueSecondSame()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "subject"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSingleSubjectIssueSecondBody()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "subject\n" +
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSingleSubjectIssueSecondFooter()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "subject\n" +
+            "\n" +
+            "Bug: bug#42\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "footer", "added@footer"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedSubjectFooter()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "subject bug#42\n" +
+            "\n" +
+            "body bug#42\n" +
+            "\n" +
+            "Bug: bug#42\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "subject\n" +
+            "bug#42\n" +
+            "\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject",
+        "body", "footer", "added@footer"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
+  public void testIssueIdsCommitWAddedMultiple()
+      throws OrmException {
+    expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+    .andReturn("bug#(\\d+)").atLeastOnce();
+
+    Change.Id changeId = createMock(Change.Id.class);
+
+    // Call for current patch set
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "1234567891123456789212345678931234567894")).andReturn(
+            "subject bug#42\n" +
+            "\n" +
+            "body bug#42 bug#16\n" +
+            "\n" +
+            "Bug: bug#42\n" +
+            "Change-Id: I1234567891123456789212345678931234567894");
+
+    // Call for previous patch set
+    PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+    RevId previousRevId = createMock(RevId.class);
+    expect(previousRevId.get())
+        .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+    PatchSet previousPatchSet = createMock(PatchSet.class);
+    expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+    PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+    expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+    expect(commitMessageFetcher.fetchGuarded("testProject",
+        "9876543211987654321298765432139876543214")).andReturn(
+            "subject\n" +
+            "bug#42 bug#4711\n" +
+            "\n" +
+            "Bug: bug#16\n" +
+            "Change-Id: I9876543211987654321298765432139876543214");
+
+    expect(db.patchSets()).andReturn(patchSetAccess);
+
+    PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+    expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+    expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+        .anyTimes();
+
+    replayMocks();
+
+    IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+    Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+        "1234567891123456789212345678931234567894", currentPatchSetId);
+
+    Map<String,Set<String>> expected = Maps.newHashMap();
+    expected.put("16", Sets.newHashSet("somewhere", "body", "added@body"));
+    expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject",
+        "body", "footer", "added@footer"));
+    assertEquals("Extracted issues do not match", expected, actual);
+
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+    assertLogMessageContains("Matching");
+  }
+
   @Override
   public void setUp() throws Exception {
     super.setUp();
@@ -656,6 +1093,9 @@
 
       commitMessageFetcher = createMock(CommitMessageFetcher.class);
       bind(CommitMessageFetcher.class).toInstance(commitMessageFetcher);
+
+      db = createMock(ReviewDb.class);
+      bind(ReviewDb.class).toInstance(db);
     }
   }
 }
\ No newline at end of file
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
index ef9270b..5c331a7 100644
--- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
+++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
@@ -20,6 +20,8 @@
 
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.config.FactoryModule;
 import com.google.gerrit.server.data.AccountAttribute;
 import com.google.gerrit.server.data.ChangeAttribute;
@@ -91,7 +93,9 @@
         .andReturn(propertyReason);
 
     changeAttribute.project = "testProject";
+    changeAttribute.number = "176";
     patchSetAttribute.revision = "testRevision";
+    patchSetAttribute.number = "3";
 
     Set<Property> common = Sets.newHashSet();
     common.add(propertyChange);
@@ -99,7 +103,8 @@
     common.add(propertyPatchSet);
     common.add(propertyReason);
 
-    eventHelper(event, "ChangeAbandonedEvent", "change-abandoned", common);
+    eventHelper(event, "ChangeAbandonedEvent", "change-abandoned", common,
+        true);
   }
 
   public void testChangeMergedEvent() {
@@ -124,14 +129,16 @@
         .andReturn(Sets.newHashSet(propertyPatchSet));
 
     changeAttribute.project = "testProject";
+    changeAttribute.number = "176";
     patchSetAttribute.revision = "testRevision";
+    patchSetAttribute.number = "3";
 
     Set<Property> common = Sets.newHashSet();
     common.add(propertyChange);
     common.add(propertySubmitter);
     common.add(propertyPatchSet);
 
-    eventHelper(event, "ChangeMergedEvent", "change-merged", common);
+    eventHelper(event, "ChangeMergedEvent", "change-merged", common, true);
   }
 
   public void testChangeRestoredEvent() {
@@ -161,7 +168,9 @@
         .andReturn(propertyReason);
 
     changeAttribute.project = "testProject";
+    changeAttribute.number = "176";
     patchSetAttribute.revision = "testRevision";
+    patchSetAttribute.number = "3";
 
     Set<Property> common = Sets.newHashSet();
     common.add(propertyChange);
@@ -169,7 +178,7 @@
     common.add(propertyPatchSet);
     common.add(propertyReason);
 
-    eventHelper(event, "ChangeRestoredEvent", "change-restored", common);
+    eventHelper(event, "ChangeRestoredEvent", "change-restored", common, true);
   }
 
   public void testCommentAddedEvent() {
@@ -199,7 +208,9 @@
         .andReturn(propertyComment);
 
     changeAttribute.project = "testProject";
+    changeAttribute.number = "176";
     patchSetAttribute.revision = "testRevision";
+    patchSetAttribute.number = "3";
 
     Set<Property> common = Sets.newHashSet();
     common.add(propertyChange);
@@ -207,7 +218,7 @@
     common.add(propertyPatchSet);
     common.add(propertyComment);
 
-    eventHelper(event, "CommentAddedEvent", "comment-added", common);
+    eventHelper(event, "CommentAddedEvent", "comment-added", common, true);
   }
 
   public void testPatchSetCreatedEvent() {
@@ -232,14 +243,17 @@
         .andReturn(Sets.newHashSet(propertyPatchSet));
 
     changeAttribute.project = "testProject";
+    changeAttribute.number = "176";
     patchSetAttribute.revision = "testRevision";
+    patchSetAttribute.number = "3";
 
     Set<Property> common = Sets.newHashSet();
     common.add(propertyChange);
     common.add(propertySubmitter);
     common.add(propertyPatchSet);
 
-    eventHelper(event, "PatchSetCreatedEvent", "patchset-created", common);
+    eventHelper(event, "PatchSetCreatedEvent", "patchset-created", common,
+        true);
   }
 
   public void testRefUpdatedEvent() {
@@ -265,11 +279,11 @@
     common.add(propertySubmitter);
     common.add(propertyRefUpdated);
 
-    eventHelper(event, "RefUpdatedEvent", "ref-updated", common);
+    eventHelper(event, "RefUpdatedEvent", "ref-updated", common, false);
   }
 
   private void eventHelper(ChangeEvent event, String className, String type,
-      Set<Property> common) {
+      Set<Property> common, boolean withRevision) {
     PropertyExtractor propertyExtractor = injector.getInstance(
         PropertyExtractor.class);
 
@@ -304,8 +318,14 @@
     HashMap<String,Set<String>> issueMap = Maps.newHashMap();
     issueMap.put("4711", Sets.newHashSet("body", "anywhere"));
     issueMap.put("42", Sets.newHashSet("footer", "anywhere"));
-    expect(issueExtractor.getIssueIds("testProject", "testRevision"))
-        .andReturn(issueMap);
+    if (withRevision) {
+      PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(176), 3);
+      expect(issueExtractor.getIssueIds("testProject", "testRevision",
+          patchSetId)).andReturn(issueMap);
+    } else {
+      expect(issueExtractor.getIssueIds("testProject", "testRevision"))
+      .andReturn(issueMap);
+    }
 
     replayMocks();