opendev/gerrit
Code Issues Proposed changes

Use a common function to unwrap ReviewDb

Browse Source 
Instead of scattered private function provide a common version
in ReviewDbUtil.

Change-Id: I5caa4cfb1deaab610959c6871439914009c21135
This commit is contained in:
Björn Pedersen
2016-06-22 18:02:42 +02:00
parent 33cbdf345d
commit c298aa6da0
14 changed files with 61 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
View File

@@ -45,6 +45,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PostReview;
@@ -60,7 +61,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.TestChangeRebuilderWrapper;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.NoteDbChecker;
import com.google.gerrit.testutil.NoteDbMode;
@@ -264,13 +264,13 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// First write doesn't create the ref, but rebuilding works.
checker.assertNoChangeRef(project, id);
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isNull();
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isNull();
checker.rebuildAndCheckChanges(id);
// Now that there is a ref, writes are "turned on" for this change, and
// NoteDb stays up to date without explicit rebuilding.
gApi.changes().id(id.get()).topic(name("new-topic"));
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isNotNull();
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isNotNull();
checker.checkChanges(id);
}
@@ -323,13 +323,13 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
Change.Id id = r.getPatchSetId().getParentKey();
ObjectId changeMetaId = getMetaRef(project, changeMetaRef(id));
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
changeMetaId.name());
putDraft(user, id, 1, "comment by user");
ObjectId userDraftsId = getMetaRef(
allUsers, refsDraftComments(id, user.getId()));
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
changeMetaId.name()
+ "," + user.getId() + "=" + userDraftsId.name());
@@ -337,7 +337,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
ObjectId adminDraftsId = getMetaRef(
allUsers, refsDraftComments(id, admin.getId()));
assertThat(admin.getId().get()).isLessThan(user.getId().get());
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
changeMetaId.name()
+ "," + admin.getId() + "=" + adminDraftsId.name()
+ "," + user.getId() + "=" + userDraftsId.name());
@@ -345,7 +345,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
putDraft(admin, id, 2, "revised comment by admin");
adminDraftsId = getMetaRef(
allUsers, refsDraftComments(id, admin.getId()));
assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
changeMetaId.name()
+ "," + admin.getId() + "=" + adminDraftsId.name()
+ "," + user.getId() + "=" + userDraftsId.name());
@@ -374,7 +374,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// Check that the bundles are equal.
ChangeBundle actual = ChangeBundle.fromNotes(
plcUtil, notesFactory.create(dbProvider.get(), project, id));
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
}
@@ -425,7 +425,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// Check that the bundles are equal.
ChangeNotes notes = notesFactory.create(dbProvider.get(), project, id);
ChangeBundle actual = ChangeBundle.fromNotes(plcUtil, notes);
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
assertThat(
Iterables.transform(
@@ -464,7 +464,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// Check that the bundles are equal.
ChangeBundle actual = ChangeBundle.fromNotes(
plcUtil, notesFactory.create(dbProvider.get(), project, id));
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
}
@@ -494,7 +494,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
assertChangeUpToDate(false, id);
assertThat(getMetaRef(project, changeMetaRef(id))).isEqualTo(oldMetaId);
ChangeBundle actual = ChangeBundle.fromNotes(plcUtil, notes);
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
assertChangeUpToDate(false, id);
@@ -536,7 +536,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// Not up to date, but the actual returned state matches anyway.
assertDraftsUpToDate(false, id, user);
ChangeBundle actual = ChangeBundle.fromNotes(plcUtil, notes);
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
// Another rebuild attempt succeeds
@@ -564,7 +564,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
setNotesMigration(false, false);
putDraft(user, id, 1, "second comment by user");
ReviewDb db = unwrapDb();
ReviewDb db = getUnwrappedDb();
Change c = db.changes().get(id);
// Leave change meta ID alone so DraftCommentNotes does the rebuild.
NoteDbChangeState bogusState = new NoteDbChangeState(
@@ -591,7 +591,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
assertChangeUpToDate(true, id);
assertDraftsUpToDate(false, id, user);
ChangeBundle actual = ChangeBundle.fromNotes(plcUtil, notes);
ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
ChangeBundle expected = ChangeBundle.fromReviewDb(getUnwrappedDb(), id);
assertThat(actual.differencesFrom(expected)).isEmpty();
// Another rebuild attempt succeeds
@@ -950,7 +950,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
}
private void setInvalidNoteDbState(Change.Id id) throws Exception {
ReviewDb db = unwrapDb();
ReviewDb db = getUnwrappedDb();
Change c = db.changes().get(id);
// In reality we would have NoteDb writes enabled, which would write a real
// state into this field. For tests however, we turn NoteDb writes off, so
@@ -963,7 +963,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
private void assertChangeUpToDate(boolean expected, Change.Id id)
throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
Change c = unwrapDb().changes().get(id);
Change c = getUnwrappedDb().changes().get(id);
assertThat(c).isNotNull();
assertThat(c.getNoteDbState()).isNotNull();
assertThat(NoteDbChangeState.parse(c).isChangeUpToDate(
@@ -975,7 +975,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
private void assertDraftsUpToDate(boolean expected, Change.Id changeId,
TestAccount account) throws Exception {
try (Repository repo = repoManager.openRepository(allUsers)) {
Change c = unwrapDb().changes().get(changeId);
Change c = getUnwrappedDb().changes().get(changeId);
assertThat(c).isNotNull();
assertThat(c.getNoteDbState()).isNotNull();
NoteDbChangeState state = NoteDbChangeState.parse(c);
@@ -1052,11 +1052,8 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
return msg;
}
private ReviewDb unwrapDb() {
private ReviewDb getUnwrappedDb() {
ReviewDb db = dbProvider.get();
if (db instanceof DisabledChangesReviewDbWrapper) {
db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
}
return db;
return ReviewDbUtil.unwrapDb(db);
}
}