Align ChangeColumns methods with Change fields
The documentation previously specified that method order in
ChangeColumns should match @Column order in Change, but this was not
actually true. Fix it.
Rename some ChangeColumns fields to match the underlying column names in
Change. In some cases, there is a mismatch in Change between the field
name and the method name, for example reviewStarted vs.
hasReviewStarted(). ChangeColumns is an AutoValue where the method names
are analogous to fields, so prefer the field name when there is a
mismatch.
Change-Id: I8ba34146bc23832c42f52e5b05bf99d194b1060a
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 5658569..853ed69 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -134,7 +134,7 @@
+ T // readOnlyUntil
+ 1 // isPrivate
+ 1 // workInProgress
- + 1; // hasReviewStarted
+ + 1; // reviewStarted
}
private static int ptr(Object o, int size) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 78734f9..ab7e580 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -95,7 +95,7 @@
@Nullable Timestamp readOnlyUntil,
boolean isPrivate,
boolean workInProgress,
- boolean hasReviewStarted,
+ boolean reviewStarted,
@Nullable Change.Id revertOf) {
checkNotNull(
metaId,
@@ -112,16 +112,16 @@
.lastUpdatedOn(lastUpdatedOn)
.owner(owner)
.branch(branch)
+ .status(status)
.currentPatchSetId(currentPatchSetId)
.subject(subject)
.topic(topic)
.originalSubject(originalSubject)
.submissionId(submissionId)
.assignee(assignee)
- .status(status)
.isPrivate(isPrivate)
- .isWorkInProgress(workInProgress)
- .hasReviewStarted(hasReviewStarted)
+ .workInProgress(workInProgress)
+ .reviewStarted(reviewStarted)
.revertOf(revertOf)
.build())
.pastAssignees(pastAssignees)
@@ -147,7 +147,7 @@
* <p>Notable exceptions include rowVersion and noteDbState, which are only make sense when read
* from NoteDb, so they cannot be cached.
*
- * <p>Fields are in listed column order.
+ * <p>Fields should match the column names in {@link Change}, and are in listed column order.
*/
@AutoValue
abstract static class ChangeColumns {
@@ -162,6 +162,10 @@
// Project not included, as it's not stored anywhere in the meta ref.
abstract String branch();
+ // TODO(dborowitz): Use a sensible default other than null
+ @Nullable
+ abstract Change.Status status();
+
@Nullable
abstract PatchSet.Id currentPatchSetId();
@@ -178,15 +182,12 @@
@Nullable
abstract Account.Id assignee();
- // TODO(dborowitz): Use a sensible default other than null
- @Nullable
- abstract Change.Status status();
abstract boolean isPrivate();
- abstract boolean isWorkInProgress();
+ abstract boolean workInProgress();
- abstract boolean hasReviewStarted();
+ abstract boolean reviewStarted();
@Nullable
abstract Change.Id revertOf();
@@ -219,9 +220,9 @@
abstract Builder isPrivate(boolean isPrivate);
- abstract Builder isWorkInProgress(boolean isWorkInProgress);
+ abstract Builder workInProgress(boolean workInProgress);
- abstract Builder hasReviewStarted(boolean hasReviewStarted);
+ abstract Builder reviewStarted(boolean reviewStarted);
abstract Builder revertOf(@Nullable Change.Id revertOf);
@@ -327,8 +328,8 @@
change.setSubmissionId(c.submissionId());
change.setAssignee(c.assignee());
change.setPrivate(c.isPrivate());
- change.setWorkInProgress(c.isWorkInProgress());
- change.setReviewStarted(c.hasReviewStarted());
+ change.setWorkInProgress(c.workInProgress());
+ change.setReviewStarted(c.reviewStarted());
change.setRevertOf(c.revertOf());
if (!patchSets().isEmpty()) {
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index d974877..b8f544a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -442,17 +442,17 @@
// Change created in WIP remains in WIP.
RevCommit commit = writeCommit("Update WIP change\n" + "\n" + "Patch-set: 1\n", true);
ChangeNotesState state = newParser(commit).parseAll();
- assertThat(state.columns().hasReviewStarted()).isFalse();
+ assertThat(state.columns().reviewStarted()).isFalse();
// Moving change out of WIP starts review.
commit =
writeCommit("New ready change\n" + "\n" + "Patch-set: 1\n" + "Work-in-progress: false\n");
state = newParser(commit).parseAll();
- assertThat(state.columns().hasReviewStarted()).isTrue();
+ assertThat(state.columns().reviewStarted()).isTrue();
// Change created not in WIP has always been in review started state.
state = assertParseSucceeds("New change that doesn't declare WIP\n" + "\n" + "Patch-set: 1\n");
- assertThat(state.columns().hasReviewStarted()).isTrue();
+ assertThat(state.columns().reviewStarted()).isTrue();
}
@Test
