opendev/gerrit
Code Issues Proposed changes

Start factoring out PatchSet accesses into a PatchSetUtil class

Browse Source 
Replaces the simplest get calls in most callers that didn't require
substantial refactoring. Doesn't actually read notedb yet because
there is no notedb support for patch sets yet.

Change-Id: I7ef198dd1257b833a3a4b21180f5012f6fdc56d4
This commit is contained in:
Dave Borowitz
2016-01-14 15:00:59 -05:00
parent 5d926ca2ba
commit 3ecfd76674
43 changed files with 365 additions and 183 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -145,9 +146,9 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
"other content", r.getChangeId());
r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin);
Change c = Iterables.getOnlyElement(
queryProvider.get().byKeyPrefix(r.getChangeId())).change();
assertThat(db.patchSets().byChange(c.getId()).toList()).hasSize(2);
ChangeData cd = Iterables.getOnlyElement(
queryProvider.get().byKeyPrefix(r.getChangeId()));
assertThat(cd.patchSets()).hasSize(2);
assertSubmitApproval(r.getPatchSetId());
assertCommit(project, "refs/heads/master");
}

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
View File

@@ -187,7 +187,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
Change change1 = db.changes().get(c1);
PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1, 1));
PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1));
// Admin's edit is not visible.
setApiUser(admin);
@@ -214,7 +214,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
Change change1 = db.changes().get(c1);
PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1, 1));
PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1));
setApiUser(admin);
editModifier.createEdit(change1, ps1);
setApiUser(user);

18
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
View File

@@ -208,8 +208,8 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
c = db.changes().get(c.getId());
assertThat(c.currentPatchSetId().get()).isEqualTo(1);
assertThat(db.patchSets().get(ps1.getId())).isNotNull();
assertThat(db.patchSets().get(ps2.getId())).isNull();
assertThat(getPatchSet(ps1.getId())).isNotNull();
assertThat(getPatchSet(ps2.getId())).isNull();
}
@Test
@@ -256,10 +256,10 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
c = db.changes().get(c.getId());
assertThat(c.currentPatchSetId().get()).isEqualTo(3);
assertThat(db.patchSets().get(ps1.getId())).isNotNull();
assertThat(db.patchSets().get(ps2.getId())).isNull();
assertThat(db.patchSets().get(ps3.getId())).isNotNull();
assertThat(db.patchSets().get(ps4.getId())).isNull();
assertThat(getPatchSet(ps1.getId())).isNotNull();
assertThat(getPatchSet(ps2.getId())).isNull();
assertThat(getPatchSet(ps3.getId())).isNotNull();
assertThat(getPatchSet(ps4.getId())).isNull();
}
@Test
@@ -284,7 +284,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
c = db.changes().get(c.getId());
assertThat(c.currentPatchSetId().get()).isEqualTo(1);
assertThat(db.patchSets().get(ps1.getId())).isNotNull();
assertThat(getPatchSet(ps1.getId())).isNotNull();
}
@Test
@@ -442,7 +442,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
c = db.changes().get(c.getId());
PatchSet.Id psId2 = new PatchSet.Id(c.getId(), 2);
assertThat(c.currentPatchSetId()).isEqualTo(psId2);
assertThat(db.patchSets().get(psId2).getRevision().get())
assertThat(getPatchSet(psId2).getRevision().get())
.isEqualTo(mergedAs.name());
assertProblems(c);
@@ -484,7 +484,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
c = db.changes().get(c.getId());
PatchSet.Id psId2 = new PatchSet.Id(c.getId(), 2);
assertThat(c.currentPatchSetId()).isEqualTo(psId2);
assertThat(db.patchSets().get(psId2).getRevision().get())
assertThat(getPatchSet(psId2).getRevision().get())
.isEqualTo(mergedAs.name());
assertProblems(c);

11
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
View File

@@ -47,9 +47,6 @@ public class GetRelatedIT extends AbstractDaemonTest {
@Inject
private ChangeEditModifier editModifier;
@Inject
private ChangeData.Factory changeDataFactory;
@Test
public void getRelatedNoResult() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
@@ -521,7 +518,7 @@ public class GetRelatedIT extends AbstractDaemonTest {
pushHead(testRepo, "refs/for/master", false);
Change ch2 = getChange(c2_1).change();
editModifier.createEdit(ch2, getPatchSet(ch2));
editModifier.createEdit(ch2, getPatchSet(ch2.currentPatchSetId()));
editModifier.modifyFile(editUtil.byChange(ch2).get(), "a.txt",
RestSession.newRawInput(new byte[] {'a'}));
ObjectId editRev =
@@ -569,7 +566,7 @@ public class GetRelatedIT extends AbstractDaemonTest {
}
// Pretend PS1,1 was pushed before the groups field was added.
PatchSet ps1_1 = db.patchSets().get(psId1_1);
PatchSet ps1_1 = getPatchSet(psId1_1);
ps1_1.setGroups(null);
db.patchSets().update(ImmutableList.of(ps1_1));
indexer.index(changeDataFactory.create(db, psId1_1.getParentKey()));
@@ -614,10 +611,6 @@ public class GetRelatedIT extends AbstractDaemonTest {
return getChange(c).change().currentPatchSetId();
}
private PatchSet getPatchSet(Change c) throws OrmException {
return db.patchSets().get(c.currentPatchSetId());
}
private ChangeData getChange(ObjectId c) throws OrmException {
return Iterables.getOnlyElement(queryProvider.get().byCommit(c));
}