ApprovalsUtil: Use ChangeNotes and CurrentUser instead of ChangeControl

Change-Id: I1ff07204f8b6b6cd3f2c00f05b94c4635e1822f7
This commit is contained in:
Patrick Hiesel
2017-09-11 10:58:14 +02:00
parent be911167d6
commit 31a5d24dfd
18 changed files with 112 additions and 68 deletions

View File

@@ -29,9 +29,9 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.git.LabelNormalizer; import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
@@ -79,7 +79,8 @@ public class ApprovalCopier {
* Apply approval copy settings from prior PatchSets to a new PatchSet. * Apply approval copy settings from prior PatchSets to a new PatchSet.
* *
* @param db review database. * @param db review database.
* @param ctl change control for user uploading PatchSet * @param notes change notes for user uploading PatchSet
* @param user user uploading PatchSet
* @param ps new PatchSet * @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand. * @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand. * @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -87,19 +88,21 @@ public class ApprovalCopier {
*/ */
public void copyInReviewDb( public void copyInReviewDb(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet ps, PatchSet ps,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig) @Nullable Config repoConfig)
throws OrmException { throws OrmException {
copyInReviewDb(db, ctl, ps, rw, repoConfig, Collections.emptyList()); copyInReviewDb(db, notes, user, ps, rw, repoConfig, Collections.emptyList());
} }
/** /**
* Apply approval copy settings from prior PatchSets to a new PatchSet. * Apply approval copy settings from prior PatchSets to a new PatchSet.
* *
* @param db review database. * @param db review database.
* @param ctl change control for user uploading PatchSet * @param notes change notes for user uploading PatchSet
* @param user user uploading PatchSet
* @param ps new PatchSet * @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand. * @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand. * @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -108,52 +111,57 @@ public class ApprovalCopier {
*/ */
public void copyInReviewDb( public void copyInReviewDb(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet ps, PatchSet ps,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig, @Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy) Iterable<PatchSetApproval> dontCopy)
throws OrmException { throws OrmException {
if (PrimaryStorage.of(ctl.getChange()) == PrimaryStorage.REVIEW_DB) { if (PrimaryStorage.of(notes.getChange()) == PrimaryStorage.REVIEW_DB) {
db.patchSetApprovals().insert(getForPatchSet(db, ctl, ps, rw, repoConfig, dontCopy)); db.patchSetApprovals().insert(getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy));
} }
} }
Iterable<PatchSetApproval> getForPatchSet( Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet.Id psId, PatchSet.Id psId,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig) @Nullable Config repoConfig)
throws OrmException { throws OrmException {
return getForPatchSet(db, ctl, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList()); return getForPatchSet(
db, notes, user, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
} }
Iterable<PatchSetApproval> getForPatchSet( Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet.Id psId, PatchSet.Id psId,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig, @Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy) Iterable<PatchSetApproval> dontCopy)
throws OrmException { throws OrmException {
PatchSet ps = psUtil.get(db, ctl.getNotes(), psId); PatchSet ps = psUtil.get(db, notes, psId);
if (ps == null) { if (ps == null) {
return Collections.emptyList(); return Collections.emptyList();
} }
return getForPatchSet(db, ctl, ps, rw, repoConfig, dontCopy); return getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy);
} }
private Iterable<PatchSetApproval> getForPatchSet( private Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet ps, PatchSet ps,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig, @Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy) Iterable<PatchSetApproval> dontCopy)
throws OrmException { throws OrmException {
checkNotNull(ps, "ps should not be null"); checkNotNull(ps, "ps should not be null");
ChangeData cd = changeDataFactory.create(db, ctl); ChangeData cd = changeDataFactory.create(db, notes);
try { try {
ProjectState project = projectCache.checkedGet(cd.change().getDest().getParentKey()); ProjectState project = projectCache.checkedGet(cd.change().getDest().getParentKey());
ListMultimap<PatchSet.Id, PatchSetApproval> all = cd.approvals(); ListMultimap<PatchSet.Id, PatchSetApproval> all = cd.approvals();
@@ -204,7 +212,7 @@ public class ApprovalCopier {
byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId())); byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId()));
} }
} }
return labelNormalizer.normalize(ctl, byUser.values()).getNormalized(); return labelNormalizer.normalize(notes, user, byUser.values()).getNormalized();
} catch (IOException | PermissionBackendException e) { } catch (IOException | PermissionBackendException e) {
throw new OrmException(e); throw new OrmException(e);
} }

View File

@@ -49,7 +49,6 @@ import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.LabelVote; import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -385,7 +384,8 @@ public class ApprovalsUtil {
public Iterable<PatchSetApproval> byPatchSet( public Iterable<PatchSetApproval> byPatchSet(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet.Id psId, PatchSet.Id psId,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@Nullable Config repoConfig) @Nullable Config repoConfig)
@@ -393,12 +393,13 @@ public class ApprovalsUtil {
if (!migration.readChanges()) { if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSet(psId)); return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
} }
return copier.getForPatchSet(db, ctl, psId, rw, repoConfig); return copier.getForPatchSet(db, notes, user, psId, rw, repoConfig);
} }
public Iterable<PatchSetApproval> byPatchSetUser( public Iterable<PatchSetApproval> byPatchSetUser(
ReviewDb db, ReviewDb db,
ChangeControl ctl, ChangeNotes notes,
CurrentUser user,
PatchSet.Id psId, PatchSet.Id psId,
Account.Id accountId, Account.Id accountId,
@Nullable RevWalk rw, @Nullable RevWalk rw,
@@ -407,7 +408,7 @@ public class ApprovalsUtil {
if (!migration.readChanges()) { if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSetUser(psId, accountId)); return sortApprovals(db.patchSetApprovals().byPatchSetUser(psId, accountId));
} }
return filterApprovals(byPatchSet(db, ctl, psId, rw, repoConfig), accountId); return filterApprovals(byPatchSet(db, notes, user, psId, rw, repoConfig), accountId);
} }
public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes, PatchSet.Id c) { public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes, PatchSet.Id c) {

View File

@@ -1080,11 +1080,16 @@ public class ChangeJson {
private Map<String, Short> currentLabels(PermissionBackend.ForChange perm, ChangeData cd) private Map<String, Short> currentLabels(PermissionBackend.ForChange perm, ChangeData cd)
throws OrmException { throws OrmException {
IdentifiedUser user = perm.user().asIdentifiedUser(); IdentifiedUser user = perm.user().asIdentifiedUser();
ChangeControl ctl = cd.changeControl().forUser(user);
Map<String, Short> result = new HashMap<>(); Map<String, Short> result = new HashMap<>();
for (PatchSetApproval psa : for (PatchSetApproval psa :
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
db.get(), ctl, cd.change().currentPatchSetId(), user.getAccountId(), null, null)) { db.get(),
cd.notes(),
user,
cd.change().currentPatchSetId(),
user.getAccountId(),
null,
null)) {
result.put(psa.getLabel(), psa.getValue()); result.put(psa.getLabel(), psa.getValue());
} }
return result; return result;

View File

@@ -41,7 +41,6 @@ import com.google.gerrit.server.extensions.events.VoteDeleted;
import com.google.gerrit.server.mail.send.DeleteVoteSender; import com.google.gerrit.server.mail.send.DeleteVoteSender;
import com.google.gerrit.server.mail.send.ReplyToChangeSender; import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl; import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -164,10 +163,9 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
public boolean updateChange(ChangeContext ctx) public boolean updateChange(ChangeContext ctx)
throws OrmException, AuthException, ResourceNotFoundException, IOException, throws OrmException, AuthException, ResourceNotFoundException, IOException,
PermissionBackendException { PermissionBackendException {
ChangeControl ctl = ctx.getControl(); change = ctx.getChange();
change = ctl.getChange();
PatchSet.Id psId = change.currentPatchSetId(); PatchSet.Id psId = change.currentPatchSetId();
ps = psUtil.current(db.get(), ctl.getNotes()); ps = psUtil.current(db.get(), ctx.getNotes());
boolean found = false; boolean found = false;
LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser()); LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
@@ -175,7 +173,8 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
for (PatchSetApproval a : for (PatchSetApproval a :
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getDb(),
ctl, ctx.getNotes(),
ctx.getUser(),
psId, psId,
account.getId(), account.getId(),
ctx.getRevWalk(), ctx.getRevWalk(),

View File

@@ -49,7 +49,6 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.BatchUpdateOp;
@@ -228,7 +227,6 @@ public class PatchSetInserter implements BatchUpdateOp {
public boolean updateChange(ChangeContext ctx) public boolean updateChange(ChangeContext ctx)
throws ResourceConflictException, OrmException, IOException { throws ResourceConflictException, OrmException, IOException {
ReviewDb db = ctx.getDb(); ReviewDb db = ctx.getDb();
ChangeControl ctl = ctx.getControl();
change = ctx.getChange(); change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(psId); ChangeUpdate update = ctx.getUpdate(psId);
@@ -261,7 +259,7 @@ public class PatchSetInserter implements BatchUpdateOp {
description); description);
if (notify != NotifyHandling.NONE) { if (notify != NotifyHandling.NONE) {
oldReviewers = approvalsUtil.getReviewers(db, ctl.getNotes()); oldReviewers = approvalsUtil.getReviewers(db, ctx.getNotes());
} }
if (message != null) { if (message != null) {
@@ -283,7 +281,12 @@ public class PatchSetInserter implements BatchUpdateOp {
change.setCurrentPatchSet(patchSetInfo); change.setCurrentPatchSet(patchSetInfo);
if (copyApprovals) { if (copyApprovals) {
approvalCopier.copyInReviewDb( approvalCopier.copyInReviewDb(
db, ctl, patchSet, ctx.getRevWalk(), ctx.getRepoView().getConfig()); db,
ctx.getNotes(),
ctx.getUser(),
patchSet,
ctx.getRevWalk(),
ctx.getRepoView().getConfig());
} }
if (changeMessage != null) { if (changeMessage != null) {
cmUtil.addChangeMessage(db, update, changeMessage); cmUtil.addChangeMessage(db, update, changeMessage);

View File

@@ -1311,7 +1311,8 @@ public class PostReview
for (PatchSetApproval a : for (PatchSetApproval a :
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getDb(),
ctx.getControl(), ctx.getNotes(),
ctx.getUser(),
psId, psId,
user.getAccountId(), user.getAccountId(),
ctx.getRevWalk(), ctx.getRevWalk(),

View File

@@ -111,7 +111,7 @@ public class ReviewerJson {
perm, perm,
cd, cd,
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
db.get(), ctl, psId, new Account.Id(out._accountId), null, null)); db.get(), cd.notes(), ctl.getUser(), psId, new Account.Id(out._accountId), null, null));
} }
public ReviewerInfo format( public ReviewerInfo format(

View File

@@ -84,7 +84,8 @@ public class Votes implements ChildCollection<ReviewerResource, VoteResource> {
Iterable<PatchSetApproval> byPatchSetUser = Iterable<PatchSetApproval> byPatchSetUser =
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
db.get(), db.get(),
rsrc.getChangeResource().getControl(), rsrc.getChangeResource().getNotes(),
rsrc.getChangeResource().getUser(),
rsrc.getChange().currentPatchSetId(), rsrc.getChange().currentPatchSetId(),
rsrc.getReviewerUser().getAccountId(), rsrc.getReviewerUser().getAccountId(),
null, null,

View File

@@ -28,16 +28,18 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
@@ -76,57 +78,59 @@ public class LabelNormalizer {
} }
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final ChangeControl.GenericFactory changeFactory;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@Inject @Inject
LabelNormalizer( LabelNormalizer(
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeControl.GenericFactory changeFactory,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
PermissionBackend permissionBackend) { PermissionBackend permissionBackend,
ProjectCache projectCache) {
this.db = db; this.db = db;
this.changeFactory = changeFactory;
this.userFactory = userFactory; this.userFactory = userFactory;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
} }
/** /**
* @param change change containing the given approvals. * @param notes change containing the given approvals.
* @param approvals list of approvals. * @param approvals list of approvals.
* @return copies of approvals normalized to the defined ranges for the label type and permissions * @return copies of approvals normalized to the defined ranges for the label type and permissions
* for the user. Approvals for unknown labels are not included in the output, nor are * for the user. Approvals for unknown labels are not included in the output, nor are
* approvals where the user has no permissions for that label. * approvals where the user has no permissions for that label.
* @throws OrmException * @throws OrmException
*/ */
public Result normalize(Change change, Collection<PatchSetApproval> approvals) public Result normalize(ChangeNotes notes, Collection<PatchSetApproval> approvals)
throws OrmException, PermissionBackendException { throws OrmException, PermissionBackendException, IOException {
IdentifiedUser user = userFactory.create(change.getOwner()); IdentifiedUser user = userFactory.create(notes.getChange().getOwner());
return normalize(changeFactory.controlFor(db.get(), change, user), approvals); return normalize(notes, user, approvals);
} }
/** /**
* @param ctl change control containing the given approvals. * @param notes change notes containing the given approvals.
* @param user current user.
* @param approvals list of approvals. * @param approvals list of approvals.
* @return copies of approvals normalized to the defined ranges for the label type and permissions * @return copies of approvals normalized to the defined ranges for the label type and permissions
* for the user. Approvals for unknown labels are not included in the output, nor are * for the user. Approvals for unknown labels are not included in the output, nor are
* approvals where the user has no permissions for that label. * approvals where the user has no permissions for that label.
*/ */
public Result normalize(ChangeControl ctl, Collection<PatchSetApproval> approvals) public Result normalize(
throws PermissionBackendException { ChangeNotes notes, CurrentUser user, Collection<PatchSetApproval> approvals)
throws PermissionBackendException, IOException {
List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size()); List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size()); List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size()); List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
LabelTypes labelTypes = LabelTypes labelTypes =
ctl.getProjectControl().getProjectState().getLabelTypes(ctl.getNotes(), ctl.getUser()); projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes, user);
for (PatchSetApproval psa : approvals) { for (PatchSetApproval psa : approvals) {
Change.Id changeId = psa.getKey().getParentKey().getParentKey(); Change.Id changeId = psa.getKey().getParentKey().getParentKey();
checkArgument( checkArgument(
changeId.equals(ctl.getId()), changeId.equals(notes.getChangeId()),
"Approval %s does not match change %s", "Approval %s does not match change %s",
psa.getKey(), psa.getKey(),
ctl.getChange().getKey()); notes.getChange().getKey());
if (psa.isLegacySubmit()) { if (psa.isLegacySubmit()) {
unchanged.add(psa); unchanged.add(psa);
continue; continue;
@@ -138,7 +142,7 @@ public class LabelNormalizer {
} }
PatchSetApproval copy = copy(psa); PatchSetApproval copy = copy(psa);
applyTypeFloor(label, copy); applyTypeFloor(label, copy);
if (!applyRightFloor(ctl.getNotes(), label, copy)) { if (!applyRightFloor(notes, label, copy)) {
deleted.add(psa); deleted.add(psa);
} else if (copy.getValue() != psa.getValue()) { } else if (copy.getValue() != psa.getValue()) {
updated.add(copy); updated.add(copy);

View File

@@ -39,11 +39,13 @@ import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.strategy.CommitMergeStatus; import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -355,7 +357,7 @@ public class MergeUtil {
PatchSetApproval submitAudit = null; PatchSetApproval submitAudit = null;
for (PatchSetApproval a : safeGetApprovals(ctl, psId)) { for (PatchSetApproval a : safeGetApprovals(ctl.getNotes(), ctl.getUser(), psId)) {
if (a.getValue() <= 0) { if (a.getValue() <= 0) {
// Negative votes aren't counted. // Negative votes aren't counted.
continue; continue;
@@ -448,9 +450,10 @@ public class MergeUtil {
return "Verified".equalsIgnoreCase(id.get()); return "Verified".equalsIgnoreCase(id.get());
} }
private Iterable<PatchSetApproval> safeGetApprovals(ChangeControl ctl, PatchSet.Id psId) { private Iterable<PatchSetApproval> safeGetApprovals(
ChangeNotes notes, CurrentUser user, PatchSet.Id psId) {
try { try {
return approvalsUtil.byPatchSet(db.get(), ctl, psId, null, null); return approvalsUtil.byPatchSet(db.get(), notes, user, psId, null, null);
} catch (OrmException e) { } catch (OrmException e) {
log.error("Can't read approval records for " + psId, e); log.error("Can't read approval records for " + psId, e);
return Collections.emptyList(); return Collections.emptyList();

View File

@@ -311,7 +311,8 @@ public class ReplaceOp implements BatchUpdateOp {
approvals); approvals);
approvalCopier.copyInReviewDb( approvalCopier.copyInReviewDb(
ctx.getDb(), ctx.getDb(),
ctx.getControl(), ctx.getNotes(),
ctx.getUser(),
newPatchSet, newPatchSet,
ctx.getRevWalk(), ctx.getRevWalk(),
ctx.getRepoView().getConfig(), ctx.getRepoView().getConfig(),
@@ -401,7 +402,8 @@ public class ReplaceOp implements BatchUpdateOp {
for (PatchSetApproval a : for (PatchSetApproval a :
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getDb(),
ctx.getControl(), ctx.getNotes(),
ctx.getUser(),
priorPatchSetId, priorPatchSetId,
ctx.getAccountId(), ctx.getAccountId(),
ctx.getRevWalk(), ctx.getRevWalk(),

View File

@@ -358,7 +358,12 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>(); Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
for (PatchSetApproval psa : for (PatchSetApproval psa :
args.approvalsUtil.byPatchSet( args.approvalsUtil.byPatchSet(
ctx.getDb(), ctx.getControl(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { ctx.getDb(),
ctx.getNotes(),
ctx.getUser(),
psId,
ctx.getRevWalk(),
ctx.getRepoView().getConfig())) {
byKey.put(psa.getKey(), psa); byKey.put(psa.getKey(), psa);
} }
@@ -372,7 +377,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
// was added. So we need to make sure votes are accurate now. This way if // was added. So we need to make sure votes are accurate now. This way if
// permissions get modified in the future, historical records stay accurate. // permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized = LabelNormalizer.Result normalized =
args.labelNormalizer.normalize(ctx.getControl(), byKey.values()); args.labelNormalizer.normalize(ctx.getNotes(), ctx.getUser(), byKey.values());
update.putApproval(submitter.getLabel(), submitter.getValue()); update.putApproval(submitter.getLabel(), submitter.getValue());
saveApprovals(normalized, ctx, update, false); saveApprovals(normalized, ctx, update, false);
return normalized; return normalized;

View File

@@ -292,7 +292,8 @@ public class MailProcessor {
approvalsUtil approvalsUtil
.byPatchSetUser( .byPatchSetUser(
ctx.getDb(), ctx.getDb(),
changeControl, changeControl.getNotes(),
changeControl.getUser(),
psId, psId,
ctx.getAccountId(), ctx.getAccountId(),
ctx.getRevWalk(), ctx.getRevWalk(),

View File

@@ -70,7 +70,12 @@ public class MergedSender extends ReplyToChangeSender {
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create(); Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca : for (PatchSetApproval ca :
args.approvalsUtil.byPatchSet( args.approvalsUtil.byPatchSet(
args.db.get(), changeData.changeControl(), patchSet.getId(), null, null)) { args.db.get(),
changeData.notes(),
changeData.changeControl().getUser(),
patchSet.getId(),
null,
null)) {
LabelType lt = labelTypes.byLabel(ca.getLabelId()); LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) { if (lt == null) {
continue; continue;

View File

@@ -293,7 +293,8 @@ public class ChangeControl {
} }
for (PatchSetApproval ap : for (PatchSetApproval ap :
approvalsUtil.byPatchSet(db, this, getChange().currentPatchSetId(), null, null)) { approvalsUtil.byPatchSet(
db, getNotes(), getUser(), getChange().currentPatchSetId(), null, null)) {
LabelType type = LabelType type =
getProjectControl() getProjectControl()
.getProjectState() .getProjectState()

View File

@@ -718,7 +718,8 @@ public class ChangeData {
try { try {
currentApprovals = currentApprovals =
ImmutableList.copyOf( ImmutableList.copyOf(
approvalsUtil.byPatchSet(db, changeControl(), c.currentPatchSetId(), null, null)); approvalsUtil.byPatchSet(
db, notes(), changeControl().getUser(), c.currentPatchSetId(), null, null));
} catch (OrmException e) { } catch (OrmException e) {
if (e.getCause() instanceof NoSuchChangeException) { if (e.getCause() instanceof NoSuchChangeException) {
currentApprovals = Collections.emptyList(); currentApprovals = Collections.emptyList();

View File

@@ -40,6 +40,7 @@ import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.LabelNormalizer.Result; import com.google.gerrit.server.git.LabelNormalizer.Result;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.schema.SchemaCreator; import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestContext;
@@ -69,12 +70,14 @@ public class LabelNormalizerTest {
@Inject private ProjectCache projectCache; @Inject private ProjectCache projectCache;
@Inject private SchemaCreator schemaCreator; @Inject private SchemaCreator schemaCreator;
@Inject protected ThreadLocalRequestContext requestContext; @Inject protected ThreadLocalRequestContext requestContext;
@Inject private ChangeNotes.Factory changeNotesFactory;
private LifecycleManager lifecycle; private LifecycleManager lifecycle;
private ReviewDb db; private ReviewDb db;
private Account.Id userId; private Account.Id userId;
private IdentifiedUser user; private IdentifiedUser user;
private Change change; private Change change;
private ChangeNotes notes;
@Before @Before
public void setUpInjector() throws Exception { public void setUpInjector() throws Exception {
@@ -131,6 +134,7 @@ public class LabelNormalizerTest {
ps.setSubject("Test change"); ps.setSubject("Test change");
change.setCurrentPatchSet(ps); change.setCurrentPatchSet(ps);
db.changes().insert(ImmutableList.of(change)); db.changes().insert(ImmutableList.of(change));
notes = changeNotesFactory.createChecked(db, change);
} }
@After @After
@@ -155,7 +159,7 @@ public class LabelNormalizerTest {
PatchSetApproval cr = psa(userId, "Code-Review", 2); PatchSetApproval cr = psa(userId, "Code-Review", 2);
PatchSetApproval v = psa(userId, "Verified", 1); PatchSetApproval v = psa(userId, "Verified", 1);
assertEquals( assertEquals(
Result.create(list(v), list(copy(cr, 1)), list()), norm.normalize(change, list(cr, v))); Result.create(list(v), list(copy(cr, 1)), list()), norm.normalize(notes, list(cr, v)));
} }
@Test @Test
@@ -169,14 +173,14 @@ public class LabelNormalizerTest {
PatchSetApproval v = psa(userId, "Verified", 5); PatchSetApproval v = psa(userId, "Verified", 5);
assertEquals( assertEquals(
Result.create(list(), list(copy(cr, 2), copy(v, 1)), list()), Result.create(list(), list(copy(cr, 2), copy(v, 1)), list()),
norm.normalize(change, list(cr, v))); norm.normalize(notes, list(cr, v)));
} }
@Test @Test
public void emptyPermissionRangeOmitsResult() throws Exception { public void emptyPermissionRangeOmitsResult() throws Exception {
PatchSetApproval cr = psa(userId, "Code-Review", 1); PatchSetApproval cr = psa(userId, "Code-Review", 1);
PatchSetApproval v = psa(userId, "Verified", 1); PatchSetApproval v = psa(userId, "Verified", 1);
assertEquals(Result.create(list(), list(), list(cr, v)), norm.normalize(change, list(cr, v))); assertEquals(Result.create(list(), list(), list(cr, v)), norm.normalize(notes, list(cr, v)));
} }
@Test @Test
@@ -187,7 +191,7 @@ public class LabelNormalizerTest {
PatchSetApproval cr = psa(userId, "Code-Review", 0); PatchSetApproval cr = psa(userId, "Code-Review", 0);
PatchSetApproval v = psa(userId, "Verified", 0); PatchSetApproval v = psa(userId, "Verified", 0);
assertEquals(Result.create(list(cr), list(), list(v)), norm.normalize(change, list(cr, v))); assertEquals(Result.create(list(cr), list(), list(v)), norm.normalize(notes, list(cr, v)));
} }
private ProjectConfig loadAllProjects() throws Exception { private ProjectConfig loadAllProjects() throws Exception {