Merge "Factor out the access to PatchSets information" into stable-2.15
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
index 546e6c7..5ed4993 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
@@ -6,6 +6,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
+import com.google.inject.ImplementedBy;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.its.base.its.ItsConfig;
@@ -24,13 +25,39 @@
IssueExtractor.class);
private final CommitMessageFetcher commitMessageFetcher;
- private final ReviewDb db;
+ private final PatchSetDb db;
private final ItsConfig itsConfig;
+ @ImplementedBy(PatchSetDbImpl.class)
+ public interface PatchSetDb {
+ public String getRevision(PatchSet.Id patchSetId);
+ }
+
+ public static class PatchSetDbImpl implements PatchSetDb {
+ private final ReviewDb db;
+
+ @Inject
+ public PatchSetDbImpl(ReviewDb db) {
+ this.db = db;
+ }
+
+ @Override
+ public String getRevision(PatchSet.Id patchSetId) {
+ try {
+ PatchSet previousPatchSet = db.patchSets().get(patchSetId);
+ if (previousPatchSet != null) {
+ return 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.
+ }
+ return null;
+ }
+ }
+
@Inject
- IssueExtractor(ItsConfig itsConfig,
- CommitMessageFetcher commitMessageFetcher,
- ReviewDb db) {
+ IssueExtractor(ItsConfig itsConfig, CommitMessageFetcher commitMessageFetcher, PatchSetDb db) {
this.commitMessageFetcher = commitMessageFetcher;
this.db = db;
this.itsConfig = itsConfig;
@@ -188,23 +215,15 @@
* "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);
+ 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();
+ 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.
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(patchSetId.getParentKey(), patchSetId.get() - 1);
+ String previousPatchSet = db.getRevision(previousPatchSetId);
+ if (previousPatchSet != null) {
+ previous = getIssueIds(projectName, previousPatchSet);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
index b79e764..3e8721d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractorTest.java
@@ -21,14 +21,13 @@
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.gwtorm.server.OrmException;
+
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.googlesource.gerrit.plugins.its.base.its.ItsConfig;
import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase;
+import com.googlesource.gerrit.plugins.its.base.util.IssueExtractor.PatchSetDb;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -46,7 +45,7 @@
private Injector injector;
private ItsConfig itsConfig;
private CommitMessageFetcher commitMessageFetcher;
- private ReviewDb db;
+ private PatchSetDb db;
public void testIssueIdsNullPattern() {
IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
@@ -806,8 +805,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedSingleSubjectIssueSecondEmpty()
- throws OrmException {
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondEmpty() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -823,15 +821,7 @@
// 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(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
expect(commitMessageFetcher.fetchGuarded("testProject",
"9876543211987654321298765432139876543214")).andReturn(
@@ -839,8 +829,6 @@
"\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)
@@ -869,8 +857,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedSingleSubjectIssueSecondSame()
- throws OrmException {
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondSame() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -886,23 +873,13 @@
// 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);
+ expect(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
expect(currentPatchSetId.get()).andReturn(2).anyTimes();
@@ -931,8 +908,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedSingleSubjectIssueSecondBody()
- throws OrmException {
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondBody() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -948,16 +924,6 @@
// 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" +
@@ -965,7 +931,7 @@
"\n" +
"Change-Id: I9876543211987654321298765432139876543214");
- expect(db.patchSets()).andReturn(patchSetAccess);
+ expect(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
expect(currentPatchSetId.get()).andReturn(2).anyTimes();
@@ -994,8 +960,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedSingleSubjectIssueSecondFooter()
- throws OrmException {
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -1012,23 +977,13 @@
// 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);
+ expect(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
expect(currentPatchSetId.get()).andReturn(2).anyTimes();
@@ -1059,8 +1014,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedSubjectFooter()
- throws OrmException {
+ public void testIssueIdsCommitWAddedSubjectFooter() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -1079,16 +1033,6 @@
// 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" +
@@ -1096,7 +1040,7 @@
"\n" +
"Change-Id: I9876543211987654321298765432139876543214");
- expect(db.patchSets()).andReturn(patchSetAccess);
+ expect(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
expect(currentPatchSetId.get()).andReturn(2).anyTimes();
@@ -1128,8 +1072,7 @@
assertLogMessageContains("Matching");
}
- public void testIssueIdsCommitWAddedMultiple()
- throws OrmException {
+ public void testIssueIdsCommitWAddedMultiple() {
expect(itsConfig.getIssuePattern()).andReturn(Pattern.compile("bug#(\\d+)"))
.atLeastOnce();
expect(itsConfig.getIssuePatternGroupIndex()).andReturn(1).atLeastOnce();
@@ -1148,16 +1091,6 @@
// 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" +
@@ -1166,7 +1099,7 @@
"Bug: bug#16\n" +
"Change-Id: I9876543211987654321298765432139876543214");
- expect(db.patchSets()).andReturn(patchSetAccess);
+ expect(db.getRevision(previousPatchSetId)).andReturn("9876543211987654321298765432139876543214");
PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
expect(currentPatchSetId.get()).andReturn(2).anyTimes();
@@ -1216,8 +1149,8 @@
commitMessageFetcher = createMock(CommitMessageFetcher.class);
bind(CommitMessageFetcher.class).toInstance(commitMessageFetcher);
- db = createMock(ReviewDb.class);
- bind(ReviewDb.class).toInstance(db);
+ db = createMock(PatchSetDb.class);
+ bind(PatchSetDb.class).toInstance(db);
}
}
}
\ No newline at end of file
