opendev/gerrit
Code Issues Proposed changes

ChangeNotes: Remove fields that read from ChangeColumns

Browse Source 
The point of ChangeColumns is that the contents are copied into the
relevant columns in the ChangeNotes's Change instance upon loading.
Callers should just be getting these fields from the Change instead;
there weren't any callers outside of tests doing the wrong thing anyway.

Change-Id: Ib6a592e4a5b28add9ed117dbcffb03ac7a507582
This commit is contained in:
Dave Borowitz 2018-05-02 09:08:42 -04:00
parent c92b828746
commit 3080c7d321
2 changed files with 12 additions and 28 deletions
java/com/google/gerrit/server/notedb
ChangeNotes.java
javatests/com/google/gerrit/server/notedb
ChangeNotesTest.java

16
java/com/google/gerrit/server/notedb/ChangeNotes.java

@ -670,22 +670,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return state.readOnlyUntil();
}
public boolean isPrivate() {
return state.columns().isPrivate();
}
public boolean isWorkInProgress() {
return state.columns().isWorkInProgress();
}
public Change.Id getRevertOf() {
return state.columns().revertOf();
}
public boolean hasReviewStarted() {
return state.columns().hasReviewStarted();
}
@Override
protected void onLoad(LoadHandle handle)
throws NoSuchChangeException, IOException, ConfigInvalidException {

24
javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java

@ -3266,7 +3266,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void privateDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isFalse();
assertThat(notes.getChange().isPrivate()).isFalse();
}
@Test
@ -3277,7 +3277,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isTrue();
assertThat(notes.getChange().isPrivate()).isTrue();
}
@Test
@ -3292,7 +3292,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.isPrivate()).isFalse();
assertThat(notes.getChange().isPrivate()).isFalse();
}
@Test
@ -3397,38 +3397,38 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test
public void hasReviewStarted() throws Exception {
ChangeNotes notes = newNotes(newChange());
assertThat(notes.hasReviewStarted()).isTrue();
assertThat(notes.getChange().hasReviewStarted()).isTrue();
notes = newNotes(newWorkInProgressChange());
assertThat(notes.hasReviewStarted()).isFalse();
assertThat(notes.getChange().hasReviewStarted()).isFalse();
Change c = newWorkInProgressChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.commit();
notes = newNotes(c);
assertThat(notes.hasReviewStarted()).isFalse();
assertThat(notes.getChange().hasReviewStarted()).isFalse();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(true);
update.commit();
notes = newNotes(c);
assertThat(notes.hasReviewStarted()).isFalse();
assertThat(notes.getChange().hasReviewStarted()).isFalse();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(false);
update.commit();
notes = newNotes(c);
assertThat(notes.hasReviewStarted()).isTrue();
assertThat(notes.getChange().hasReviewStarted()).isTrue();
// Once review is started, setting WIP should have no impact.
c = newChange();
notes = newNotes(c);
assertThat(notes.hasReviewStarted()).isTrue();
assertThat(notes.getChange().hasReviewStarted()).isTrue();
update = newUpdate(c, changeOwner);
update.setWorkInProgress(true);
update.commit();
notes = newNotes(c);
assertThat(notes.hasReviewStarted()).isTrue();
assertThat(notes.getChange().hasReviewStarted()).isTrue();
}
@Test
@ -3493,7 +3493,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void revertOfIsNullByDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.getRevertOf()).isNull();
assertThat(notes.getChange().getRevertOf()).isNull();
}
@Test
@ -3504,7 +3504,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.setChangeId(c.getKey().get());
update.setRevertOf(changeToRevert.getId().get());
update.commit();
assertThat(newNotes(c).getRevertOf()).isEqualTo(changeToRevert.getId());
assertThat(newNotes(c).getChange().getRevertOf()).isEqualTo(changeToRevert.getId());
}
@Test