PostReview#forceCallerAsReviewer should set reviewer in notedb

This problem was revealed when using ChangeNotes.Factory in
AbstractQueryChangesTest#updatedOrderWithSubMinuteResolution to load
the change after a review has been posted. Using ChangeNotes.Factory
for loading the change instead of loading it directly from the
database means that we now get the change from notedb when it is
enabled. The test started to fail because posting the empty review
didn't result in any update in the notedb, while the change in the
database was updated by adding the caller as reviewer. For notedb we
should do the same and add the caller as reviewer.

Another problem when loading the change via notedb was that the
timestamps are only precise up to 1s, while loading the change from
the database leads to 1ms precise timestamps. E.g. when the timestamp
from the database is

  2009-09-30 23:00:00.004

the same timestamp from notedb is

  2009-09-30 23:00:00.0

As result the timestamp comparison failed. To fix this the clock step
for this test had to be set to 1s.

Also fixing PostReview#forceCallerAsReviewer caused the
CustomLabelIT#customLabelAnyWithBlock_Addreviewer_ZeroVote to fail
because it was wrongly expecting only one approval when notedb was
enabled. This was done by change I321f84 which assumed that with
notedb no approvals are returned for reviewers that haven't voted
because notedb doesn't insert a dummy approval for them. While it is
true that no dummy approval is inserted, still the presense of a
reviewer leads to an empty approval being returned (see
ChangeJson#setAllApprovals).

Change-Id: I13caedaddfe37986bfe1bd8e4c9916acde501c5f
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.project.Util.category;
 import static com.google.gerrit.server.project.Util.value;
-import static com.google.gerrit.testutil.GerritServerTests.isNoteDbTestEnabled;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
@@ -128,7 +127,7 @@ public class CustomLabelIT extends AbstractDaemonTest {
     revision(r).review(new ReviewInput().label(P.getName(), 0));
     ChangeInfo c = get(r.getChangeId());
     LabelInfo q = c.labels.get(P.getName());
-    assertThat(q.all).hasSize(isNoteDbTestEnabled() ? 1 : 2);
+    assertThat(q.all).hasSize(2);
     assertThat(q.disliked).isNull();
     assertThat(q.rejected).isNull();
     assertThat(q.blocking).isNull();

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -600,6 +600,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
           ups.add(c);
         }
       }
+      ctx.getUpdate(ctx.getChange().currentPatchSetId())
+          .putReviewer(user.getAccountId(), REVIEWER);
     }
 
     private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx,

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -20,6 +20,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
 import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
@@ -59,6 +60,7 @@ import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.index.ChangeField;
 import com.google.gerrit.server.index.IndexCollection;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.RefControl;
@@ -110,6 +112,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Inject protected InMemoryRepositoryManager repoManager;
   @Inject protected InternalChangeQuery internalChangeQuery;
   @Inject protected NotesMigration notesMigration;
+  @Inject protected ChangeNotes.Factory notesFactory;
   @Inject protected PatchSetInserter.Factory patchSetFactory;
   @Inject protected ProjectControl.GenericFactory projectControlFactory;
   @Inject protected QueryProcessor queryProcessor;
@@ -720,6 +723,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
   @Test
   public void updatedOrderWithSubMinuteResolution() throws Exception {
+    TestTimeUtil.resetWithClockStep(1, SECONDS);
+
     TestRepository<Repo> repo = createProject("repo");
     ChangeInserter ins1 = newChange(repo);
     Change change1 = insert(repo, ins1);
@@ -731,7 +736,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
     gApi.changes().id(change1.getId().get()).current()
         .review(new ReviewInput());
-    change1 = db.changes().get(change1.getId());
+    change1 = notesFactory.create(db, change1.getProject(), change1.getId())
+        .getChange();
 
     assertThat(lastUpdatedMs(change1)).isGreaterThan(lastUpdatedMs(change2));
     assertThat(lastUpdatedMs(change1) - lastUpdatedMs(change2))