diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java index 4ace9f48d9..120b9af847 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java @@ -39,6 +39,7 @@ import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; @@ -110,7 +111,7 @@ public class ChangeDetailFactory extends Handler { @Override public ChangeDetail call() throws OrmException, NoSuchEntityException, PatchSetInfoNotAvailableException, NoSuchChangeException, - RepositoryNotFoundException, IOException { + RepositoryNotFoundException, IOException, NoSuchProjectException { control = changeControlFactory.validateFor(changeId); final Change change = control.getChange(); final PatchSet patch = db.patchSets().get(change.currentPatchSetId()); @@ -206,7 +207,8 @@ public class ChangeDetailFactory extends Handler { } } - private void load() throws OrmException, NoSuchChangeException { + private void load() throws OrmException, NoSuchChangeException, + NoSuchProjectException { final Change.Status status = detail.getChange().getStatus(); if ((status.equals(Change.Status.NEW) || status.equals(Change.Status.DRAFT)) && testMerge) { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/EditCommitMessageHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/EditCommitMessageHandler.java index 5e58a35656..5b064c8576 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/EditCommitMessageHandler.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/EditCommitMessageHandler.java @@ -36,6 +36,7 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -114,7 +115,7 @@ class EditCommitMessageHandler extends Handler { public ChangeDetail call() throws NoSuchChangeException, OrmException, EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException, MissingObjectException, IncorrectObjectTypeException, IOException, - InvalidChangeOperationException { + InvalidChangeOperationException, NoSuchProjectException { final Change.Id changeId = patchSetId.getParentKey(); final ChangeControl control = changeControlFactory.validateFor(changeId); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java index 4755c6f8b3..06c6964f38 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java @@ -15,7 +15,6 @@ package com.google.gerrit.httpd.rpc.changedetail; import com.google.gerrit.common.data.LabelType; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.PatchSetPublishDetail; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.SubmitRecord; @@ -47,7 +46,6 @@ final class PatchSetPublishDetailFactory extends Handler private final PatchSetInfoFactory infoFactory; private final ReviewDb db; private final ChangeControl.Factory changeControlFactory; - private final LabelTypes labelTypes; private final AccountInfoCacheFactory aic; private final IdentifiedUser user; @@ -62,12 +60,10 @@ final class PatchSetPublishDetailFactory extends Handler final ReviewDb db, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, final ChangeControl.Factory changeControlFactory, - final LabelTypes labelTypes, final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) { this.infoFactory = infoFactory; this.db = db; this.changeControlFactory = changeControlFactory; - this.labelTypes = labelTypes; this.aic = accountInfoCacheFactory.create(); this.user = user; @@ -124,7 +120,7 @@ final class PatchSetPublishDetailFactory extends Handler boolean canMakeOk = false; PermissionRange range = rangeByName.get(lbl.label); if (range != null) { - LabelType lt = labelTypes.byLabel(lbl.label); + LabelType lt = control.getLabelTypes().byLabel(lbl.label); if (lt == null || lt.getMax().getValue() == range.getMax()) { canMakeOk = true; } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java index f57b29c449..38f8fc62bf 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java @@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -53,7 +54,8 @@ class PublishAction extends Handler { @Override public ChangeDetail call() throws OrmException, NoSuchEntityException, IllegalStateException, PatchSetInfoNotAvailableException, - NoSuchChangeException, RepositoryNotFoundException, IOException { + NoSuchChangeException, RepositoryNotFoundException, IOException, + NoSuchProjectException { final ReviewResult result = publishFactory.create(patchSetId).call(); if (result.getErrors().size() > 0) { throw new IllegalStateException("Cannot publish patchset"); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RebaseChangeHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RebaseChangeHandler.java index b47c8f2c3f..b9acfa9c6c 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RebaseChangeHandler.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RebaseChangeHandler.java @@ -24,6 +24,7 @@ import com.google.gerrit.server.changedetail.RebaseChange; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -60,7 +61,7 @@ class RebaseChangeHandler extends Handler { public ChangeDetail call() throws NoSuchChangeException, OrmException, EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException, MissingObjectException, IncorrectObjectTypeException, IOException, - InvalidChangeOperationException { + InvalidChangeOperationException, NoSuchProjectException { rebaseChange.rebase(patchSetId, currentUser.getAccountId()); return changeDetailFactory.create(patchSetId.getParentKey()).call(); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index 0177ac19c2..b019b58581 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java @@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.patch; import com.google.gerrit.common.data.ApprovalSummary; import com.google.gerrit.common.data.ApprovalSummarySet; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.PatchDetailService; import com.google.gerrit.common.data.PatchScript; @@ -39,6 +38,7 @@ import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.workflow.FunctionState; import com.google.gwtjsonrpc.common.AsyncCallback; import com.google.gwtjsonrpc.common.VoidResult; @@ -56,8 +56,6 @@ import java.util.Set; class PatchDetailServiceImpl extends BaseServiceImplementation implements PatchDetailService { - private final LabelTypes labelTypes; - private final AccountInfoCacheFactory.Factory accountInfoCacheFactory; private final ChangeControl.Factory changeControlFactory; private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; @@ -69,7 +67,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements @Inject PatchDetailServiceImpl(final Provider schema, final Provider currentUser, - final LabelTypes labelTypes, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, final ChangeControl.Factory changeControlFactory, final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory, @@ -78,7 +75,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements final SaveDraft.Factory saveDraftFactory, final ChangeDetailFactory.Factory changeDetailFactory) { super(schema, currentUser); - this.labelTypes = labelTypes; this.accountInfoCacheFactory = accountInfoCacheFactory; this.changeControlFactory = changeControlFactory; @@ -149,6 +145,8 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements return changeDetailFactory.create(result.getChangeId()).call(); } catch (NoSuchChangeException e) { throw new Failure(new NoSuchChangeException(result.getChangeId())); + } catch (NoSuchProjectException e) { + throw new Failure(e); } catch (NoSuchEntityException e) { throw new Failure(e); } catch (PatchSetInfoNotAvailableException e) { @@ -189,7 +187,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements continue; } if (change.getStatus().isOpen()) { - fs.normalize(labelTypes.byId(category.get()), ca); + fs.normalize(cc.getLabelTypes().byId(category.get()), ca); } if (ca.getValue() == 0) { continue; @@ -235,7 +233,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements continue; } if (change.getStatus().isOpen()) { - fs.normalize(labelTypes.byId(category.get()), ca); + fs.normalize(cc.getLabelTypes().byId(category.get()), ca); } if (ca.getValue() == 0) { continue; diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java index 79a444509a..6e8639452f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java @@ -206,8 +206,6 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { private final AccountCache accountCache; - private final LabelTypes labelTypes; - private final EventFactory eventFactory; private final SitePaths sitePaths; @@ -232,16 +230,17 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { final GitRepositoryManager repoManager, final @GerritServerConfig Config config, final @AnonymousCowardName String anonymousCowardName, - final SitePaths sitePath, final ProjectCache projectCache, - final AccountCache accountCache, final LabelTypes labelTypes, - final EventFactory eventFactory, final SitePaths sitePaths, + final SitePaths sitePath, + final ProjectCache projectCache, + final AccountCache accountCache, + final EventFactory eventFactory, + final SitePaths sitePaths, final DynamicSet unrestrictedListeners) { this.anonymousCowardName = anonymousCowardName; this.repoManager = repoManager; this.hookQueue = queue.createQueue(1, "hook"); this.projectCache = projectCache; this.accountCache = accountCache; - this.labelTypes = labelTypes; this.eventFactory = eventFactory; this.sitePaths = sitePath; this.unrestrictedListeners = unrestrictedListeners; @@ -395,11 +394,12 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { event.patchSet = eventFactory.asPatchSetAttribute(patchSet); event.comment = comment; + LabelTypes labelTypes = projectCache.get(change.getProject()).getLabelTypes(); if (approvals.size() > 0) { event.approvals = new ApprovalAttribute[approvals.size()]; int i = 0; for (Map.Entry approval : approvals.entrySet()) { - event.approvals[i++] = getApprovalAttribute(approval); + event.approvals[i++] = getApprovalAttribute(labelTypes, approval); } } @@ -612,7 +612,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener { * @param approval * @return object suitable for serialization to JSON */ - private ApprovalAttribute getApprovalAttribute( + private ApprovalAttribute getApprovalAttribute(LabelTypes labelTypes, Entry approval) { ApprovalAttribute a = new ApprovalAttribute(); a.type = approval.getKey().get(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java index 078c2262d4..5fe9a871af 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java @@ -46,20 +46,17 @@ import java.util.Set; */ public class ApprovalsUtil { private final ReviewDb db; - private final LabelTypes labelTypes; @Inject - public ApprovalsUtil(ReviewDb db, LabelTypes labelTypes) { + public ApprovalsUtil(ReviewDb db) { this.db = db; - this.labelTypes = labelTypes; } /** * Resync the changeOpen status which is cached in the approvals table for * performance reasons */ - public void syncChangeStatus(final Change change) - throws OrmException { + public void syncChangeStatus(final Change change) throws OrmException { final List approvals = db.patchSetApprovals().byChange(change.getId()).toList(); for (PatchSetApproval a : approvals) { @@ -78,7 +75,7 @@ public class ApprovalsUtil { * @return List The previous approvals */ public List copyVetosToPatchSet(ReviewDb db, - PatchSet.Id dest) throws OrmException { + LabelTypes labelTypes, PatchSet.Id dest) throws OrmException { PatchSet.Id source; if (dest.get() > 1) { source = new PatchSet.Id(dest.getParentKey(), dest.get() - 1); @@ -103,8 +100,8 @@ public class ApprovalsUtil { return patchSetApprovals; } - public void addReviewers(ReviewDb db, Change change, PatchSet ps, - PatchSetInfo info, Set wantReviewers, + public void addReviewers(ReviewDb db, LabelTypes labelTypes, Change change, + PatchSet ps, PatchSetInfo info, Set wantReviewers, Set existingReviewers) throws OrmException { List allTypes = labelTypes.getLabelTypes(); if (allTypes.isEmpty()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 7127dce22f..480cd66025 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -40,6 +40,7 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.util.IdGenerator; import com.google.gwtorm.server.AtomicUpdate; @@ -176,7 +177,8 @@ public class ChangeUtil { db.trackingIds().delete(toDelete); } - public static void testMerge(MergeOp.Factory opFactory, Change change) { + public static void testMerge(MergeOp.Factory opFactory, Change change) + throws NoSuchProjectException { opFactory.create(change.getDest()).verifyMergeability(change); } @@ -443,7 +445,9 @@ public class ChangeUtil { "Change %s was modified", change.getId())); } - approvalsUtil.copyVetosToPatchSet(db, change.currentPatchSetId()); + approvalsUtil.copyVetosToPatchSet(db, + refControl.getProjectControl().getLabelTypes(), + change.currentPatchSetId()); final List footerLines = newCommit.getFooterLines(); updateTrackingIds(db, change, trackingFooters, footerLines); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 79083912a7..f766297a6f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -101,7 +101,7 @@ public class Abandon implements RestModifyView { } message = newMessage(input, caller, change); db.changeMessages().insert(Collections.singleton(message)); - new ApprovalsUtil(db, null).syncChangeStatus(change); + new ApprovalsUtil(db).syncChangeStatus(change); db.commit(); } finally { db.rollback(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index bc8cc264e4..01c337cb87 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -114,7 +114,6 @@ public class ChangeJson { } private final Provider db; - private final LabelTypes labelTypes; private final CurrentUser user; private final AnonymousUser anonymous; private final IdentifiedUser.GenericFactory userFactory; @@ -133,7 +132,6 @@ public class ChangeJson { @Inject ChangeJson( Provider db, - LabelTypes at, CurrentUser u, AnonymousUser au, IdentifiedUser.GenericFactory uf, @@ -144,7 +142,6 @@ public class ChangeJson { @CanonicalWebUrl Provider curl, Urls urls) { this.db = db; - this.labelTypes = at; this.user = u; this.anonymous = au; this.userFactory = uf; @@ -321,16 +318,18 @@ public class ChangeJson { return null; } + LabelTypes labelTypes = ctl.getLabelTypes(); if (cd.getChange().getStatus().isOpen()) { - return labelsForOpenChange(cd, standard, detailed); + return labelsForOpenChange(cd, labelTypes, standard, detailed); } else { - return labelsForClosedChange(cd, standard, detailed); + return labelsForClosedChange(cd, labelTypes, standard, detailed); } } private Map labelsForOpenChange(ChangeData cd, - boolean standard, boolean detailed) throws OrmException { - Map labels = initLabels(cd, standard); + LabelTypes labelTypes, boolean standard, boolean detailed) + throws OrmException { + Map labels = initLabels(cd, labelTypes, standard); if (detailed) { setAllApprovals(cd, labels); } @@ -349,8 +348,8 @@ public class ChangeJson { return labels; } - private Map initLabels(ChangeData cd, boolean standard) - throws OrmException { + private Map initLabels(ChangeData cd, + LabelTypes labelTypes, boolean standard) throws OrmException { // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167. Map labels = new TreeMap(LabelOrdering.create(labelTypes)); @@ -448,7 +447,7 @@ public class ChangeJson { } for (PatchSetApproval psa : current.get(accountId)) { // TODO Support arbitrary labels placed by a reviewer. - LabelType lt = labelTypes.byId(psa.getCategoryId().get()); + LabelType lt = ctl.getLabelTypes().byId(psa.getCategoryId().get()); if (lt == null) { continue; } @@ -465,7 +464,8 @@ public class ChangeJson { } private Map labelsForClosedChange(ChangeData cd, - boolean standard, boolean detailed) throws OrmException { + LabelTypes labelTypes, boolean standard, boolean detailed) + throws OrmException { Set allUsers = Sets.newHashSet(); for (PatchSetApproval psa : cd.allApprovals(db)) { allUsers.add(psa.getAccountId()); @@ -570,6 +570,7 @@ public class ChangeJson { return null; } + LabelTypes labelTypes = ctl.getLabelTypes(); ListMultimap permitted = LinkedListMultimap.create(); for (SubmitRecord rec : submitRecords(cd)) { if (rec.labels == null) { 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 bb38b9e382..647853afb7 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 @@ -102,7 +102,6 @@ public class PostReview implements RestModifyView { } private final ReviewDb db; - private final LabelTypes labelTypes; private final EmailReviewComments.Factory email; @Deprecated private final ChangeHooks hooks; @@ -116,11 +115,9 @@ public class PostReview implements RestModifyView { @Inject PostReview(ReviewDb db, - LabelTypes labelTypes, EmailReviewComments.Factory email, ChangeHooks hooks) { this.db = db; - this.labelTypes = labelTypes; this.email = email; this.hooks = hooks; } @@ -180,7 +177,8 @@ public class PostReview implements RestModifyView { Map.Entry ent = itr.next(); // TODO Support more generic label assignments. - LabelType lt = labelTypes.byLabel(ent.getKey()); + LabelType lt = revision.getControl().getLabelTypes() + .byLabel(ent.getKey()); if (lt == null) { if (strict) { throw new BadRequestException(String.format( @@ -343,6 +341,7 @@ public class PostReview implements RestModifyView { List upd = Lists.newArrayList(); Map current = scanLabels(rsrc, del); + LabelTypes labelTypes = rsrc.getControl().getLabelTypes(); for (Map.Entry ent : labels.entrySet()) { // TODO Support arbitrary label names. LabelType lt = labelTypes.byLabel(ent.getKey()); @@ -406,7 +405,8 @@ public class PostReview implements RestModifyView { PatchSetApproval c = new PatchSetApproval(new PatchSetApproval.Key( rsrc.getPatchSet().getId(), rsrc.getAccountId(), - labelTypes.getLabelTypes().get(0).getApprovalCategoryId()), + rsrc.getControl().getLabelTypes().getLabelTypes().get(0) + .getApprovalCategoryId()), (short) 0); c.setGranted(timestamp); c.cache(change); @@ -426,6 +426,7 @@ public class PostReview implements RestModifyView { private Map scanLabels(RevisionResource rsrc, List del) throws OrmException { + LabelTypes labelTypes = rsrc.getControl().getLabelTypes(); Map current = Maps.newHashMap(); for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser( rsrc.getPatchSet().getId(), rsrc.getAccountId())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index c8f31a314c..c95f4edd0a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -22,7 +22,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.data.GroupDescription; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.extensions.restapi.AuthException; @@ -84,7 +83,6 @@ public class PostReviewers implements RestModifyView { private final Provider db; private final IdentifiedUser currentUser; private final IdentifiedUser.GenericFactory identifiedUserFactory; - private final ApprovalCategory.Id addReviewerCategoryId; private final Config cfg; private final ChangeHooks hooks; private final AccountCache accountCache; @@ -100,7 +98,6 @@ public class PostReviewers implements RestModifyView { Provider db, IdentifiedUser currentUser, IdentifiedUser.GenericFactory identifiedUserFactory, - LabelTypes labelTypes, @GerritServerConfig Config cfg, ChangeHooks hooks, AccountCache accountCache, @@ -118,9 +115,6 @@ public class PostReviewers implements RestModifyView { this.hooks = hooks; this.accountCache = accountCache; this.json = json; - - this.addReviewerCategoryId = Iterables.getLast(labelTypes.getLabelTypes()) - .getApprovalCategoryId(); } @Override @@ -234,7 +228,7 @@ public class PostReviewers implements RestModifyView { continue; } ChangeControl control = rsrc.getControl().forUser(user); - PatchSetApproval psa = dummyApproval(control.getChange(), psid, id); + PatchSetApproval psa = dummyApproval(control, psid, id); result.reviewers.add(json.format( new ReviewerInfo(id), control, ImmutableList.of(psa))); toInsert.add(psa); @@ -283,12 +277,13 @@ public class PostReviewers implements RestModifyView { || AccountGroup.REGISTERED_USERS.equals(groupUUID)); } - private PatchSetApproval dummyApproval(Change change, PatchSet.Id patchSetId, - Account.Id reviewerId) { - PatchSetApproval dummyApproval = - new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId, - addReviewerCategoryId), (short) 0); - dummyApproval.cache(change); + private PatchSetApproval dummyApproval(ChangeControl ctl, + PatchSet.Id patchSetId, Account.Id reviewerId) { + ApprovalCategory.Id id = new ApprovalCategory.Id( + Iterables.getLast(ctl.getLabelTypes().getLabelTypes()).getId()); + PatchSetApproval dummyApproval = new PatchSetApproval( + new PatchSetApproval.Key(patchSetId, reviewerId, id), (short) 0); + dummyApproval.cache(ctl.getChange()); return dummyApproval; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java index 7bae29379d..afb58f9f99 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java @@ -100,7 +100,7 @@ public class Restore implements RestModifyView { } message = newMessage(input, caller, change); db.changeMessages().insert(Collections.singleton(message)); - new ApprovalsUtil(db, null).syncChangeStatus(change); + new ApprovalsUtil(db).syncChangeStatus(change); db.commit(); } finally { db.rollback(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java index c3c951d535..2893cc3d6b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java @@ -43,17 +43,14 @@ import java.util.TreeMap; public class ReviewerJson { private final Provider db; - private final LabelTypes labelTypes; private final FunctionState.Factory functionState; private final AccountInfo.Loader.Factory accountLoaderFactory; @Inject ReviewerJson(Provider db, - LabelTypes labelTypes, FunctionState.Factory functionState, AccountInfo.Loader.Factory accountLoaderFactory) { this.db = db; - this.labelTypes = labelTypes; this.functionState = functionState; this.accountLoaderFactory = accountLoaderFactory; } @@ -84,6 +81,7 @@ public class ReviewerJson { .byPatchSetUser(psId, out._id)); } + LabelTypes labelTypes = ctl.getLabelTypes(); FunctionState fs = functionState.create(ctl, psId, approvals); for (LabelType at : labelTypes.getLabelTypes()) { CategoryFunction.forType(at).run(at, fs); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java index 1cf0bd24c0..22eae2d9fa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; import com.google.gerrit.common.ChangeHooks; +import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.ReviewResult; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; @@ -108,6 +109,7 @@ public class PublishDraft implements Callable { final Change.Id changeId = patchSetId.getParentKey(); result.setChangeId(changeId); final ChangeControl control = changeControlFactory.validateFor(changeId); + final LabelTypes labelTypes = control.getLabelTypes(); final PatchSet patch = db.patchSets().get(patchSetId); if (patch == null) { throw new NoSuchChangeException(changeId); @@ -147,7 +149,8 @@ public class PublishDraft implements Callable { hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, db); sendNotifications(control.getChange().getStatus() == Change.Status.DRAFT, - (IdentifiedUser) control.getCurrentUser(), updatedChange, updatedPatchSet); + (IdentifiedUser) control.getCurrentUser(), updatedChange, updatedPatchSet, + labelTypes); } } @@ -156,8 +159,8 @@ public class PublishDraft implements Callable { private void sendNotifications(final boolean newChange, final IdentifiedUser currentUser, final Change updatedChange, - final PatchSet updatedPatchSet) throws OrmException, IOException, - PatchSetInfoNotAvailableException { + final PatchSet updatedPatchSet, final LabelTypes labelTypes) + throws OrmException, IOException, PatchSetInfoNotAvailableException { final Repository git = repoManager.openRepository(updatedChange.getProject()); try { final RevWalk revWalk = new RevWalk(git); @@ -175,7 +178,7 @@ public class PublishDraft implements Callable { recipients.remove(me); if (newChange) { - approvalsUtil.addReviewers(db, updatedChange, updatedPatchSet, info, + approvalsUtil.addReviewers(db, labelTypes, updatedChange, updatedPatchSet, info, recipients.getReviewers(), Collections. emptySet()); try { CreateChangeSender cm = createChangeSenderFactory.create(updatedChange); @@ -192,7 +195,7 @@ public class PublishDraft implements Callable { db.patchSetApprovals().byChange(updatedChange.getId()).toList(); final MailRecipients oldRecipients = getRecipientsFromApprovals(patchSetApprovals); - approvalsUtil.addReviewers(db, updatedChange, updatedPatchSet, info, + approvalsUtil.addReviewers(db, labelTypes, updatedChange, updatedPatchSet, info, recipients.getReviewers(), oldRecipients.getAll()); final ChangeMessage msg = new ChangeMessage(new ChangeMessage.Key(updatedChange.getId(), diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java index 98c0133f8b..5c4fdcaf2d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.changedetail; import com.google.common.collect.Sets; import com.google.gerrit.common.ChangeHookRunner; +import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; @@ -373,7 +374,10 @@ public class RebaseChange { "Change %s was modified", change.getId())); } - approvalsUtil.copyVetosToPatchSet(db, change.currentPatchSetId()); + final LabelTypes labelTypes = changeControlFactory.controlFor(change) + .getLabelTypes(); + approvalsUtil.copyVetosToPatchSet(db, labelTypes, + change.currentPatchSetId()); final ChangeMessage cmsg = new ChangeMessage(new ChangeMessage.Key(change.getId(), diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/LabelTypesProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/LabelTypesProvider.java index 08ca057888..0aa396b1a9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/LabelTypesProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/LabelTypesProvider.java @@ -53,7 +53,7 @@ public class LabelTypesProvider implements Provider { db.close(); } } catch (OrmException e) { - throw new ProvisionException("Cannot query approval categories", e); + throw new ProvisionException("Cannot query label categories", e); } return new LabelTypes(Collections.unmodifiableList(types)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index 7637e880aa..acdbcc616c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -62,7 +62,6 @@ public class EventFactory { private static final Logger log = LoggerFactory.getLogger(EventFactory.class); private final AccountCache accountCache; private final Provider urlProvider; - private final LabelTypes labelTypes; private final PatchListCache patchListCache; private final SchemaFactory schema; private final PatchSetInfoFactory psInfoFactory; @@ -71,13 +70,11 @@ public class EventFactory { @Inject EventFactory(AccountCache accountCache, @CanonicalWebUrl @Nullable Provider urlProvider, - LabelTypes labelTypes, - final PatchSetInfoFactory psif, + PatchSetInfoFactory psif, PatchListCache patchListCache, SchemaFactory schema, @GerritPersonIdent PersonIdent myIdent) { this.accountCache = accountCache; this.urlProvider = urlProvider; - this.labelTypes = labelTypes; this.patchListCache = patchListCache; this.schema = schema; this.psInfoFactory = psif; @@ -244,25 +241,26 @@ public class EventFactory { a.commitMessage = commitMessage; } - public void addPatchSets(ChangeAttribute a, Collection ps) { - addPatchSets(a, ps, null, false, null); + public void addPatchSets(ChangeAttribute a, Collection ps, + LabelTypes labelTypes) { + addPatchSets(a, ps, null, false, null, labelTypes); } public void addPatchSets(ChangeAttribute ca, Collection ps, - Map> approvals) { - addPatchSets(ca, ps, approvals, false, null); + Map> approvals, + LabelTypes labelTypes) { + addPatchSets(ca, ps, approvals, false, null, labelTypes); } public void addPatchSets(ChangeAttribute ca, Collection ps, - Map> approvals, - boolean includeFiles, Change change) { - + Map> approvals, + boolean includeFiles, Change change, LabelTypes labelTypes) { if (!ps.isEmpty()) { ca.patchSets = new ArrayList(ps.size()); for (PatchSet p : ps) { PatchSetAttribute psa = asPatchSetAttribute(p); if (approvals != null) { - addApprovals(psa, p.getId(), approvals); + addApprovals(psa, p.getId(), approvals, labelTypes); } ca.patchSets.add(psa); if (includeFiles && change != null) { @@ -381,20 +379,21 @@ public class EventFactory { } public void addApprovals(PatchSetAttribute p, PatchSet.Id id, - Map> all) { + Map> all, + LabelTypes labelTypes) { Collection list = all.get(id); if (list != null) { - addApprovals(p, list); + addApprovals(p, list, labelTypes); } } public void addApprovals(PatchSetAttribute p, - Collection list) { + Collection list, LabelTypes labelTypes) { if (!list.isEmpty()) { p.approvals = new ArrayList(list.size()); for (PatchSetApproval a : list) { if (a.getValue() != 0) { - p.approvals.add(asApprovalAttribute(a)); + p.approvals.add(asApprovalAttribute(a, labelTypes)); } } if (p.approvals.isEmpty()) { @@ -451,9 +450,11 @@ public class EventFactory { * serialization to JSON. * * @param approval + * @param labelTypes label types for the containing project * @return object suitable for serialization to JSON */ - public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval) { + public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval, + LabelTypes labelTypes) { ApprovalAttribute a = new ApprovalAttribute(); a.type = approval.getCategoryId().get(); a.value = Short.toString(approval.getValue()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 4a16399e70..965d4e15e5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -26,9 +26,9 @@ import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.gerrit.common.ChangeHooks; +import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; -import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.ApprovalCategory; @@ -52,6 +52,7 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; 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.ProjectState; import com.google.gerrit.server.util.RequestScopePropagator; @@ -130,7 +131,6 @@ public class MergeOp { private final GitReferenceUpdated gitRefUpdated; private final MergedSender.Factory mergedSenderFactory; private final MergeFailSender.Factory mergeFailSenderFactory; - private final LabelTypes labelTypes; private final PatchSetInfoFactory patchSetInfoFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory; private final ChangeControl.GenericFactory changeControlFactory; @@ -182,7 +182,6 @@ public class MergeOp { gitRefUpdated = gru; mergedSenderFactory = msf; mergeFailSenderFactory = mfsf; - this.labelTypes = labelTypes; patchSetInfoFactory = psif; identifiedUserFactory = iuf; this.changeControlFactory = changeControlFactory; @@ -201,7 +200,7 @@ public class MergeOp { commits = new HashMap(); } - public void verifyMergeability(Change change) { + public void verifyMergeability(Change change) throws NoSuchProjectException { try { setDestProject(); openRepository(); @@ -269,7 +268,7 @@ public class MergeOp { } } - public void merge() throws MergeException { + public void merge() throws MergeException, NoSuchProjectException { setDestProject(); try { openSchema(); @@ -389,7 +388,8 @@ public class MergeOp { commits.putAll(strategy.getNewCommits()); } - private SubmitStrategy createStrategy(final SubmitType submitType) throws MergeException { + private SubmitStrategy createStrategy(final SubmitType submitType) + throws MergeException, NoSuchProjectException { return submitStrategyFactory.create(submitType, db, repo, rw, inserter, canMergeFlag, getAlreadyAccepted(branchTip), destBranch, destProject.isUseContentMerge()); @@ -931,12 +931,11 @@ public class MergeOp { c.setStatus(Change.Status.MERGED); final List approvals = db.patchSetApprovals().byChange(changeId).toList(); + final ChangeControl control = changeControlFactory.controlFor(c, + identifiedUserFactory.create(c.getOwner())); final FunctionState fs = functionState.create( - changeControlFactory.controlFor( - c, - identifiedUserFactory.create(c.getOwner())), - merged, approvals); - for (LabelType lt : labelTypes.getLabelTypes()) { + control, merged, approvals); + for (LabelType lt : control.getLabelTypes().getLabelTypes()) { CategoryFunction.forType(lt).run(lt, fs); } for (PatchSetApproval a : approvals) { @@ -987,7 +986,9 @@ public class MergeOp { } try { - final MergedSender cm = mergedSenderFactory.create(c); + final ChangeControl control = changeControlFactory.controlFor(c, + identifiedUserFactory.create(c.getOwner())); + final MergedSender cm = mergedSenderFactory.create(control); if (from != null) { cm.setFrom(from.getAccountId()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index cf541bb506..820e196f63 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -18,6 +18,7 @@ import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; + import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK; @@ -42,6 +43,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.common.ChangeHookRunner.HookResult; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.data.Capable; +import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; @@ -263,6 +265,7 @@ public class ReceiveCommits { private final ReceivePack rp; private final NoteMap rejectCommits; private MagicBranchInput magicBranch; + private LabelTypes labelTypes; private List newChanges = Collections.emptyList(); private final Map replaceByChange = @@ -1116,6 +1119,7 @@ public class ReceiveCommits { reject(cmd, "cannot upload review"); return; } + labelTypes = projectControl.getLabelTypes(); // Validate that the new commits are connected with the target // branch. If they aren't, we want to abort. We do this check by @@ -1463,7 +1467,7 @@ public class ReceiveCommits { db.patchSets().insert(Collections.singleton(ps)); db.changes().insert(Collections.singleton(change)); ChangeUtil.updateTrackingIds(db, change, trackingFooters, footerLines); - approvalsUtil.addReviewers(db, change, ps, info, + approvalsUtil.addReviewers(db, labelTypes, change, ps, info, recipients.getReviewers(), Collections. emptySet()); db.commit(); } finally { @@ -1769,10 +1773,10 @@ public class ReceiveCommits { } List patchSetApprovals = - approvalsUtil.copyVetosToPatchSet(db, newPatchSet.getId()); + approvalsUtil.copyVetosToPatchSet(db, labelTypes, newPatchSet.getId()); final MailRecipients oldRecipients = getRecipientsFromApprovals(patchSetApprovals); - approvalsUtil.addReviewers(db, change, newPatchSet, info, + approvalsUtil.addReviewers(db, labelTypes, change, newPatchSet, info, recipients.getReviewers(), oldRecipients.getAll()); recipients.add(oldRecipients); @@ -2161,7 +2165,7 @@ public class ReceiveCommits { @Override public void run() { try { - final MergedSender cm = mergedSenderFactory.create(result.change); + final MergedSender cm = mergedSenderFactory.create(result.changeCtl); cm.setFrom(currentUser.getAccountId()); cm.setPatchSet(result.newPatchSet, result.info); cm.send(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java index 093a7583d2..f4cf6c5308 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java @@ -24,6 +24,8 @@ import com.google.gerrit.server.changedetail.RebaseChange; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.patch.PatchSetInfoFactory; +import com.google.gerrit.server.project.NoSuchProjectException; +import com.google.gerrit.server.project.ProjectControl; import com.google.inject.Inject; import com.google.inject.Provider; @@ -49,9 +51,9 @@ public class SubmitStrategyFactory { private final PersonIdent myIdent; private final PatchSetInfoFactory patchSetInfoFactory; private final Provider urlProvider; - private final LabelTypes labelTypes; private final GitReferenceUpdated gitRefUpdated; private final RebaseChange rebaseChange; + private final ProjectControl.Factory projectFactory; @Inject SubmitStrategyFactory( @@ -59,28 +61,30 @@ public class SubmitStrategyFactory { @GerritPersonIdent final PersonIdent myIdent, final PatchSetInfoFactory patchSetInfoFactory, @CanonicalWebUrl @Nullable final Provider urlProvider, - final LabelTypes labelTypes, final GitReferenceUpdated gitRefUpdated, - final RebaseChange rebaseChange) { + final GitReferenceUpdated gitRefUpdated, final RebaseChange rebaseChange, + final ProjectControl.Factory projectFactory) { this.identifiedUserFactory = identifiedUserFactory; this.myIdent = myIdent; this.patchSetInfoFactory = patchSetInfoFactory; this.urlProvider = urlProvider; - this.labelTypes = labelTypes; this.gitRefUpdated = gitRefUpdated; this.rebaseChange = rebaseChange; + this.projectFactory = projectFactory; } public SubmitStrategy create(final SubmitType submitType, final ReviewDb db, final Repository repo, final RevWalk rw, final ObjectInserter inserter, final RevFlag canMergeFlag, final Set alreadyAccepted, final Branch.NameKey destBranch, final boolean useContentMerge) - throws MergeException { + throws MergeException, NoSuchProjectException { final SubmitStrategy.Arguments args = new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db, repo, rw, inserter, canMergeFlag, alreadyAccepted, destBranch, useContentMerge); switch (submitType) { case CHERRY_PICK: + LabelTypes labelTypes = + projectFactory.controlFor(destBranch.getParentKey()).getLabelTypes(); return new CherryPick(args, patchSetInfoFactory, urlProvider, labelTypes, gitRefUpdated); case FAST_FORWARD_ONLY: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java index 42f1dd33f2..2255584226 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java @@ -22,8 +22,8 @@ import com.google.gerrit.common.data.LabelValue; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.server.project.ChangeControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -31,15 +31,15 @@ import com.google.inject.assistedinject.Assisted; /** Send notice about a change successfully merged. */ public class MergedSender extends ReplyToChangeSender { public static interface Factory { - public MergedSender create(Change change); + public MergedSender create(ChangeControl change); } private final LabelTypes labelTypes; @Inject - public MergedSender(EmailArguments ea, LabelTypes lt, @Assisted Change c) { - super(ea, c, "merged"); - labelTypes = lt; + public MergedSender(EmailArguments ea, @Assisted ChangeControl c) { + super(ea, c.getChange(), "merged"); + labelTypes = c.getLabelTypes(); } @Override 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 6064560947..d0185dd8c0 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 @@ -14,6 +14,7 @@ package com.google.gerrit.server.project; +import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitTypeRecord; @@ -211,6 +212,11 @@ public class ChangeControl { && getRefControl().canUpload(); // as long as you can upload too } + /** All available label types for this project. */ + public LabelTypes getLabelTypes() { + return getProjectControl().getLabelTypes(); + } + /** All value ranges of any allowed label permission. */ public List getLabelRanges() { return getRefControl().getLabelRanges(); 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 8f8d3152c6..9cfdddd439 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 @@ -20,6 +20,7 @@ import com.google.gerrit.common.data.AccessSection; 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; @@ -181,6 +182,10 @@ public class ProjectControl { return state.getProject(); } + public LabelTypes getLabelTypes() { + return state.getLabelTypes(); + } + private boolean isHidden() { return getProject().getState().equals(Project.State.HIDDEN); } 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 e42be92fc4..42c69f2e7b 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 @@ -20,6 +20,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.gerrit.common.data.AccessSection; 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.reviewdb.client.AccountGroup; @@ -65,6 +66,7 @@ public class ProjectState { private final PrologEnvironment.Factory envFactory; private final GitRepositoryManager gitMgr; private final RulesCache rulesCache; + private final LabelTypes labelTypes; private final ProjectConfig config; private final Set localOwners; @@ -89,6 +91,7 @@ public class ProjectState { final PrologEnvironment.Factory envFactory, final GitRepositoryManager gitMgr, final RulesCache rulesCache, + final LabelTypes labelTypes, @Assisted final ProjectConfig config) { this.projectCache = projectCache; this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName); @@ -101,6 +104,7 @@ public class ProjectState { this.capabilities = isAllProjects ? new CapabilityCollection(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)) : null; + this.labelTypes = labelTypes; if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) { localOwners = Collections.emptySet(); @@ -337,6 +341,10 @@ public class ProjectState { }); } + public LabelTypes getLabelTypes() { + return labelTypes; + } + private boolean getInheritableBoolean(Function func) { for (ProjectState s : tree()) { switch (func.apply(s.getProject())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index e80fa331e8..7bbb073a6c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.query.change; import com.google.common.collect.Lists; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; @@ -110,7 +109,6 @@ public class ChangeQueryBuilder extends QueryBuilder { final ChangeControl.GenericFactory changeControlGenericFactory; final AccountResolver accountResolver; final GroupBackend groupBackend; - final LabelTypes labelTypes; final AllProjectsName allProjectsName; final PatchListCache patchListCache; final GitRepositoryManager repoManager; @@ -124,7 +122,6 @@ public class ChangeQueryBuilder extends QueryBuilder { ChangeControl.GenericFactory changeControlGenericFactory, AccountResolver accountResolver, GroupBackend groupBackend, - LabelTypes labelTypes, AllProjectsName allProjectsName, PatchListCache patchListCache, GitRepositoryManager repoManager, @@ -136,7 +133,6 @@ public class ChangeQueryBuilder extends QueryBuilder { this.changeControlGenericFactory = changeControlGenericFactory; this.accountResolver = accountResolver; this.groupBackend = groupBackend; - this.labelTypes = labelTypes; this.allProjectsName = allProjectsName; this.patchListCache = patchListCache; this.repoManager = repoManager; @@ -301,8 +297,9 @@ public class ChangeQueryBuilder extends QueryBuilder { @Operator public Predicate label(String name) { - return new LabelPredicate(args.changeControlGenericFactory, - args.userFactory, args.dbProvider, args.labelTypes, name); + return new LabelPredicate(args.projectCache, + args.changeControlGenericFactory, args.userFactory, args.dbProvider, + name); } @Operator diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java index 761aba231a..e6251bc1a6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java @@ -39,7 +39,7 @@ public class ChangeQueryRewriter extends QueryRewriter { new ChangeQueryBuilder.Arguments( // new InvalidProvider(), // new InvalidProvider(), // - null, null, null, null, null, null, // + null, null, null, null, null, // null, null, null, null), null)); private final Provider dbProvider; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java index 4fc48a5d6f..e1bcb559b8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java @@ -24,6 +24,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.IdentifiedUser; 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.query.OperatorPredicate; import com.google.gwtorm.server.OrmException; import com.google.inject.Provider; @@ -103,52 +105,66 @@ class LabelPredicate extends OperatorPredicate { return Integer.parseInt(value); } + private final ProjectCache projectCache; private final ChangeControl.GenericFactory ccFactory; private final IdentifiedUser.GenericFactory userFactory; private final Provider dbProvider; private final Test test; - private final LabelType type; - private final String permissionName; + private final String type; private final int expVal; - LabelPredicate(ChangeControl.GenericFactory ccFactory, - IdentifiedUser.GenericFactory userFactory, Provider dbProvider, - LabelTypes types, String value) { + LabelPredicate(ProjectCache projectCache, + ChangeControl.GenericFactory ccFactory, + IdentifiedUser.GenericFactory userFactory, + Provider dbProvider, + String value) { super(ChangeQueryBuilder.FIELD_LABEL, value); this.ccFactory = ccFactory; + this.projectCache = projectCache; this.userFactory = userFactory; this.dbProvider = dbProvider; Matcher m1 = Pattern.compile("(=|>=|<=)([+-]?\\d+)$").matcher(value); Matcher m2 = Pattern.compile("([+-]\\d+)$").matcher(value); if (m1.find()) { - type = type(types, value.substring(0, m1.start())); + type = value.substring(0, m1.start()); test = op(m1.group(1)); expVal = value(m1.group(2)); } else if (m2.find()) { - type = type(types, value.substring(0, m2.start())); + type = value.substring(0, m2.start()); test = Test.EQ; expVal = value(m2.group(1)); } else { - type = type(types, value); + type = value; test = Test.EQ; expVal = 1; } - - this.permissionName = Permission.forLabel(type.getName()); } @Override public boolean match(final ChangeData object) throws OrmException { + final Change c = object.change(dbProvider); + if (c == null) { + // The change has disappeared. + // + return false; + } + final ProjectState project = projectCache.get(c.getDest().getParentKey()); + if (project == null) { + // The project has disappeared. + // + return false; + } + final LabelType labelType = type(project.getLabelTypes(), type); final Set allApprovers = new HashSet(); final Set approversThatVotedInCategory = new HashSet(); for (PatchSetApproval p : object.currentApprovals(dbProvider)) { allApprovers.add(p.getAccountId()); - if (p.getCategoryId().get().equals(type.getId())) { + if (p.getCategoryId().get().equals(labelType.getId())) { approversThatVotedInCategory.add(p.getAccountId()); - if (match(object.change(dbProvider), p.getValue(), p.getAccountId())) { + if (match(c, p.getValue(), p.getAccountId(), labelType)) { return true; } } @@ -156,8 +172,8 @@ class LabelPredicate extends OperatorPredicate { final Set approversThatDidNotVoteInCategory = new HashSet(allApprovers); approversThatDidNotVoteInCategory.removeAll(approversThatVotedInCategory); - for (final Account.Id a : approversThatDidNotVoteInCategory) { - if (match(object.change(dbProvider), 0, a)) { + for (Account.Id a : approversThatDidNotVoteInCategory) { + if (match(c, 0, a, labelType)) { return true; } } @@ -166,7 +182,8 @@ class LabelPredicate extends OperatorPredicate { } private boolean match(final Change change, final int value, - final Account.Id approver) throws OrmException { + final Account.Id approver, final LabelType type) + throws OrmException { int psVal = value; if (test.match(psVal, expVal)) { // Double check the value is still permitted for the user. @@ -179,7 +196,7 @@ class LabelPredicate extends OperatorPredicate { // return false; } - psVal = cc.getRange(permissionName).squash(psVal); + psVal = cc.getRange(Permission.forLabel(type.getName())).squash(psVal); } catch (NoSuchChangeException e) { // The project has disappeared. // diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java index 8ca0548b14..c781d97913 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.common.data.GlobalCapability; +import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; @@ -25,6 +26,8 @@ import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.events.PatchSetAttribute; import com.google.gerrit.server.events.QueryStats; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gson.Gson; @@ -93,6 +96,7 @@ public class QueryProcessor { private final ChangeQueryRewriter queryRewriter; private final Provider db; private final GitRepositoryManager repoManager; + private final ChangeControl.Factory changeControlFactory; private final int maxLimit; private OutputFormat outputFormat = OutputFormat.TEXT; @@ -116,12 +120,14 @@ public class QueryProcessor { QueryProcessor(EventFactory eventFactory, ChangeQueryBuilder.Factory queryBuilder, CurrentUser currentUser, ChangeQueryRewriter queryRewriter, Provider db, - GitRepositoryManager repoManager) { + GitRepositoryManager repoManager, + ChangeControl.Factory changeControlFactory) { this.eventFactory = eventFactory; this.queryBuilder = queryBuilder.create(currentUser); this.queryRewriter = queryRewriter; this.db = db; this.repoManager = repoManager; + this.changeControlFactory = changeControlFactory; this.maxLimit = currentUser.getCapabilities() .getRange(GlobalCapability.QUERY_LIMIT) .getMax(); @@ -267,6 +273,8 @@ public class QueryProcessor { List results = queryChanges(queryString); ChangeAttribute c = null; for (ChangeData d : results) { + LabelTypes labelTypes = changeControlFactory.controlFor(d.getChange()) + .getLabelTypes(); c = eventFactory.asChangeAttribute(d.getChange()); eventFactory.extend(c, d.getChange()); eventFactory.addTrackingIds(c, d.trackingIds(db)); @@ -287,10 +295,11 @@ public class QueryProcessor { if (includeFiles) { eventFactory.addPatchSets(c, d.patches(db), includeApprovals ? d.approvalsMap(db).asMap() : null, - includeFiles, d.change(db)); + includeFiles, d.change(db), labelTypes); } else { eventFactory.addPatchSets(c, d.patches(db), - includeApprovals ? d.approvalsMap(db).asMap() : null); + includeApprovals ? d.approvalsMap(db).asMap() : null, + labelTypes); } } @@ -298,8 +307,8 @@ public class QueryProcessor { PatchSet current = d.currentPatchSet(db); if (current != null) { c.currentPatchSet = eventFactory.asPatchSetAttribute(current); - eventFactory.addApprovals(c.currentPatchSet, // - d.currentApprovals(db)); + eventFactory.addApprovals(c.currentPatchSet, + d.currentApprovals(db), labelTypes); if (includeFiles) { eventFactory.addPatchSetFileNames(c.currentPatchSet, @@ -342,6 +351,11 @@ public class QueryProcessor { ErrorMessage m = new ErrorMessage(); m.message = e.getMessage(); show(m); + } catch (NoSuchChangeException e) { + log.error("Missing change: " + e.getMessage(), e); + ErrorMessage m = new ErrorMessage(); + m.message = "missing change " + e.getMessage(); + show(m); } } finally { try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java index a7a542230f..6b9cb93099 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.workflow; import com.google.gerrit.common.data.LabelType; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.LabelValue; import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRange; @@ -42,7 +41,6 @@ public class FunctionState { Collection all); } - private final LabelTypes labelTypes; private final IdentifiedUser.GenericFactory userFactory; private final Map> approvals = @@ -52,11 +50,9 @@ public class FunctionState { private final Change change; @Inject - FunctionState(final LabelTypes labelTypes, - final IdentifiedUser.GenericFactory userFactory, + FunctionState(final IdentifiedUser.GenericFactory userFactory, @Assisted final ChangeControl c, @Assisted final PatchSet.Id psId, @Assisted final Collection all) { - this.labelTypes = labelTypes; this.userFactory = userFactory; callerChangeControl = c; @@ -68,7 +64,7 @@ public class FunctionState { approvals.get(ca.getCategoryId().get()); if (l == null) { l = new ArrayList(); - LabelType lt = labelTypes.byId(ca.getCategoryId().get()); + LabelType lt = c.getLabelTypes().byId(ca.getCategoryId().get()); if (lt != null) { // TODO: Support arbitrary labels approvals.put(lt.getName(), l); @@ -80,7 +76,7 @@ public class FunctionState { } List getLabelTypes() { - return labelTypes.getLabelTypes(); + return callerChangeControl.getLabelTypes().getLabelTypes(); } Change getChange() { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 51ea5a2725..f818746c15 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -541,10 +541,10 @@ public class RefControlTest extends TestCase { RulesCache rulesCache = null; all.put(local.getProject().getNameKey(), new ProjectState( projectCache, allProjectsName, projectControlFactory, - envFactory, mgr, rulesCache, local)); + envFactory, mgr, rulesCache, null, local)); all.put(parent.getProject().getNameKey(), new ProjectState( projectCache, allProjectsName, projectControlFactory, - envFactory, mgr, rulesCache, parent)); + envFactory, mgr, rulesCache, null, parent)); return all.get(local.getProject().getNameKey()); }