In NoteDb fused mode, the new patch set object might not be flushed into
the repo at the time when ApprovalCopier is called to copy patch sets.
ChangeKindCache supports passing in a RevWalk and Config for exactly
this use case, but those weren't previously plumbed through
ApprovalCopier. Do so now.
This doesn't affect other NoteDb migration states because in other cases
the object will have been flushed before calling updateChange.
Without this change, the rebase tests in ChangeIT would show a log entry
like:
WARN com.google.gerrit.server.change.ChangeKindCacheImpl : Cannot check trivial rebase of new patch set 4bf6c821b45761f3cf62a97b9b61ce80171d653b in com.google.gerrit.acceptance.api.change.ChangeIT_rebaseViaRevisionApi_project
java.util.concurrent.ExecutionException: org.eclipse.jgit.errors.MissingObjectException: Missing unknown 4bf6c821b45761f3cf62a97b9b61ce80171d653b
...
at com.google.gerrit.server.change.ChangeKindCacheImpl.getChangeKind(ChangeKindCacheImpl.java:355)
at com.google.gerrit.server.ApprovalCopier.getForPatchSet(ApprovalCopier.java:156)
at com.google.gerrit.server.ApprovalCopier.copy(ApprovalCopier.java:102)
at com.google.gerrit.server.ApprovalCopier.copy(ApprovalCopier.java:88)
at com.google.gerrit.server.change.PatchSetInserter.updateChange(PatchSetInserter.java:279)
at com.google.gerrit.server.change.RebaseChangeOp.updateChange(RebaseChangeOp.java:201)
at com.google.gerrit.server.update.FusedNoteDbBatchUpdate.executeChangeOps(FusedNoteDbBatchUpdate.java:415)
...
This particular log entry turns out to be harmless, because the only
thing this trivial rebase check is used for is copying approvals to the
new patch set, which only happens in ReviewDb anyway. But since the
object flushing issue only happens when we've gone full-NoteDb, failing
to copy these approvals really does no harm, it just causes logspam.
Added some asserts to the existing rebase check anyway.
Despite the fact that there was no correctness issue here, it's still
nicer and could potentially reduce latency on a cache miss if we pass in
the RevWalk we already have.
Finally, rename the ApprovalCopier methods to indicate that copying is
really only a ReviewDb operation, and explicitly short-circuit the call
when NoteDb is the source of truth.
Change-Id: I188829a36081576eff79034cf7a822dbfc7934bc
26aec0d705