Move ChangeControl#getLabelTypes() to ProjectState and migrate callers
In an effort to make {Ref,Project,Change}Control package-private all
public methods that aren't replaced by PermissionBackend need to move
elsewhere. Both ProjectControl#getLabelTypes() and
ChangeControl#getLabelTypes() rely on the project state, so they
can move there. ChangeControl#getLabelTypes() needs the destination
branch and current user in addition, which are added as arguments.
Change-Id: I080fd93103679552952872dde49cc149a59cd43e
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index afec3b6..070eefd 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -221,7 +221,7 @@
|| (checkReadConfig && perm.ref(RefNames.REFS_CONFIG).testOrFalse(CREATE_CHANGE)));
detail.setConfigVisible(pc.isOwner() || checkReadConfig);
detail.setGroupInfo(buildGroupInfo(local));
- detail.setLabelTypes(pc.getLabelTypes());
+ detail.setLabelTypes(pc.getProjectState().getLabelTypes());
detail.setFileHistoryLinks(getConfigFileLogLinks(projectName.get()));
return detail;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index ac7248b..385baba 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -94,7 +94,6 @@
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final IdentifiedUser.GenericFactory userFactory;
- private final ChangeControl.GenericFactory changeControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetUtil psUtil;
private final ApprovalsUtil approvalsUtil;
@@ -144,7 +143,6 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
IdentifiedUser.GenericFactory userFactory,
- ChangeControl.GenericFactory changeControlFactory,
PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil,
ApprovalsUtil approvalsUtil,
@@ -161,7 +159,6 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.userFactory = userFactory;
- this.changeControlFactory = changeControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil;
this.approvalsUtil = approvalsUtil;
@@ -436,7 +433,7 @@
reviewersToAdd.addAll(extraCC);
}
- LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes();
+ LabelTypes labelTypes = ctl.getProjectControl().getProjectState().getLabelTypes();
approvalsUtil.addReviewers(
db,
update,
@@ -493,7 +490,7 @@
}
@Override
- public void postUpdate(Context ctx) throws OrmException {
+ public void postUpdate(Context ctx) throws OrmException, IOException {
if (sendMail && (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty())) {
Runnable sender =
new Runnable() {
@@ -536,9 +533,11 @@
if (fireRevisionCreated) {
revisionCreated.fire(change, patchSet, ctx.getAccount(), ctx.getWhen(), notify);
if (approvals != null && !approvals.isEmpty()) {
- ChangeControl changeControl =
- changeControlFactory.controlFor(ctx.getDb(), change, ctx.getUser());
- List<LabelType> labels = changeControl.getLabelTypes().getLabelTypes();
+ List<LabelType> labels =
+ projectCache
+ .checkedGet(ctx.getProject())
+ .getLabelTypes(change.getDest(), ctx.getUser())
+ .getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 933705a..c743769 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.BatchUpdateReviewDb;
@@ -48,6 +49,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -73,6 +75,7 @@
private final NotesMigration migration;
private final NotifyUtil notifyUtil;
private final RemoveReviewerControl removeReviewerControl;
+ private final ProjectCache projectCache;
private final Account reviewer;
private final DeleteReviewerInput input;
@@ -95,6 +98,7 @@
NotesMigration migration,
NotifyUtil notifyUtil,
RemoveReviewerControl removeReviewerControl,
+ ProjectCache projectCache,
@Assisted Account reviewerAccount,
@Assisted DeleteReviewerInput input) {
this.approvalsUtil = approvalsUtil;
@@ -107,14 +111,15 @@
this.migration = migration;
this.notifyUtil = notifyUtil;
this.removeReviewerControl = removeReviewerControl;
-
+ this.projectCache = projectCache;
this.reviewer = reviewerAccount;
this.input = input;
}
@Override
public boolean updateChange(ChangeContext ctx)
- throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException {
+ throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException,
+ IOException {
Account.Id reviewerId = reviewer.getId();
if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
throw new ResourceNotFoundException();
@@ -122,7 +127,8 @@
currChange = ctx.getChange();
currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
- LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+ LabelTypes labelTypes =
+ projectCache.checkedGet(ctx.getProject()).getLabelTypes(ctx.getNotes(), ctx.getUser());
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index 7029101..ee93da0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -42,6 +42,8 @@
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
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.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -75,6 +77,7 @@
private final DeleteVoteSender.Factory deleteVoteSenderFactory;
private final NotifyUtil notifyUtil;
private final RemoveReviewerControl removeReviewerControl;
+ private final ProjectCache projectCache;
@Inject
DeleteVote(
@@ -87,7 +90,8 @@
VoteDeleted voteDeleted,
DeleteVoteSender.Factory deleteVoteSenderFactory,
NotifyUtil notifyUtil,
- RemoveReviewerControl removeReviewerControl) {
+ RemoveReviewerControl removeReviewerControl,
+ ProjectCache projectCache) {
super(retryHelper);
this.db = db;
this.approvalsUtil = approvalsUtil;
@@ -98,12 +102,13 @@
this.deleteVoteSenderFactory = deleteVoteSenderFactory;
this.notifyUtil = notifyUtil;
this.removeReviewerControl = removeReviewerControl;
+ this.projectCache = projectCache;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, VoteResource rsrc, DeleteVoteInput input)
- throws RestApiException, UpdateException {
+ throws RestApiException, UpdateException, IOException {
if (input == null) {
input = new DeleteVoteInput();
}
@@ -123,7 +128,13 @@
try (BatchUpdate bu =
updateFactory.create(
db.get(), change.getProject(), r.getControl().getUser(), TimeUtil.nowTs())) {
- bu.addOp(change.getId(), new Op(r.getReviewerUser().getAccount(), rsrc.getLabel(), input));
+ bu.addOp(
+ change.getId(),
+ new Op(
+ projectCache.checkedGet(r.getChange().getProject()),
+ r.getReviewerUser().getAccount(),
+ rsrc.getLabel(),
+ input));
bu.execute();
}
@@ -131,16 +142,19 @@
}
private class Op implements BatchUpdateOp {
+ private final ProjectState projectState;
private final Account account;
private final String label;
private final DeleteVoteInput input;
+
private ChangeMessage changeMessage;
private Change change;
private PatchSet ps;
private Map<String, Short> newApprovals = new HashMap<>();
private Map<String, Short> oldApprovals = new HashMap<>();
- private Op(Account account, String label, DeleteVoteInput input) {
+ private Op(ProjectState projectState, Account account, String label, DeleteVoteInput input) {
+ this.projectState = projectState;
this.account = account;
this.label = label;
this.input = input;
@@ -156,7 +170,7 @@
ps = psUtil.current(db.get(), ctl.getNotes());
boolean found = false;
- LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 88e6665..8c4dd04 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -98,6 +98,8 @@
import com.google.gerrit.server.permissions.PermissionBackend;
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.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -159,6 +161,7 @@
private final NotifyUtil notifyUtil;
private final Config gerritConfig;
private final WorkInProgressOp.Factory workInProgressOpFactory;
+ private final ProjectCache projectCache;
@Inject
PostReview(
@@ -178,7 +181,8 @@
NotesMigration migration,
NotifyUtil notifyUtil,
@GerritServerConfig Config gerritConfig,
- WorkInProgressOp.Factory workInProgressOpFactory) {
+ WorkInProgressOp.Factory workInProgressOpFactory,
+ ProjectCache projectCache) {
super(retryHelper);
this.db = db;
this.changes = changes;
@@ -196,6 +200,7 @@
this.notifyUtil = notifyUtil;
this.gerritConfig = gerritConfig;
this.workInProgressOpFactory = workInProgressOpFactory;
+ this.projectCache = projectCache;
}
@Override
@@ -215,13 +220,15 @@
if (revision.getEdit().isPresent()) {
throw new ResourceConflictException("cannot post review on edit");
}
+ ProjectState projectState = projectCache.checkedGet(revision.getProject());
+ LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes(), revision.getUser());
if (input.onBehalfOf != null) {
- revision = onBehalfOf(revision, input);
+ revision = onBehalfOf(revision, labelTypes, input);
} else if (input.drafts == null) {
input.drafts = DraftHandling.DELETE;
}
if (input.labels != null) {
- checkLabels(revision, input.strictLabels, input.labels);
+ checkLabels(revision, labelTypes, input.strictLabels, input.labels);
}
if (input.comments != null) {
cleanUpComments(input.comments);
@@ -348,7 +355,7 @@
// Add the review op.
bu.addOp(
revision.getChange().getId(),
- new Op(revision.getPatchSet().getId(), input, accountsToNotify));
+ new Op(projectState, revision.getPatchSet().getId(), input, accountsToNotify));
bu.execute();
@@ -411,7 +418,7 @@
}
}
- private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in)
+ private RevisionResource onBehalfOf(RevisionResource rev, LabelTypes labelTypes, ReviewInput in)
throws BadRequestException, AuthException, UnprocessableEntityException, OrmException,
PermissionBackendException, IOException, ConfigInvalidException {
if (in.labels == null || in.labels.isEmpty()) {
@@ -427,7 +434,6 @@
CurrentUser caller = rev.getUser();
PermissionBackend.ForChange perm = rev.permissions().database(db);
- LabelTypes labelTypes = rev.getControl().getLabelTypes();
Iterator<Map.Entry<String, Short>> itr = in.labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
@@ -468,14 +474,14 @@
return new RevisionResource(changes.parse(ctl), rev.getPatchSet());
}
- private void checkLabels(RevisionResource rsrc, boolean strict, Map<String, Short> labels)
+ private void checkLabels(
+ RevisionResource rsrc, LabelTypes labelTypes, boolean strict, Map<String, Short> labels)
throws BadRequestException, AuthException, PermissionBackendException {
- LabelTypes types = rsrc.getControl().getLabelTypes();
PermissionBackend.ForChange perm = rsrc.permissions();
Iterator<Map.Entry<String, Short>> itr = labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
- LabelType lt = types.byLabel(ent.getKey());
+ LabelType lt = labelTypes.byLabel(ent.getKey());
if (lt == null) {
if (strict) {
throw new BadRequestException(
@@ -810,6 +816,7 @@
}
private class Op implements BatchUpdateOp {
+ private final ProjectState projectState;
private final PatchSet.Id psId;
private final ReviewInput in;
private final ListMultimap<RecipientType, Account.Id> accountsToNotify;
@@ -824,9 +831,11 @@
private Map<String, Short> oldApprovals = new HashMap<>();
private Op(
+ ProjectState projectState,
PatchSet.Id psId,
ReviewInput in,
ListMultimap<RecipientType, Account.Id> accountsToNotify) {
+ this.projectState = projectState;
this.psId = psId;
this.in = in;
this.accountsToNotify = checkNotNull(accountsToNotify);
@@ -841,7 +850,7 @@
boolean dirty = false;
dirty |= insertComments(ctx);
dirty |= insertRobotComments(ctx);
- dirty |= updateLabels(ctx);
+ dirty |= updateLabels(projectState, ctx);
dirty |= insertMessage(ctx);
return dirty;
}
@@ -1107,7 +1116,7 @@
return false;
}
- private boolean updateLabels(ChangeContext ctx)
+ private boolean updateLabels(ProjectState projectState, ChangeContext ctx)
throws OrmException, ResourceConflictException, IOException {
Map<String, Short> inLabels =
MoreObjects.firstNonNull(in.labels, Collections.<String, Short>emptyMap());
@@ -1121,8 +1130,8 @@
List<PatchSetApproval> del = new ArrayList<>();
List<PatchSetApproval> ups = new ArrayList<>();
- Map<String, PatchSetApproval> current = scanLabels(ctx, del);
- LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+ Map<String, PatchSetApproval> current = scanLabels(projectState, ctx, del);
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
Map<String, Short> allApprovals =
getAllApprovals(labelTypes, approvalsByKey(current.values()), inLabels);
Map<String, Short> previous =
@@ -1182,7 +1191,7 @@
return false;
}
- forceCallerAsReviewer(ctx, current, ups, del);
+ forceCallerAsReviewer(projectState, ctx, current, ups, del);
ctx.getDb().patchSetApprovals().delete(del);
ctx.getDb().patchSetApprovals().upsert(ups);
return !del.isEmpty() || !ups.isEmpty();
@@ -1260,6 +1269,7 @@
}
private void forceCallerAsReviewer(
+ ProjectState projectState,
ChangeContext ctx,
Map<String, PatchSetApproval> current,
List<PatchSetApproval> ups,
@@ -1269,7 +1279,12 @@
if (del.isEmpty()) {
// If no existing label is being set to 0, hack in the caller
// as a reviewer by picking the first server-wide LabelType.
- LabelId labelId = ctx.getControl().getLabelTypes().getLabelTypes().get(0).getLabelId();
+ LabelId labelId =
+ projectState
+ .getLabelTypes(ctx.getNotes(), ctx.getUser())
+ .getLabelTypes()
+ .get(0)
+ .getLabelId();
PatchSetApproval c = ApprovalsUtil.newApproval(psId, user, labelId, 0, ctx.getWhen());
c.setTag(in.tag);
c.setGranted(ctx.getWhen());
@@ -1287,9 +1302,10 @@
ctx.getUpdate(ctx.getChange().currentPatchSetId()).putReviewer(user.getAccountId(), REVIEWER);
}
- private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx, List<PatchSetApproval> del)
+ private Map<String, PatchSetApproval> scanLabels(
+ ProjectState projectState, ChangeContext ctx, List<PatchSetApproval> del)
throws OrmException, IOException {
- LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
Map<String, PatchSetApproval> current = new HashMap<>();
for (PatchSetApproval a :
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewersOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewersOp.java
index df6d389..aed6cd0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewersOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewersOp.java
@@ -42,6 +42,7 @@
import com.google.gerrit.server.mail.send.AddReviewerSender;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -94,6 +95,7 @@
private final PatchSetUtil psUtil;
private final ReviewerAdded reviewerAdded;
private final AccountCache accountCache;
+ private final ProjectCache projectCache;
private final AddReviewerSender.Factory addReviewerSenderFactory;
private final NotesMigration migration;
private final Provider<IdentifiedUser> user;
@@ -117,6 +119,7 @@
PatchSetUtil psUtil,
ReviewerAdded reviewerAdded,
AccountCache accountCache,
+ ProjectCache projectCache,
AddReviewerSender.Factory addReviewerSenderFactory,
NotesMigration migration,
Provider<IdentifiedUser> user,
@@ -131,6 +134,7 @@
this.psUtil = psUtil;
this.reviewerAdded = reviewerAdded;
this.accountCache = accountCache;
+ this.projectCache = projectCache;
this.addReviewerSenderFactory = addReviewerSenderFactory;
this.migration = migration;
this.user = user;
@@ -161,7 +165,9 @@
ctx.getDb(),
ctx.getNotes(),
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
- rsrc.getControl().getLabelTypes(),
+ projectCache
+ .checkedGet(rsrc.getProject())
+ .getLabelTypes(rsrc.getChange().getDest(), ctx.getUser()),
rsrc.getChange(),
reviewers);
if (addedReviewers.isEmpty()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
index fb151ef..f448d92 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
@@ -44,6 +44,8 @@
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -80,6 +82,7 @@
private final Provider<ReviewDb> dbProvider;
private final ReplacePatchSetSender.Factory replacePatchSetFactory;
private final DraftPublished draftPublished;
+ private final ProjectCache projectCache;
@Inject
public PublishDraftPatchSet(
@@ -91,7 +94,8 @@
PatchSetUtil psUtil,
Provider<ReviewDb> dbProvider,
ReplacePatchSetSender.Factory replacePatchSetFactory,
- DraftPublished draftPublished) {
+ DraftPublished draftPublished,
+ ProjectCache projectCache) {
super(retryHelper);
this.accountResolver = accountResolver;
this.approvalsUtil = approvalsUtil;
@@ -101,12 +105,13 @@
this.dbProvider = dbProvider;
this.replacePatchSetFactory = replacePatchSetFactory;
this.draftPublished = draftPublished;
+ this.projectCache = projectCache;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input)
- throws RestApiException, UpdateException {
+ throws RestApiException, UpdateException, IOException {
return apply(
updateFactory,
rsrc.getUser(),
@@ -117,10 +122,10 @@
private Response<?> apply(
BatchUpdate.Factory updateFactory, CurrentUser u, Change c, PatchSet.Id psId, PatchSet ps)
- throws RestApiException, UpdateException {
+ throws RestApiException, UpdateException, IOException {
try (BatchUpdate bu =
updateFactory.create(dbProvider.get(), c.getProject(), u, TimeUtil.nowTs())) {
- bu.addOp(c.getId(), new Op(psId, ps));
+ bu.addOp(c.getId(), new Op(psId, projectCache.checkedGet(c.getProject()), ps));
bu.execute();
}
return Response.none();
@@ -152,7 +157,7 @@
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
- throws RestApiException, UpdateException {
+ throws RestApiException, UpdateException, IOException {
return publish.apply(
updateFactory,
rsrc.getUser(),
@@ -164,6 +169,7 @@
private class Op implements BatchUpdateOp {
private final PatchSet.Id psId;
+ private final ProjectState projectState;
private PatchSet patchSet;
private Change change;
@@ -171,8 +177,9 @@
private PatchSetInfo patchSetInfo;
private MailRecipients recipients;
- private Op(PatchSet.Id psId, @Nullable PatchSet patchSet) {
+ private Op(PatchSet.Id psId, ProjectState projectState, @Nullable PatchSet patchSet) {
this.psId = psId;
+ this.projectState = projectState;
this.patchSet = patchSet;
}
@@ -212,7 +219,7 @@
}
private void addReviewers(ChangeContext ctx) throws OrmException, IOException {
- LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
Collection<Account.Id> oldReviewers =
approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all();
RevCommit commit =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
index 323f352..a3f5093 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
@@ -112,7 +112,8 @@
List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
- LabelTypes labelTypes = ctl.getLabelTypes();
+ LabelTypes labelTypes =
+ ctl.getProjectControl().getProjectState().getLabelTypes(ctl.getNotes(), ctl.getUser());
for (PatchSetApproval psa : approvals) {
Change.Id changeId = psa.getKey().getParentKey().getParentKey();
checkArgument(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 5851eb5..80c896f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -455,7 +455,7 @@
repo = rp.getRepository();
user = projectControl.getUser().asIdentifiedUser();
project = projectControl.getProject();
- labelTypes = projectControl.getLabelTypes();
+ labelTypes = projectControl.getProjectState().getLabelTypes();
permissions = permissionBackend.user(user).project(project.getNameKey());
receiveId = RequestId.forProject(project.getNameKey());
rejectCommits = BanCommit.loadRejectCommitsMap(rp.getRepository(), rp.getRevWalk());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index a558c6c..0187304 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -52,7 +52,7 @@
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -103,7 +103,6 @@
private final AccountResolver accountResolver;
private final ApprovalCopier approvalCopier;
private final ApprovalsUtil approvalsUtil;
- private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeData.Factory changeDataFactory;
private final ChangeKindCache changeKindCache;
private final ChangeMessagesUtil cmUtil;
@@ -115,6 +114,7 @@
private final MergedByPushOp.Factory mergedByPushOpFactory;
private final PatchSetUtil psUtil;
private final ReplacePatchSetSender.Factory replacePatchSetFactory;
+ private final ProjectCache projectCache;
private final ProjectControl projectControl;
private final Branch.NameKey dest;
@@ -146,7 +146,6 @@
AccountResolver accountResolver,
ApprovalCopier approvalCopier,
ApprovalsUtil approvalsUtil,
- ChangeControl.GenericFactory changeControlFactory,
ChangeData.Factory changeDataFactory,
ChangeKindCache changeKindCache,
ChangeMessagesUtil cmUtil,
@@ -157,6 +156,7 @@
MergedByPushOp.Factory mergedByPushOpFactory,
PatchSetUtil psUtil,
ReplacePatchSetSender.Factory replacePatchSetFactory,
+ ProjectCache projectCache,
@SendEmailExecutor ExecutorService sendEmailExecutor,
@Assisted ProjectControl projectControl,
@Assisted Branch.NameKey dest,
@@ -172,7 +172,6 @@
this.accountResolver = accountResolver;
this.approvalCopier = approvalCopier;
this.approvalsUtil = approvalsUtil;
- this.changeControlFactory = changeControlFactory;
this.changeDataFactory = changeDataFactory;
this.changeKindCache = changeKindCache;
this.cmUtil = cmUtil;
@@ -183,6 +182,7 @@
this.mergedByPushOpFactory = mergedByPushOpFactory;
this.psUtil = psUtil;
this.replacePatchSetFactory = replacePatchSetFactory;
+ this.projectCache = projectCache;
this.sendEmailExecutor = sendEmailExecutor;
this.projectControl = projectControl;
@@ -304,7 +304,7 @@
approvalsUtil.addApprovalsForNewPatchSet(
ctx.getDb(),
update,
- projectControl.getLabelTypes(),
+ projectControl.getProjectState().getLabelTypes(),
newPatchSet,
ctx.getControl(),
approvals);
@@ -318,7 +318,7 @@
approvalsUtil.addReviewers(
ctx.getDb(),
update,
- projectControl.getLabelTypes(),
+ projectControl.getProjectState().getLabelTypes(),
change,
newPatchSet,
info,
@@ -409,7 +409,7 @@
continue;
}
- LabelType lt = projectControl.getLabelTypes().byLabel(a.getLabelId());
+ LabelType lt = projectControl.getProjectState().getLabelTypes().byLabel(a.getLabelId());
if (lt != null) {
current.put(lt.getName(), a);
}
@@ -526,7 +526,7 @@
}
}
- private void fireCommentAddedEvent(Context ctx) throws OrmException {
+ private void fireCommentAddedEvent(Context ctx) throws IOException {
if (approvals.isEmpty()) {
return;
}
@@ -536,9 +536,11 @@
* For labels that are set in this operation, the value was modified, so
* show a transition from an oldValue of 0 to the new value.
*/
- ChangeControl changeControl =
- changeControlFactory.controlFor(ctx.getDb(), notes.getChange(), ctx.getUser());
- List<LabelType> labels = changeControl.getLabelTypes().getLabelTypes();
+ List<LabelType> labels =
+ projectCache
+ .checkedGet(ctx.getProject())
+ .getLabelTypes(notes, ctx.getUser())
+ .getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 6597951..b9649ed 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -18,14 +18,11 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
-import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
-import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.PermissionRange;
-import com.google.gerrit.common.data.RefConfigSection;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -272,29 +269,6 @@
return canAbandon(db) && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
}
- /** All available label types for this change. */
- public LabelTypes getLabelTypes() {
- String destBranch = getChange().getDest().get();
- List<LabelType> all = getProjectControl().getLabelTypes().getLabelTypes();
-
- List<LabelType> r = Lists.newArrayListWithCapacity(all.size());
- for (LabelType l : all) {
- List<String> refs = l.getRefPatterns();
- if (refs == null) {
- r.add(l);
- } else {
- for (String refPattern : refs) {
- if (RefConfigSection.isValid(refPattern) && match(destBranch, refPattern)) {
- r.add(l);
- break;
- }
- }
- }
- }
-
- return new LabelTypes(r);
- }
-
/** All value ranges of any allowed label permission. */
public List<PermissionRange> getLabelRanges() {
return getRefControl().getLabelRanges(isOwner());
@@ -326,7 +300,11 @@
for (PatchSetApproval ap :
approvalsUtil.byPatchSet(db, this, getChange().currentPatchSetId(), null, null)) {
- LabelType type = getLabelTypes().byLabel(ap.getLabel());
+ LabelType type =
+ getProjectControl()
+ .getProjectState()
+ .getLabelTypes(getNotes(), getUser())
+ .byLabel(ap.getLabel());
if (type != null
&& ap.getValue() == 1
&& type.getFunctionName().equalsIgnoreCase("PatchSetLock")) {
@@ -403,10 +381,6 @@
|| getProjectControl().isAdmin();
}
- private boolean match(String destBranch, String refPattern) {
- return RefPatternMatcher.getMatcher(refPattern).match(destBranch, getUser());
- }
-
private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) {
return cd != null ? cd : changeDataFactory.create(db, this);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index ff0070d..2011cd5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -24,7 +24,6 @@
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.GroupReference;
-import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
@@ -143,7 +142,6 @@
private List<SectionMatcher> allSections;
private List<SectionMatcher> localSections;
- private LabelTypes labelTypes;
private Map<String, RefControl> refControls;
private Boolean declaredOwner;
@@ -218,13 +216,6 @@
return state.getProject();
}
- public LabelTypes getLabelTypes() {
- if (labelTypes == null) {
- labelTypes = state.getLabelTypes();
- }
- return labelTypes;
- }
-
/** Returns whether the project is hidden. */
private boolean isHidden() {
return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectJson.java
index 5ffd4da..6f15c7d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectJson.java
@@ -42,10 +42,10 @@
this.webLinks = webLinks;
}
- public ProjectInfo format(ProjectState state) {
- ProjectInfo info = format(state.getProject());
+ public ProjectInfo format(ProjectState projectState) {
+ ProjectInfo info = format(projectState.getProject());
info.labels = new HashMap<>();
- for (LabelType t : state.getLabelTypes().getLabelTypes()) {
+ for (LabelType t : projectState.getLabelTypes().getLabelTypes()) {
LabelTypeInfo labelInfo = new LabelTypeInfo();
labelInfo.values =
t.getValues()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
index 58421b0..d4c344b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
@@ -27,6 +27,7 @@
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.common.data.RefConfigSection;
import com.google.gerrit.common.data.SubscribeSection;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.extensions.api.projects.ThemeInfo;
@@ -46,6 +47,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ProjectLevelConfig;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.googlecode.prolog_cafe.exceptions.CompileException;
@@ -109,6 +111,9 @@
/** If this is all projects, the capabilities used by the server. */
private final CapabilityCollection capabilities;
+ /** All label types applicable to changes in this project. */
+ private LabelTypes labelTypes;
+
@Inject
public ProjectState(
final SitePaths sitePaths,
@@ -406,24 +411,39 @@
return getInheritableBoolean(Project::getMatchAuthorToCommitterDate);
}
+ /** All available label types. */
public LabelTypes getLabelTypes() {
- Map<String, LabelType> types = new LinkedHashMap<>();
- for (ProjectState s : treeInOrder()) {
- for (LabelType type : s.getConfig().getLabelSections().values()) {
- String lower = type.getName().toLowerCase();
- LabelType old = types.get(lower);
- if (old == null || old.canOverride()) {
- types.put(lower, type);
+ if (labelTypes == null) {
+ labelTypes = loadLabelTypes();
+ }
+ return labelTypes;
+ }
+
+ /** All available label types for this change and user. */
+ public LabelTypes getLabelTypes(ChangeNotes notes, CurrentUser user) {
+ return getLabelTypes(notes.getChange().getDest(), user);
+ }
+
+ /** All available label types for this branch and user. */
+ public LabelTypes getLabelTypes(Branch.NameKey destination, CurrentUser user) {
+ List<LabelType> all = getLabelTypes().getLabelTypes();
+
+ List<LabelType> r = Lists.newArrayListWithCapacity(all.size());
+ for (LabelType l : all) {
+ List<String> refs = l.getRefPatterns();
+ if (refs == null) {
+ r.add(l);
+ } else {
+ for (String refPattern : refs) {
+ if (RefConfigSection.isValid(refPattern) && match(destination, refPattern, user)) {
+ r.add(l);
+ break;
+ }
}
}
}
- List<LabelType> all = Lists.newArrayListWithCapacity(types.size());
- for (LabelType type : types.values()) {
- if (!type.getValues().isEmpty()) {
- all.add(type);
- }
- }
- return new LabelTypes(Collections.unmodifiableList(all));
+
+ return new LabelTypes(r);
}
public List<CommentLinkInfo> getCommentLinks() {
@@ -522,4 +542,28 @@
}
return false;
}
+
+ private LabelTypes loadLabelTypes() {
+ Map<String, LabelType> types = new LinkedHashMap<>();
+ for (ProjectState s : treeInOrder()) {
+ for (LabelType type : s.getConfig().getLabelSections().values()) {
+ String lower = type.getName().toLowerCase();
+ LabelType old = types.get(lower);
+ if (old == null || old.canOverride()) {
+ types.put(lower, type);
+ }
+ }
+ }
+ List<LabelType> all = Lists.newArrayListWithCapacity(types.size());
+ for (LabelType type : types.values()) {
+ if (!type.getValues().isEmpty()) {
+ all.add(type);
+ }
+ }
+ return new LabelTypes(Collections.unmodifiableList(all));
+ }
+
+ private boolean match(Branch.NameKey destination, String refPattern, CurrentUser user) {
+ return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), user);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 88bff63..fb19ba2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -73,6 +73,7 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gwtorm.server.OrmException;
@@ -405,6 +406,7 @@
private PersonIdent author;
private PersonIdent committer;
private Integer unresolvedCommentCount;
+ private LabelTypes labelTypes;
private ImmutableList<byte[]> refStates;
private ImmutableList<byte[]> refStatePatterns;
@@ -665,7 +667,17 @@
}
public LabelTypes getLabelTypes() throws OrmException {
- return changeControl().getLabelTypes();
+ if (labelTypes == null) {
+ ProjectState state;
+ try {
+ state = projectCache.checkedGet(project());
+ } catch (IOException e) {
+ throw new OrmException("project state not available", e);
+ }
+ labelTypes =
+ state.getLabelTypes(changeControl().getChange().getDest(), changeControl().getUser());
+ }
+ return labelTypes;
}
public ChangeNotes notes() throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index 879d34b..34fd2a2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -245,9 +245,7 @@
private ChangeAttribute buildChangeAttribute(
ChangeData d, Map<Project.NameKey, Repository> repos, Map<Project.NameKey, RevWalk> revWalks)
throws OrmException, IOException {
- ChangeControl cc = d.changeControl().forUser(user);
-
- LabelTypes labelTypes = cc.getLabelTypes();
+ LabelTypes labelTypes = d.getLabelTypes();
ChangeAttribute c = eventFactory.asChangeAttribute(db, d.change());
eventFactory.extend(c, d.change());
@@ -299,6 +297,7 @@
if (includeCurrentPatchSet) {
PatchSet current = d.currentPatchSet();
+ ChangeControl cc = d.changeControl().forUser(user);
if (current != null && cc.isPatchVisible(current, d.db())) {
c.currentPatchSet = eventFactory.asPatchSetAttribute(db, rw, d.change(), current);
eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index 491455d..c46f2ca 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -33,8 +33,9 @@
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
@@ -165,7 +166,7 @@
customLabels.put(v.label(), v.value());
}
- @Inject private ProjectControl.Factory projectControlFactory;
+ @Inject private ProjectCache projectCache;
@Inject private AllProjectsName allProjects;
@@ -375,14 +376,14 @@
optionList = new ArrayList<>();
customLabels = new HashMap<>();
- ProjectControl allProjectsControl;
+ ProjectState allProjectsState;
try {
- allProjectsControl = projectControlFactory.controlFor(allProjects);
- } catch (NoSuchProjectException e) {
+ allProjectsState = projectCache.checkedGet(allProjects);
+ } catch (IOException e) {
throw die("missing " + allProjects.get());
}
- for (LabelType type : allProjectsControl.getLabelTypes().getLabelTypes()) {
+ for (LabelType type : allProjectsState.getLabelTypes().getLabelTypes()) {
StringBuilder usage = new StringBuilder("score for ").append(type.getName()).append("\n");
for (LabelValue v : type.getValues()) {
