opendev/gerrit
Code Issues Proposed changes

Reread change before PostReviewers.Addition#gatherResults

Browse Source 
Previously, we were passing the ChangeNotes that was read prior to
updating the change. This was converted to a ChangeData and passed to
ReviewerJson#format. Without knowing the details of that method, callers
should be assuming that they need to pass the most up-to-date version of
the change. We were just lucky so far that passing stale data didn't
apparently return incorrect results.

Change-Id: I6e63cb9fed7aba4b301ec4a3215589b74fbf1d1f
This commit is contained in:
Dave Borowitz
2018-10-01 14:10:54 -07:00
parent e2aa20cc8b
commit 85bd885a9b
2 changed files with 9 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
java/com/google/gerrit/server/restapi/change/PostReviewers.java
View File

@@ -166,9 +166,11 @@ public class PostReviewers
Change.Id id = rsrc.getChange().getId();
bu.addOp(id, addition.op);
bu.execute();
// TODO(dborowitz): Should this be re-read to take updates into account?
addition.gatherResults(rsrc.getNotes());
}
// Re-read change to take into account results of the update.
addition.gatherResults(
changeDataFactory.create(dbProvider.get(), rsrc.getProject(), rsrc.getId()));
return addition.result;
}
@@ -478,11 +480,10 @@ public class PostReviewers
this.exactMatchFound = exactMatchFound;
}
void gatherResults(ChangeNotes notes) throws OrmException, PermissionBackendException {
void gatherResults(ChangeData cd) throws OrmException, PermissionBackendException {
checkState(op != null, "addition did not result in an update op");
checkState(op.getResult() != null, "op did not return a result");
ChangeData cd = changeDataFactory.create(dbProvider.get(), notes);
// Generate result details and fill AccountLoader. This occurs outside
// the Op because the accounts are in a different table.
PostReviewersOp.Result opResult = op.getResult();