Access LabelTypes through ProjectState rather than globally

We want to allow projects to define their own labels, so we can't
assume the label list is global. In typical cases, we can access it
through a ProjectControl, ChangeControl, or related factory. This does
result in a few more places where we propagate
NoSuchProject/ChangeExceptions where there were none before. This is
intended: operations that query/modify labels on, say, a bare
Change.Id now do need to verify that the project/change exists.

For now, leave code in LabelTypesProvider, but try not to inject
LabelTypes where at all possible.

Change-Id: I4936ccafdb41848aaac3e335adf4648369d6abbc
This commit is contained in:
Dave Borowitz
2013-02-14 17:11:25 -08:00
parent 742a046fd0
commit 6cae753a4c
32 changed files with 212 additions and 156 deletions

View File

@@ -39,6 +39,7 @@ import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; 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.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.ResultSet;
@@ -110,7 +111,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
@Override @Override
public ChangeDetail call() throws OrmException, NoSuchEntityException, public ChangeDetail call() throws OrmException, NoSuchEntityException,
PatchSetInfoNotAvailableException, NoSuchChangeException, PatchSetInfoNotAvailableException, NoSuchChangeException,
RepositoryNotFoundException, IOException { RepositoryNotFoundException, IOException, NoSuchProjectException {
control = changeControlFactory.validateFor(changeId); control = changeControlFactory.validateFor(changeId);
final Change change = control.getChange(); final Change change = control.getChange();
final PatchSet patch = db.patchSets().get(change.currentPatchSetId()); final PatchSet patch = db.patchSets().get(change.currentPatchSetId());
@@ -206,7 +207,8 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
} }
} }
private void load() throws OrmException, NoSuchChangeException { private void load() throws OrmException, NoSuchChangeException,
NoSuchProjectException {
final Change.Status status = detail.getChange().getStatus(); final Change.Status status = detail.getChange().getStatus();
if ((status.equals(Change.Status.NEW) || status.equals(Change.Status.DRAFT)) && if ((status.equals(Change.Status.NEW) || status.equals(Change.Status.DRAFT)) &&
testMerge) { testMerge) {

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -114,7 +115,7 @@ class EditCommitMessageHandler extends Handler<ChangeDetail> {
public ChangeDetail call() throws NoSuchChangeException, OrmException, public ChangeDetail call() throws NoSuchChangeException, OrmException,
EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException, EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException,
MissingObjectException, IncorrectObjectTypeException, IOException, MissingObjectException, IncorrectObjectTypeException, IOException,
InvalidChangeOperationException { InvalidChangeOperationException, NoSuchProjectException {
final Change.Id changeId = patchSetId.getParentKey(); final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl control = changeControlFactory.validateFor(changeId); final ChangeControl control = changeControlFactory.validateFor(changeId);

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.httpd.rpc.changedetail; package com.google.gerrit.httpd.rpc.changedetail;
import com.google.gerrit.common.data.LabelType; 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.PatchSetPublishDetail;
import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
@@ -47,7 +46,6 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
private final PatchSetInfoFactory infoFactory; private final PatchSetInfoFactory infoFactory;
private final ReviewDb db; private final ReviewDb db;
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final LabelTypes labelTypes;
private final AccountInfoCacheFactory aic; private final AccountInfoCacheFactory aic;
private final IdentifiedUser user; private final IdentifiedUser user;
@@ -62,12 +60,10 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
final ReviewDb db, final ReviewDb db,
final AccountInfoCacheFactory.Factory accountInfoCacheFactory, final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
final ChangeControl.Factory changeControlFactory, final ChangeControl.Factory changeControlFactory,
final LabelTypes labelTypes,
final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) { final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
this.infoFactory = infoFactory; this.infoFactory = infoFactory;
this.db = db; this.db = db;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.labelTypes = labelTypes;
this.aic = accountInfoCacheFactory.create(); this.aic = accountInfoCacheFactory.create();
this.user = user; this.user = user;
@@ -124,7 +120,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
boolean canMakeOk = false; boolean canMakeOk = false;
PermissionRange range = rangeByName.get(lbl.label); PermissionRange range = rangeByName.get(lbl.label);
if (range != null) { if (range != null) {
LabelType lt = labelTypes.byLabel(lbl.label); LabelType lt = control.getLabelTypes().byLabel(lbl.label);
if (lt == null || lt.getMax().getValue() == range.getMax()) { if (lt == null || lt.getMax().getValue() == range.getMax()) {
canMakeOk = true; canMakeOk = true;
} }

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -53,7 +54,8 @@ class PublishAction extends Handler<ChangeDetail> {
@Override @Override
public ChangeDetail call() throws OrmException, NoSuchEntityException, public ChangeDetail call() throws OrmException, NoSuchEntityException,
IllegalStateException, PatchSetInfoNotAvailableException, IllegalStateException, PatchSetInfoNotAvailableException,
NoSuchChangeException, RepositoryNotFoundException, IOException { NoSuchChangeException, RepositoryNotFoundException, IOException,
NoSuchProjectException {
final ReviewResult result = publishFactory.create(patchSetId).call(); final ReviewResult result = publishFactory.create(patchSetId).call();
if (result.getErrors().size() > 0) { if (result.getErrors().size() > 0) {
throw new IllegalStateException("Cannot publish patchset"); throw new IllegalStateException("Cannot publish patchset");

View File

@@ -24,6 +24,7 @@ import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -60,7 +61,7 @@ class RebaseChangeHandler extends Handler<ChangeDetail> {
public ChangeDetail call() throws NoSuchChangeException, OrmException, public ChangeDetail call() throws NoSuchChangeException, OrmException,
EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException, EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException,
MissingObjectException, IncorrectObjectTypeException, IOException, MissingObjectException, IncorrectObjectTypeException, IOException,
InvalidChangeOperationException { InvalidChangeOperationException, NoSuchProjectException {
rebaseChange.rebase(patchSetId, currentUser.getAccountId()); rebaseChange.rebase(patchSetId, currentUser.getAccountId());
return changeDetailFactory.create(patchSetId.getParentKey()).call(); return changeDetailFactory.create(patchSetId.getParentKey()).call();
} }

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.patch;
import com.google.gerrit.common.data.ApprovalSummary; import com.google.gerrit.common.data.ApprovalSummary;
import com.google.gerrit.common.data.ApprovalSummarySet; 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.ChangeDetail;
import com.google.gerrit.common.data.PatchDetailService; import com.google.gerrit.common.data.PatchDetailService;
import com.google.gerrit.common.data.PatchScript; 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.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.workflow.FunctionState; import com.google.gerrit.server.workflow.FunctionState;
import com.google.gwtjsonrpc.common.AsyncCallback; import com.google.gwtjsonrpc.common.AsyncCallback;
import com.google.gwtjsonrpc.common.VoidResult; import com.google.gwtjsonrpc.common.VoidResult;
@@ -56,8 +56,6 @@ import java.util.Set;
class PatchDetailServiceImpl extends BaseServiceImplementation implements class PatchDetailServiceImpl extends BaseServiceImplementation implements
PatchDetailService { PatchDetailService {
private final LabelTypes labelTypes;
private final AccountInfoCacheFactory.Factory accountInfoCacheFactory; private final AccountInfoCacheFactory.Factory accountInfoCacheFactory;
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
@@ -69,7 +67,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
@Inject @Inject
PatchDetailServiceImpl(final Provider<ReviewDb> schema, PatchDetailServiceImpl(final Provider<ReviewDb> schema,
final Provider<CurrentUser> currentUser, final Provider<CurrentUser> currentUser,
final LabelTypes labelTypes,
final AccountInfoCacheFactory.Factory accountInfoCacheFactory, final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
final ChangeControl.Factory changeControlFactory, final ChangeControl.Factory changeControlFactory,
final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory, final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory,
@@ -78,7 +75,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
final SaveDraft.Factory saveDraftFactory, final SaveDraft.Factory saveDraftFactory,
final ChangeDetailFactory.Factory changeDetailFactory) { final ChangeDetailFactory.Factory changeDetailFactory) {
super(schema, currentUser); super(schema, currentUser);
this.labelTypes = labelTypes;
this.accountInfoCacheFactory = accountInfoCacheFactory; this.accountInfoCacheFactory = accountInfoCacheFactory;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
@@ -149,6 +145,8 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
return changeDetailFactory.create(result.getChangeId()).call(); return changeDetailFactory.create(result.getChangeId()).call();
} catch (NoSuchChangeException e) { } catch (NoSuchChangeException e) {
throw new Failure(new NoSuchChangeException(result.getChangeId())); throw new Failure(new NoSuchChangeException(result.getChangeId()));
} catch (NoSuchProjectException e) {
throw new Failure(e);
} catch (NoSuchEntityException e) { } catch (NoSuchEntityException e) {
throw new Failure(e); throw new Failure(e);
} catch (PatchSetInfoNotAvailableException e) { } catch (PatchSetInfoNotAvailableException e) {
@@ -189,7 +187,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
continue; continue;
} }
if (change.getStatus().isOpen()) { if (change.getStatus().isOpen()) {
fs.normalize(labelTypes.byId(category.get()), ca); fs.normalize(cc.getLabelTypes().byId(category.get()), ca);
} }
if (ca.getValue() == 0) { if (ca.getValue() == 0) {
continue; continue;
@@ -235,7 +233,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
continue; continue;
} }
if (change.getStatus().isOpen()) { if (change.getStatus().isOpen()) {
fs.normalize(labelTypes.byId(category.get()), ca); fs.normalize(cc.getLabelTypes().byId(category.get()), ca);
} }
if (ca.getValue() == 0) { if (ca.getValue() == 0) {
continue; continue;

View File

@@ -206,8 +206,6 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
private final AccountCache accountCache; private final AccountCache accountCache;
private final LabelTypes labelTypes;
private final EventFactory eventFactory; private final EventFactory eventFactory;
private final SitePaths sitePaths; private final SitePaths sitePaths;
@@ -232,16 +230,17 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
final GitRepositoryManager repoManager, final GitRepositoryManager repoManager,
final @GerritServerConfig Config config, final @GerritServerConfig Config config,
final @AnonymousCowardName String anonymousCowardName, final @AnonymousCowardName String anonymousCowardName,
final SitePaths sitePath, final ProjectCache projectCache, final SitePaths sitePath,
final AccountCache accountCache, final LabelTypes labelTypes, final ProjectCache projectCache,
final EventFactory eventFactory, final SitePaths sitePaths, final AccountCache accountCache,
final EventFactory eventFactory,
final SitePaths sitePaths,
final DynamicSet<ChangeListener> unrestrictedListeners) { final DynamicSet<ChangeListener> unrestrictedListeners) {
this.anonymousCowardName = anonymousCowardName; this.anonymousCowardName = anonymousCowardName;
this.repoManager = repoManager; this.repoManager = repoManager;
this.hookQueue = queue.createQueue(1, "hook"); this.hookQueue = queue.createQueue(1, "hook");
this.projectCache = projectCache; this.projectCache = projectCache;
this.accountCache = accountCache; this.accountCache = accountCache;
this.labelTypes = labelTypes;
this.eventFactory = eventFactory; this.eventFactory = eventFactory;
this.sitePaths = sitePath; this.sitePaths = sitePath;
this.unrestrictedListeners = unrestrictedListeners; this.unrestrictedListeners = unrestrictedListeners;
@@ -395,11 +394,12 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
event.patchSet = eventFactory.asPatchSetAttribute(patchSet); event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
event.comment = comment; event.comment = comment;
LabelTypes labelTypes = projectCache.get(change.getProject()).getLabelTypes();
if (approvals.size() > 0) { if (approvals.size() > 0) {
event.approvals = new ApprovalAttribute[approvals.size()]; event.approvals = new ApprovalAttribute[approvals.size()];
int i = 0; int i = 0;
for (Map.Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval : approvals.entrySet()) { for (Map.Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> 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 * @param approval
* @return object suitable for serialization to JSON * @return object suitable for serialization to JSON
*/ */
private ApprovalAttribute getApprovalAttribute( private ApprovalAttribute getApprovalAttribute(LabelTypes labelTypes,
Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval) { Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval) {
ApprovalAttribute a = new ApprovalAttribute(); ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getKey().get(); a.type = approval.getKey().get();

View File

@@ -46,20 +46,17 @@ import java.util.Set;
*/ */
public class ApprovalsUtil { public class ApprovalsUtil {
private final ReviewDb db; private final ReviewDb db;
private final LabelTypes labelTypes;
@Inject @Inject
public ApprovalsUtil(ReviewDb db, LabelTypes labelTypes) { public ApprovalsUtil(ReviewDb db) {
this.db = db; this.db = db;
this.labelTypes = labelTypes;
} }
/** /**
* Resync the changeOpen status which is cached in the approvals table for * Resync the changeOpen status which is cached in the approvals table for
* performance reasons * performance reasons
*/ */
public void syncChangeStatus(final Change change) public void syncChangeStatus(final Change change) throws OrmException {
throws OrmException {
final List<PatchSetApproval> approvals = final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(change.getId()).toList(); db.patchSetApprovals().byChange(change.getId()).toList();
for (PatchSetApproval a : approvals) { for (PatchSetApproval a : approvals) {
@@ -78,7 +75,7 @@ public class ApprovalsUtil {
* @return List<PatchSetApproval> The previous approvals * @return List<PatchSetApproval> The previous approvals
*/ */
public List<PatchSetApproval> copyVetosToPatchSet(ReviewDb db, public List<PatchSetApproval> copyVetosToPatchSet(ReviewDb db,
PatchSet.Id dest) throws OrmException { LabelTypes labelTypes, PatchSet.Id dest) throws OrmException {
PatchSet.Id source; PatchSet.Id source;
if (dest.get() > 1) { if (dest.get() > 1) {
source = new PatchSet.Id(dest.getParentKey(), dest.get() - 1); source = new PatchSet.Id(dest.getParentKey(), dest.get() - 1);
@@ -103,8 +100,8 @@ public class ApprovalsUtil {
return patchSetApprovals; return patchSetApprovals;
} }
public void addReviewers(ReviewDb db, Change change, PatchSet ps, public void addReviewers(ReviewDb db, LabelTypes labelTypes, Change change,
PatchSetInfo info, Set<Id> wantReviewers, PatchSet ps, PatchSetInfo info, Set<Id> wantReviewers,
Set<Account.Id> existingReviewers) throws OrmException { Set<Account.Id> existingReviewers) throws OrmException {
List<LabelType> allTypes = labelTypes.getLabelTypes(); List<LabelType> allTypes = labelTypes.getLabelTypes();
if (allTypes.isEmpty()) { if (allTypes.isEmpty()) {

View File

@@ -40,6 +40,7 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; 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.project.RefControl;
import com.google.gerrit.server.util.IdGenerator; import com.google.gerrit.server.util.IdGenerator;
import com.google.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.AtomicUpdate;
@@ -176,7 +177,8 @@ public class ChangeUtil {
db.trackingIds().delete(toDelete); 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); opFactory.create(change.getDest()).verifyMergeability(change);
} }
@@ -443,7 +445,9 @@ public class ChangeUtil {
"Change %s was modified", change.getId())); "Change %s was modified", change.getId()));
} }
approvalsUtil.copyVetosToPatchSet(db, change.currentPatchSetId()); approvalsUtil.copyVetosToPatchSet(db,
refControl.getProjectControl().getLabelTypes(),
change.currentPatchSetId());
final List<FooterLine> footerLines = newCommit.getFooterLines(); final List<FooterLine> footerLines = newCommit.getFooterLines();
updateTrackingIds(db, change, trackingFooters, footerLines); updateTrackingIds(db, change, trackingFooters, footerLines);

View File

@@ -101,7 +101,7 @@ public class Abandon implements RestModifyView<ChangeResource, Input> {
} }
message = newMessage(input, caller, change); message = newMessage(input, caller, change);
db.changeMessages().insert(Collections.singleton(message)); db.changeMessages().insert(Collections.singleton(message));
new ApprovalsUtil(db, null).syncChangeStatus(change); new ApprovalsUtil(db).syncChangeStatus(change);
db.commit(); db.commit();
} finally { } finally {
db.rollback(); db.rollback();

View File

@@ -114,7 +114,6 @@ public class ChangeJson {
} }
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final LabelTypes labelTypes;
private final CurrentUser user; private final CurrentUser user;
private final AnonymousUser anonymous; private final AnonymousUser anonymous;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
@@ -133,7 +132,6 @@ public class ChangeJson {
@Inject @Inject
ChangeJson( ChangeJson(
Provider<ReviewDb> db, Provider<ReviewDb> db,
LabelTypes at,
CurrentUser u, CurrentUser u,
AnonymousUser au, AnonymousUser au,
IdentifiedUser.GenericFactory uf, IdentifiedUser.GenericFactory uf,
@@ -144,7 +142,6 @@ public class ChangeJson {
@CanonicalWebUrl Provider<String> curl, @CanonicalWebUrl Provider<String> curl,
Urls urls) { Urls urls) {
this.db = db; this.db = db;
this.labelTypes = at;
this.user = u; this.user = u;
this.anonymous = au; this.anonymous = au;
this.userFactory = uf; this.userFactory = uf;
@@ -321,16 +318,18 @@ public class ChangeJson {
return null; return null;
} }
LabelTypes labelTypes = ctl.getLabelTypes();
if (cd.getChange().getStatus().isOpen()) { if (cd.getChange().getStatus().isOpen()) {
return labelsForOpenChange(cd, standard, detailed); return labelsForOpenChange(cd, labelTypes, standard, detailed);
} else { } else {
return labelsForClosedChange(cd, standard, detailed); return labelsForClosedChange(cd, labelTypes, standard, detailed);
} }
} }
private Map<String, LabelInfo> labelsForOpenChange(ChangeData cd, private Map<String, LabelInfo> labelsForOpenChange(ChangeData cd,
boolean standard, boolean detailed) throws OrmException { LabelTypes labelTypes, boolean standard, boolean detailed)
Map<String, LabelInfo> labels = initLabels(cd, standard); throws OrmException {
Map<String, LabelInfo> labels = initLabels(cd, labelTypes, standard);
if (detailed) { if (detailed) {
setAllApprovals(cd, labels); setAllApprovals(cd, labels);
} }
@@ -349,8 +348,8 @@ public class ChangeJson {
return labels; return labels;
} }
private Map<String, LabelInfo> initLabels(ChangeData cd, boolean standard) private Map<String, LabelInfo> initLabels(ChangeData cd,
throws OrmException { LabelTypes labelTypes, boolean standard) throws OrmException {
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167. // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels = Map<String, LabelInfo> labels =
new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes)); new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes));
@@ -448,7 +447,7 @@ public class ChangeJson {
} }
for (PatchSetApproval psa : current.get(accountId)) { for (PatchSetApproval psa : current.get(accountId)) {
// TODO Support arbitrary labels placed by a reviewer. // 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) { if (lt == null) {
continue; continue;
} }
@@ -465,7 +464,8 @@ public class ChangeJson {
} }
private Map<String, LabelInfo> labelsForClosedChange(ChangeData cd, private Map<String, LabelInfo> labelsForClosedChange(ChangeData cd,
boolean standard, boolean detailed) throws OrmException { LabelTypes labelTypes, boolean standard, boolean detailed)
throws OrmException {
Set<Account.Id> allUsers = Sets.newHashSet(); Set<Account.Id> allUsers = Sets.newHashSet();
for (PatchSetApproval psa : cd.allApprovals(db)) { for (PatchSetApproval psa : cd.allApprovals(db)) {
allUsers.add(psa.getAccountId()); allUsers.add(psa.getAccountId());
@@ -570,6 +570,7 @@ public class ChangeJson {
return null; return null;
} }
LabelTypes labelTypes = ctl.getLabelTypes();
ListMultimap<String, String> permitted = LinkedListMultimap.create(); ListMultimap<String, String> permitted = LinkedListMultimap.create();
for (SubmitRecord rec : submitRecords(cd)) { for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels == null) { if (rec.labels == null) {

View File

@@ -102,7 +102,6 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
} }
private final ReviewDb db; private final ReviewDb db;
private final LabelTypes labelTypes;
private final EmailReviewComments.Factory email; private final EmailReviewComments.Factory email;
@Deprecated private final ChangeHooks hooks; @Deprecated private final ChangeHooks hooks;
@@ -116,11 +115,9 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
@Inject @Inject
PostReview(ReviewDb db, PostReview(ReviewDb db,
LabelTypes labelTypes,
EmailReviewComments.Factory email, EmailReviewComments.Factory email,
ChangeHooks hooks) { ChangeHooks hooks) {
this.db = db; this.db = db;
this.labelTypes = labelTypes;
this.email = email; this.email = email;
this.hooks = hooks; this.hooks = hooks;
} }
@@ -180,7 +177,8 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
Map.Entry<String, Short> ent = itr.next(); Map.Entry<String, Short> ent = itr.next();
// TODO Support more generic label assignments. // TODO Support more generic label assignments.
LabelType lt = labelTypes.byLabel(ent.getKey()); LabelType lt = revision.getControl().getLabelTypes()
.byLabel(ent.getKey());
if (lt == null) { if (lt == null) {
if (strict) { if (strict) {
throw new BadRequestException(String.format( throw new BadRequestException(String.format(
@@ -343,6 +341,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
List<PatchSetApproval> upd = Lists.newArrayList(); List<PatchSetApproval> upd = Lists.newArrayList();
Map<String, PatchSetApproval> current = scanLabels(rsrc, del); Map<String, PatchSetApproval> current = scanLabels(rsrc, del);
LabelTypes labelTypes = rsrc.getControl().getLabelTypes();
for (Map.Entry<String, Short> ent : labels.entrySet()) { for (Map.Entry<String, Short> ent : labels.entrySet()) {
// TODO Support arbitrary label names. // TODO Support arbitrary label names.
LabelType lt = labelTypes.byLabel(ent.getKey()); LabelType lt = labelTypes.byLabel(ent.getKey());
@@ -406,7 +405,8 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
PatchSetApproval c = new PatchSetApproval(new PatchSetApproval.Key( PatchSetApproval c = new PatchSetApproval(new PatchSetApproval.Key(
rsrc.getPatchSet().getId(), rsrc.getPatchSet().getId(),
rsrc.getAccountId(), rsrc.getAccountId(),
labelTypes.getLabelTypes().get(0).getApprovalCategoryId()), rsrc.getControl().getLabelTypes().getLabelTypes().get(0)
.getApprovalCategoryId()),
(short) 0); (short) 0);
c.setGranted(timestamp); c.setGranted(timestamp);
c.cache(change); c.cache(change);
@@ -426,6 +426,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
private Map<String, PatchSetApproval> scanLabels(RevisionResource rsrc, private Map<String, PatchSetApproval> scanLabels(RevisionResource rsrc,
List<PatchSetApproval> del) throws OrmException { List<PatchSetApproval> del) throws OrmException {
LabelTypes labelTypes = rsrc.getControl().getLabelTypes();
Map<String, PatchSetApproval> current = Maps.newHashMap(); Map<String, PatchSetApproval> current = Maps.newHashMap();
for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser( for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser(
rsrc.getPatchSet().getId(), rsrc.getAccountId())) { rsrc.getPatchSet().getId(), rsrc.getAccountId())) {

View File

@@ -22,7 +22,6 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.GroupDescription; 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.EmailException;
import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
@@ -84,7 +83,6 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final IdentifiedUser currentUser; private final IdentifiedUser currentUser;
private final IdentifiedUser.GenericFactory identifiedUserFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ApprovalCategory.Id addReviewerCategoryId;
private final Config cfg; private final Config cfg;
private final ChangeHooks hooks; private final ChangeHooks hooks;
private final AccountCache accountCache; private final AccountCache accountCache;
@@ -100,7 +98,6 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
Provider<ReviewDb> db, Provider<ReviewDb> db,
IdentifiedUser currentUser, IdentifiedUser currentUser,
IdentifiedUser.GenericFactory identifiedUserFactory, IdentifiedUser.GenericFactory identifiedUserFactory,
LabelTypes labelTypes,
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
ChangeHooks hooks, ChangeHooks hooks,
AccountCache accountCache, AccountCache accountCache,
@@ -118,9 +115,6 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
this.hooks = hooks; this.hooks = hooks;
this.accountCache = accountCache; this.accountCache = accountCache;
this.json = json; this.json = json;
this.addReviewerCategoryId = Iterables.getLast(labelTypes.getLabelTypes())
.getApprovalCategoryId();
} }
@Override @Override
@@ -234,7 +228,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
continue; continue;
} }
ChangeControl control = rsrc.getControl().forUser(user); ChangeControl control = rsrc.getControl().forUser(user);
PatchSetApproval psa = dummyApproval(control.getChange(), psid, id); PatchSetApproval psa = dummyApproval(control, psid, id);
result.reviewers.add(json.format( result.reviewers.add(json.format(
new ReviewerInfo(id), control, ImmutableList.of(psa))); new ReviewerInfo(id), control, ImmutableList.of(psa)));
toInsert.add(psa); toInsert.add(psa);
@@ -283,12 +277,13 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
|| AccountGroup.REGISTERED_USERS.equals(groupUUID)); || AccountGroup.REGISTERED_USERS.equals(groupUUID));
} }
private PatchSetApproval dummyApproval(Change change, PatchSet.Id patchSetId, private PatchSetApproval dummyApproval(ChangeControl ctl,
Account.Id reviewerId) { PatchSet.Id patchSetId, Account.Id reviewerId) {
PatchSetApproval dummyApproval = ApprovalCategory.Id id = new ApprovalCategory.Id(
new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId, Iterables.getLast(ctl.getLabelTypes().getLabelTypes()).getId());
addReviewerCategoryId), (short) 0); PatchSetApproval dummyApproval = new PatchSetApproval(
dummyApproval.cache(change); new PatchSetApproval.Key(patchSetId, reviewerId, id), (short) 0);
dummyApproval.cache(ctl.getChange());
return dummyApproval; return dummyApproval;
} }
} }

View File

@@ -100,7 +100,7 @@ public class Restore implements RestModifyView<ChangeResource, Input> {
} }
message = newMessage(input, caller, change); message = newMessage(input, caller, change);
db.changeMessages().insert(Collections.singleton(message)); db.changeMessages().insert(Collections.singleton(message));
new ApprovalsUtil(db, null).syncChangeStatus(change); new ApprovalsUtil(db).syncChangeStatus(change);
db.commit(); db.commit();
} finally { } finally {
db.rollback(); db.rollback();

View File

@@ -43,17 +43,14 @@ import java.util.TreeMap;
public class ReviewerJson { public class ReviewerJson {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final LabelTypes labelTypes;
private final FunctionState.Factory functionState; private final FunctionState.Factory functionState;
private final AccountInfo.Loader.Factory accountLoaderFactory; private final AccountInfo.Loader.Factory accountLoaderFactory;
@Inject @Inject
ReviewerJson(Provider<ReviewDb> db, ReviewerJson(Provider<ReviewDb> db,
LabelTypes labelTypes,
FunctionState.Factory functionState, FunctionState.Factory functionState,
AccountInfo.Loader.Factory accountLoaderFactory) { AccountInfo.Loader.Factory accountLoaderFactory) {
this.db = db; this.db = db;
this.labelTypes = labelTypes;
this.functionState = functionState; this.functionState = functionState;
this.accountLoaderFactory = accountLoaderFactory; this.accountLoaderFactory = accountLoaderFactory;
} }
@@ -84,6 +81,7 @@ public class ReviewerJson {
.byPatchSetUser(psId, out._id)); .byPatchSetUser(psId, out._id));
} }
LabelTypes labelTypes = ctl.getLabelTypes();
FunctionState fs = functionState.create(ctl, psId, approvals); FunctionState fs = functionState.create(ctl, psId, approvals);
for (LabelType at : labelTypes.getLabelTypes()) { for (LabelType at : labelTypes.getLabelTypes()) {
CategoryFunction.forType(at).run(at, fs); CategoryFunction.forType(at).run(at, fs);

View File

@@ -19,6 +19,7 @@ import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.ReviewResult; import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
@@ -108,6 +109,7 @@ public class PublishDraft implements Callable<ReviewResult> {
final Change.Id changeId = patchSetId.getParentKey(); final Change.Id changeId = patchSetId.getParentKey();
result.setChangeId(changeId); result.setChangeId(changeId);
final ChangeControl control = changeControlFactory.validateFor(changeId); final ChangeControl control = changeControlFactory.validateFor(changeId);
final LabelTypes labelTypes = control.getLabelTypes();
final PatchSet patch = db.patchSets().get(patchSetId); final PatchSet patch = db.patchSets().get(patchSetId);
if (patch == null) { if (patch == null) {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
@@ -147,7 +149,8 @@ public class PublishDraft implements Callable<ReviewResult> {
hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, db); hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, db);
sendNotifications(control.getChange().getStatus() == Change.Status.DRAFT, 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<ReviewResult> {
private void sendNotifications(final boolean newChange, private void sendNotifications(final boolean newChange,
final IdentifiedUser currentUser, final Change updatedChange, final IdentifiedUser currentUser, final Change updatedChange,
final PatchSet updatedPatchSet) throws OrmException, IOException, final PatchSet updatedPatchSet, final LabelTypes labelTypes)
PatchSetInfoNotAvailableException { throws OrmException, IOException, PatchSetInfoNotAvailableException {
final Repository git = repoManager.openRepository(updatedChange.getProject()); final Repository git = repoManager.openRepository(updatedChange.getProject());
try { try {
final RevWalk revWalk = new RevWalk(git); final RevWalk revWalk = new RevWalk(git);
@@ -175,7 +178,7 @@ public class PublishDraft implements Callable<ReviewResult> {
recipients.remove(me); recipients.remove(me);
if (newChange) { if (newChange) {
approvalsUtil.addReviewers(db, updatedChange, updatedPatchSet, info, approvalsUtil.addReviewers(db, labelTypes, updatedChange, updatedPatchSet, info,
recipients.getReviewers(), Collections.<Account.Id> emptySet()); recipients.getReviewers(), Collections.<Account.Id> emptySet());
try { try {
CreateChangeSender cm = createChangeSenderFactory.create(updatedChange); CreateChangeSender cm = createChangeSenderFactory.create(updatedChange);
@@ -192,7 +195,7 @@ public class PublishDraft implements Callable<ReviewResult> {
db.patchSetApprovals().byChange(updatedChange.getId()).toList(); db.patchSetApprovals().byChange(updatedChange.getId()).toList();
final MailRecipients oldRecipients = final MailRecipients oldRecipients =
getRecipientsFromApprovals(patchSetApprovals); getRecipientsFromApprovals(patchSetApprovals);
approvalsUtil.addReviewers(db, updatedChange, updatedPatchSet, info, approvalsUtil.addReviewers(db, labelTypes, updatedChange, updatedPatchSet, info,
recipients.getReviewers(), oldRecipients.getAll()); recipients.getReviewers(), oldRecipients.getAll());
final ChangeMessage msg = final ChangeMessage msg =
new ChangeMessage(new ChangeMessage.Key(updatedChange.getId(), new ChangeMessage(new ChangeMessage.Key(updatedChange.getId(),

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.changedetail;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.common.ChangeHookRunner; import com.google.gerrit.common.ChangeHookRunner;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
@@ -373,7 +374,10 @@ public class RebaseChange {
"Change %s was modified", change.getId())); "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 = final ChangeMessage cmsg =
new ChangeMessage(new ChangeMessage.Key(change.getId(), new ChangeMessage(new ChangeMessage.Key(change.getId(),

View File

@@ -53,7 +53,7 @@ public class LabelTypesProvider implements Provider<LabelTypes> {
db.close(); db.close();
} }
} catch (OrmException e) { } 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)); return new LabelTypes(Collections.unmodifiableList(types));

View File

@@ -62,7 +62,6 @@ public class EventFactory {
private static final Logger log = LoggerFactory.getLogger(EventFactory.class); private static final Logger log = LoggerFactory.getLogger(EventFactory.class);
private final AccountCache accountCache; private final AccountCache accountCache;
private final Provider<String> urlProvider; private final Provider<String> urlProvider;
private final LabelTypes labelTypes;
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final SchemaFactory<ReviewDb> schema; private final SchemaFactory<ReviewDb> schema;
private final PatchSetInfoFactory psInfoFactory; private final PatchSetInfoFactory psInfoFactory;
@@ -71,13 +70,11 @@ public class EventFactory {
@Inject @Inject
EventFactory(AccountCache accountCache, EventFactory(AccountCache accountCache,
@CanonicalWebUrl @Nullable Provider<String> urlProvider, @CanonicalWebUrl @Nullable Provider<String> urlProvider,
LabelTypes labelTypes, PatchSetInfoFactory psif,
final PatchSetInfoFactory psif,
PatchListCache patchListCache, SchemaFactory<ReviewDb> schema, PatchListCache patchListCache, SchemaFactory<ReviewDb> schema,
@GerritPersonIdent PersonIdent myIdent) { @GerritPersonIdent PersonIdent myIdent) {
this.accountCache = accountCache; this.accountCache = accountCache;
this.urlProvider = urlProvider; this.urlProvider = urlProvider;
this.labelTypes = labelTypes;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.schema = schema; this.schema = schema;
this.psInfoFactory = psif; this.psInfoFactory = psif;
@@ -244,25 +241,26 @@ public class EventFactory {
a.commitMessage = commitMessage; a.commitMessage = commitMessage;
} }
public void addPatchSets(ChangeAttribute a, Collection<PatchSet> ps) { public void addPatchSets(ChangeAttribute a, Collection<PatchSet> ps,
addPatchSets(a, ps, null, false, null); LabelTypes labelTypes) {
addPatchSets(a, ps, null, false, null, labelTypes);
} }
public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps, public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps,
Map<PatchSet.Id,Collection<PatchSetApproval>> approvals) { Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
addPatchSets(ca, ps, approvals, false, null); LabelTypes labelTypes) {
addPatchSets(ca, ps, approvals, false, null, labelTypes);
} }
public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps, public void addPatchSets(ChangeAttribute ca, Collection<PatchSet> ps,
Map<PatchSet.Id,Collection<PatchSetApproval>> approvals, Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
boolean includeFiles, Change change) { boolean includeFiles, Change change, LabelTypes labelTypes) {
if (!ps.isEmpty()) { if (!ps.isEmpty()) {
ca.patchSets = new ArrayList<PatchSetAttribute>(ps.size()); ca.patchSets = new ArrayList<PatchSetAttribute>(ps.size());
for (PatchSet p : ps) { for (PatchSet p : ps) {
PatchSetAttribute psa = asPatchSetAttribute(p); PatchSetAttribute psa = asPatchSetAttribute(p);
if (approvals != null) { if (approvals != null) {
addApprovals(psa, p.getId(), approvals); addApprovals(psa, p.getId(), approvals, labelTypes);
} }
ca.patchSets.add(psa); ca.patchSets.add(psa);
if (includeFiles && change != null) { if (includeFiles && change != null) {
@@ -381,20 +379,21 @@ public class EventFactory {
} }
public void addApprovals(PatchSetAttribute p, PatchSet.Id id, public void addApprovals(PatchSetAttribute p, PatchSet.Id id,
Map<PatchSet.Id,Collection<PatchSetApproval>> all) { Map<PatchSet.Id, Collection<PatchSetApproval>> all,
LabelTypes labelTypes) {
Collection<PatchSetApproval> list = all.get(id); Collection<PatchSetApproval> list = all.get(id);
if (list != null) { if (list != null) {
addApprovals(p, list); addApprovals(p, list, labelTypes);
} }
} }
public void addApprovals(PatchSetAttribute p, public void addApprovals(PatchSetAttribute p,
Collection<PatchSetApproval> list) { Collection<PatchSetApproval> list, LabelTypes labelTypes) {
if (!list.isEmpty()) { if (!list.isEmpty()) {
p.approvals = new ArrayList<ApprovalAttribute>(list.size()); p.approvals = new ArrayList<ApprovalAttribute>(list.size());
for (PatchSetApproval a : list) { for (PatchSetApproval a : list) {
if (a.getValue() != 0) { if (a.getValue() != 0) {
p.approvals.add(asApprovalAttribute(a)); p.approvals.add(asApprovalAttribute(a, labelTypes));
} }
} }
if (p.approvals.isEmpty()) { if (p.approvals.isEmpty()) {
@@ -451,9 +450,11 @@ public class EventFactory {
* serialization to JSON. * serialization to JSON.
* *
* @param approval * @param approval
* @param labelTypes label types for the containing project
* @return object suitable for serialization to JSON * @return object suitable for serialization to JSON
*/ */
public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval) { public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval,
LabelTypes labelTypes) {
ApprovalAttribute a = new ApprovalAttribute(); ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getCategoryId().get(); a.type = approval.getCategoryId().get();
a.value = Short.toString(approval.getValue()); a.value = Short.toString(approval.getValue());

View File

@@ -26,9 +26,9 @@ import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap; import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.ChangeHooks; 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.LabelType;
import com.google.gerrit.common.data.LabelTypes; 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.common.data.SubmitTypeRecord;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory; 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.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; 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.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.RequestScopePropagator; import com.google.gerrit.server.util.RequestScopePropagator;
@@ -130,7 +131,6 @@ public class MergeOp {
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final MergedSender.Factory mergedSenderFactory; private final MergedSender.Factory mergedSenderFactory;
private final MergeFailSender.Factory mergeFailSenderFactory; private final MergeFailSender.Factory mergeFailSenderFactory;
private final LabelTypes labelTypes;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final IdentifiedUser.GenericFactory identifiedUserFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
@@ -182,7 +182,6 @@ public class MergeOp {
gitRefUpdated = gru; gitRefUpdated = gru;
mergedSenderFactory = msf; mergedSenderFactory = msf;
mergeFailSenderFactory = mfsf; mergeFailSenderFactory = mfsf;
this.labelTypes = labelTypes;
patchSetInfoFactory = psif; patchSetInfoFactory = psif;
identifiedUserFactory = iuf; identifiedUserFactory = iuf;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
@@ -201,7 +200,7 @@ public class MergeOp {
commits = new HashMap<Change.Id, CodeReviewCommit>(); commits = new HashMap<Change.Id, CodeReviewCommit>();
} }
public void verifyMergeability(Change change) { public void verifyMergeability(Change change) throws NoSuchProjectException {
try { try {
setDestProject(); setDestProject();
openRepository(); openRepository();
@@ -269,7 +268,7 @@ public class MergeOp {
} }
} }
public void merge() throws MergeException { public void merge() throws MergeException, NoSuchProjectException {
setDestProject(); setDestProject();
try { try {
openSchema(); openSchema();
@@ -389,7 +388,8 @@ public class MergeOp {
commits.putAll(strategy.getNewCommits()); 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, return submitStrategyFactory.create(submitType, db, repo, rw, inserter,
canMergeFlag, getAlreadyAccepted(branchTip), destBranch, canMergeFlag, getAlreadyAccepted(branchTip), destBranch,
destProject.isUseContentMerge()); destProject.isUseContentMerge());
@@ -931,12 +931,11 @@ public class MergeOp {
c.setStatus(Change.Status.MERGED); c.setStatus(Change.Status.MERGED);
final List<PatchSetApproval> approvals = final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(changeId).toList(); db.patchSetApprovals().byChange(changeId).toList();
final ChangeControl control = changeControlFactory.controlFor(c,
identifiedUserFactory.create(c.getOwner()));
final FunctionState fs = functionState.create( final FunctionState fs = functionState.create(
changeControlFactory.controlFor( control, merged, approvals);
c, for (LabelType lt : control.getLabelTypes().getLabelTypes()) {
identifiedUserFactory.create(c.getOwner())),
merged, approvals);
for (LabelType lt : labelTypes.getLabelTypes()) {
CategoryFunction.forType(lt).run(lt, fs); CategoryFunction.forType(lt).run(lt, fs);
} }
for (PatchSetApproval a : approvals) { for (PatchSetApproval a : approvals) {
@@ -987,7 +986,9 @@ public class MergeOp {
} }
try { 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) { if (from != null) {
cm.setFrom(from.getAccountId()); cm.setFrom(from.getAccountId());
} }

View File

@@ -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.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static org.eclipse.jgit.lib.Constants.R_HEADS; 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.NOT_ATTEMPTED;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK; 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.ChangeHookRunner.HookResult;
import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.Capable; 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.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
@@ -263,6 +265,7 @@ public class ReceiveCommits {
private final ReceivePack rp; private final ReceivePack rp;
private final NoteMap rejectCommits; private final NoteMap rejectCommits;
private MagicBranchInput magicBranch; private MagicBranchInput magicBranch;
private LabelTypes labelTypes;
private List<CreateRequest> newChanges = Collections.emptyList(); private List<CreateRequest> newChanges = Collections.emptyList();
private final Map<Change.Id, ReplaceRequest> replaceByChange = private final Map<Change.Id, ReplaceRequest> replaceByChange =
@@ -1116,6 +1119,7 @@ public class ReceiveCommits {
reject(cmd, "cannot upload review"); reject(cmd, "cannot upload review");
return; return;
} }
labelTypes = projectControl.getLabelTypes();
// Validate that the new commits are connected with the target // Validate that the new commits are connected with the target
// branch. If they aren't, we want to abort. We do this check by // 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.patchSets().insert(Collections.singleton(ps));
db.changes().insert(Collections.singleton(change)); db.changes().insert(Collections.singleton(change));
ChangeUtil.updateTrackingIds(db, change, trackingFooters, footerLines); ChangeUtil.updateTrackingIds(db, change, trackingFooters, footerLines);
approvalsUtil.addReviewers(db, change, ps, info, approvalsUtil.addReviewers(db, labelTypes, change, ps, info,
recipients.getReviewers(), Collections.<Account.Id> emptySet()); recipients.getReviewers(), Collections.<Account.Id> emptySet());
db.commit(); db.commit();
} finally { } finally {
@@ -1769,10 +1773,10 @@ public class ReceiveCommits {
} }
List<PatchSetApproval> patchSetApprovals = List<PatchSetApproval> patchSetApprovals =
approvalsUtil.copyVetosToPatchSet(db, newPatchSet.getId()); approvalsUtil.copyVetosToPatchSet(db, labelTypes, newPatchSet.getId());
final MailRecipients oldRecipients = final MailRecipients oldRecipients =
getRecipientsFromApprovals(patchSetApprovals); getRecipientsFromApprovals(patchSetApprovals);
approvalsUtil.addReviewers(db, change, newPatchSet, info, approvalsUtil.addReviewers(db, labelTypes, change, newPatchSet, info,
recipients.getReviewers(), oldRecipients.getAll()); recipients.getReviewers(), oldRecipients.getAll());
recipients.add(oldRecipients); recipients.add(oldRecipients);
@@ -2161,7 +2165,7 @@ public class ReceiveCommits {
@Override @Override
public void run() { public void run() {
try { try {
final MergedSender cm = mergedSenderFactory.create(result.change); final MergedSender cm = mergedSenderFactory.create(result.changeCtl);
cm.setFrom(currentUser.getAccountId()); cm.setFrom(currentUser.getAccountId());
cm.setPatchSet(result.newPatchSet, result.info); cm.setPatchSet(result.newPatchSet, result.info);
cm.send(); cm.send();

View File

@@ -24,6 +24,8 @@ import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.patch.PatchSetInfoFactory; 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.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -49,9 +51,9 @@ public class SubmitStrategyFactory {
private final PersonIdent myIdent; private final PersonIdent myIdent;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final Provider<String> urlProvider; private final Provider<String> urlProvider;
private final LabelTypes labelTypes;
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final RebaseChange rebaseChange; private final RebaseChange rebaseChange;
private final ProjectControl.Factory projectFactory;
@Inject @Inject
SubmitStrategyFactory( SubmitStrategyFactory(
@@ -59,28 +61,30 @@ public class SubmitStrategyFactory {
@GerritPersonIdent final PersonIdent myIdent, @GerritPersonIdent final PersonIdent myIdent,
final PatchSetInfoFactory patchSetInfoFactory, final PatchSetInfoFactory patchSetInfoFactory,
@CanonicalWebUrl @Nullable final Provider<String> urlProvider, @CanonicalWebUrl @Nullable final Provider<String> urlProvider,
final LabelTypes labelTypes, final GitReferenceUpdated gitRefUpdated, final GitReferenceUpdated gitRefUpdated, final RebaseChange rebaseChange,
final RebaseChange rebaseChange) { final ProjectControl.Factory projectFactory) {
this.identifiedUserFactory = identifiedUserFactory; this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent; this.myIdent = myIdent;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.urlProvider = urlProvider; this.urlProvider = urlProvider;
this.labelTypes = labelTypes;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.rebaseChange = rebaseChange; this.rebaseChange = rebaseChange;
this.projectFactory = projectFactory;
} }
public SubmitStrategy create(final SubmitType submitType, final ReviewDb db, public SubmitStrategy create(final SubmitType submitType, final ReviewDb db,
final Repository repo, final RevWalk rw, final ObjectInserter inserter, final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted, final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch, final boolean useContentMerge) final Branch.NameKey destBranch, final boolean useContentMerge)
throws MergeException { throws MergeException, NoSuchProjectException {
final SubmitStrategy.Arguments args = final SubmitStrategy.Arguments args =
new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db, repo, new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db, repo,
rw, inserter, canMergeFlag, alreadyAccepted, destBranch, rw, inserter, canMergeFlag, alreadyAccepted, destBranch,
useContentMerge); useContentMerge);
switch (submitType) { switch (submitType) {
case CHERRY_PICK: case CHERRY_PICK:
LabelTypes labelTypes =
projectFactory.controlFor(destBranch.getParentKey()).getLabelTypes();
return new CherryPick(args, patchSetInfoFactory, urlProvider, return new CherryPick(args, patchSetInfoFactory, urlProvider,
labelTypes, gitRefUpdated); labelTypes, gitRefUpdated);
case FAST_FORWARD_ONLY: case FAST_FORWARD_ONLY:

View File

@@ -22,8 +22,8 @@ import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; 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.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -31,15 +31,15 @@ import com.google.inject.assistedinject.Assisted;
/** Send notice about a change successfully merged. */ /** Send notice about a change successfully merged. */
public class MergedSender extends ReplyToChangeSender { public class MergedSender extends ReplyToChangeSender {
public static interface Factory { public static interface Factory {
public MergedSender create(Change change); public MergedSender create(ChangeControl change);
} }
private final LabelTypes labelTypes; private final LabelTypes labelTypes;
@Inject @Inject
public MergedSender(EmailArguments ea, LabelTypes lt, @Assisted Change c) { public MergedSender(EmailArguments ea, @Assisted ChangeControl c) {
super(ea, c, "merged"); super(ea, c.getChange(), "merged");
labelTypes = lt; labelTypes = c.getLabelTypes();
} }
@Override @Override

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.project; 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.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.common.data.SubmitTypeRecord;
@@ -211,6 +212,11 @@ public class ChangeControl {
&& getRefControl().canUpload(); // as long as you can upload too && 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. */ /** All value ranges of any allowed label permission. */
public List<PermissionRange> getLabelRanges() { public List<PermissionRange> getLabelRanges() {
return getRefControl().getLabelRanges(); return getRefControl().getLabelRanges();

View File

@@ -20,6 +20,7 @@ import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.Capable;
import com.google.gerrit.common.data.ContributorAgreement; import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.GroupReference; 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.Permission;
import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action; import com.google.gerrit.common.data.PermissionRule.Action;
@@ -181,6 +182,10 @@ public class ProjectControl {
return state.getProject(); return state.getProject();
} }
public LabelTypes getLabelTypes() {
return state.getLabelTypes();
}
private boolean isHidden() { private boolean isHidden() {
return getProject().getState().equals(Project.State.HIDDEN); return getProject().getState().equals(Project.State.HIDDEN);
} }

View File

@@ -20,6 +20,7 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GroupReference; 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.Permission;
import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -65,6 +66,7 @@ public class ProjectState {
private final PrologEnvironment.Factory envFactory; private final PrologEnvironment.Factory envFactory;
private final GitRepositoryManager gitMgr; private final GitRepositoryManager gitMgr;
private final RulesCache rulesCache; private final RulesCache rulesCache;
private final LabelTypes labelTypes;
private final ProjectConfig config; private final ProjectConfig config;
private final Set<AccountGroup.UUID> localOwners; private final Set<AccountGroup.UUID> localOwners;
@@ -89,6 +91,7 @@ public class ProjectState {
final PrologEnvironment.Factory envFactory, final PrologEnvironment.Factory envFactory,
final GitRepositoryManager gitMgr, final GitRepositoryManager gitMgr,
final RulesCache rulesCache, final RulesCache rulesCache,
final LabelTypes labelTypes,
@Assisted final ProjectConfig config) { @Assisted final ProjectConfig config) {
this.projectCache = projectCache; this.projectCache = projectCache;
this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName); this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName);
@@ -101,6 +104,7 @@ public class ProjectState {
this.capabilities = isAllProjects this.capabilities = isAllProjects
? new CapabilityCollection(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)) ? new CapabilityCollection(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES))
: null; : null;
this.labelTypes = labelTypes;
if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) { if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) {
localOwners = Collections.emptySet(); localOwners = Collections.emptySet();
@@ -337,6 +341,10 @@ public class ProjectState {
}); });
} }
public LabelTypes getLabelTypes() {
return labelTypes;
}
private boolean getInheritableBoolean(Function<Project, InheritableBoolean> func) { private boolean getInheritableBoolean(Function<Project, InheritableBoolean> func) {
for (ProjectState s : tree()) { for (ProjectState s : tree()) {
switch (func.apply(s.getProject())) { switch (func.apply(s.getProject())) {

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.query.change; package com.google.gerrit.server.query.change;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -110,7 +109,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
final ChangeControl.GenericFactory changeControlGenericFactory; final ChangeControl.GenericFactory changeControlGenericFactory;
final AccountResolver accountResolver; final AccountResolver accountResolver;
final GroupBackend groupBackend; final GroupBackend groupBackend;
final LabelTypes labelTypes;
final AllProjectsName allProjectsName; final AllProjectsName allProjectsName;
final PatchListCache patchListCache; final PatchListCache patchListCache;
final GitRepositoryManager repoManager; final GitRepositoryManager repoManager;
@@ -124,7 +122,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
ChangeControl.GenericFactory changeControlGenericFactory, ChangeControl.GenericFactory changeControlGenericFactory,
AccountResolver accountResolver, AccountResolver accountResolver,
GroupBackend groupBackend, GroupBackend groupBackend,
LabelTypes labelTypes,
AllProjectsName allProjectsName, AllProjectsName allProjectsName,
PatchListCache patchListCache, PatchListCache patchListCache,
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
@@ -136,7 +133,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.changeControlGenericFactory = changeControlGenericFactory; this.changeControlGenericFactory = changeControlGenericFactory;
this.accountResolver = accountResolver; this.accountResolver = accountResolver;
this.groupBackend = groupBackend; this.groupBackend = groupBackend;
this.labelTypes = labelTypes;
this.allProjectsName = allProjectsName; this.allProjectsName = allProjectsName;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.repoManager = repoManager; this.repoManager = repoManager;
@@ -301,8 +297,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@Operator @Operator
public Predicate<ChangeData> label(String name) { public Predicate<ChangeData> label(String name) {
return new LabelPredicate(args.changeControlGenericFactory, return new LabelPredicate(args.projectCache,
args.userFactory, args.dbProvider, args.labelTypes, name); args.changeControlGenericFactory, args.userFactory, args.dbProvider,
name);
} }
@Operator @Operator

View File

@@ -39,7 +39,7 @@ public class ChangeQueryRewriter extends QueryRewriter<ChangeData> {
new ChangeQueryBuilder.Arguments( // new ChangeQueryBuilder.Arguments( //
new InvalidProvider<ReviewDb>(), // new InvalidProvider<ReviewDb>(), //
new InvalidProvider<ChangeQueryRewriter>(), // new InvalidProvider<ChangeQueryRewriter>(), //
null, null, null, null, null, null, // null, null, null, null, null, //
null, null, null, null), null)); null, null, null, null), null));
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;

View File

@@ -24,6 +24,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; 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.gerrit.server.query.OperatorPredicate;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -103,52 +105,66 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
return Integer.parseInt(value); return Integer.parseInt(value);
} }
private final ProjectCache projectCache;
private final ChangeControl.GenericFactory ccFactory; private final ChangeControl.GenericFactory ccFactory;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final Test test; private final Test test;
private final LabelType type; private final String type;
private final String permissionName;
private final int expVal; private final int expVal;
LabelPredicate(ChangeControl.GenericFactory ccFactory, LabelPredicate(ProjectCache projectCache,
IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider, ChangeControl.GenericFactory ccFactory,
LabelTypes types, String value) { IdentifiedUser.GenericFactory userFactory,
Provider<ReviewDb> dbProvider,
String value) {
super(ChangeQueryBuilder.FIELD_LABEL, value); super(ChangeQueryBuilder.FIELD_LABEL, value);
this.ccFactory = ccFactory; this.ccFactory = ccFactory;
this.projectCache = projectCache;
this.userFactory = userFactory; this.userFactory = userFactory;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
Matcher m1 = Pattern.compile("(=|>=|<=)([+-]?\\d+)$").matcher(value); Matcher m1 = Pattern.compile("(=|>=|<=)([+-]?\\d+)$").matcher(value);
Matcher m2 = Pattern.compile("([+-]\\d+)$").matcher(value); Matcher m2 = Pattern.compile("([+-]\\d+)$").matcher(value);
if (m1.find()) { if (m1.find()) {
type = type(types, value.substring(0, m1.start())); type = value.substring(0, m1.start());
test = op(m1.group(1)); test = op(m1.group(1));
expVal = value(m1.group(2)); expVal = value(m1.group(2));
} else if (m2.find()) { } else if (m2.find()) {
type = type(types, value.substring(0, m2.start())); type = value.substring(0, m2.start());
test = Test.EQ; test = Test.EQ;
expVal = value(m2.group(1)); expVal = value(m2.group(1));
} else { } else {
type = type(types, value); type = value;
test = Test.EQ; test = Test.EQ;
expVal = 1; expVal = 1;
} }
this.permissionName = Permission.forLabel(type.getName());
} }
@Override @Override
public boolean match(final ChangeData object) throws OrmException { 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<Account.Id> allApprovers = new HashSet<Account.Id>(); final Set<Account.Id> allApprovers = new HashSet<Account.Id>();
final Set<Account.Id> approversThatVotedInCategory = new HashSet<Account.Id>(); final Set<Account.Id> approversThatVotedInCategory = new HashSet<Account.Id>();
for (PatchSetApproval p : object.currentApprovals(dbProvider)) { for (PatchSetApproval p : object.currentApprovals(dbProvider)) {
allApprovers.add(p.getAccountId()); allApprovers.add(p.getAccountId());
if (p.getCategoryId().get().equals(type.getId())) { if (p.getCategoryId().get().equals(labelType.getId())) {
approversThatVotedInCategory.add(p.getAccountId()); approversThatVotedInCategory.add(p.getAccountId());
if (match(object.change(dbProvider), p.getValue(), p.getAccountId())) { if (match(c, p.getValue(), p.getAccountId(), labelType)) {
return true; return true;
} }
} }
@@ -156,8 +172,8 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
final Set<Account.Id> approversThatDidNotVoteInCategory = new HashSet<Account.Id>(allApprovers); final Set<Account.Id> approversThatDidNotVoteInCategory = new HashSet<Account.Id>(allApprovers);
approversThatDidNotVoteInCategory.removeAll(approversThatVotedInCategory); approversThatDidNotVoteInCategory.removeAll(approversThatVotedInCategory);
for (final Account.Id a : approversThatDidNotVoteInCategory) { for (Account.Id a : approversThatDidNotVoteInCategory) {
if (match(object.change(dbProvider), 0, a)) { if (match(c, 0, a, labelType)) {
return true; return true;
} }
} }
@@ -166,7 +182,8 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
} }
private boolean match(final Change change, final int value, 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; int psVal = value;
if (test.match(psVal, expVal)) { if (test.match(psVal, expVal)) {
// Double check the value is still permitted for the user. // Double check the value is still permitted for the user.
@@ -179,7 +196,7 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
// //
return false; return false;
} }
psVal = cc.getRange(permissionName).squash(psVal); psVal = cc.getRange(Permission.forLabel(type.getName())).squash(psVal);
} catch (NoSuchChangeException e) { } catch (NoSuchChangeException e) {
// The project has disappeared. // The project has disappeared.
// //

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change; package com.google.gerrit.server.query.change;
import com.google.gerrit.common.data.GlobalCapability; 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.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; 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.PatchSetAttribute;
import com.google.gerrit.server.events.QueryStats; import com.google.gerrit.server.events.QueryStats;
import com.google.gerrit.server.git.GitRepositoryManager; 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.Predicate;
import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.QueryParseException;
import com.google.gson.Gson; import com.google.gson.Gson;
@@ -93,6 +96,7 @@ public class QueryProcessor {
private final ChangeQueryRewriter queryRewriter; private final ChangeQueryRewriter queryRewriter;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final ChangeControl.Factory changeControlFactory;
private final int maxLimit; private final int maxLimit;
private OutputFormat outputFormat = OutputFormat.TEXT; private OutputFormat outputFormat = OutputFormat.TEXT;
@@ -116,12 +120,14 @@ public class QueryProcessor {
QueryProcessor(EventFactory eventFactory, QueryProcessor(EventFactory eventFactory,
ChangeQueryBuilder.Factory queryBuilder, CurrentUser currentUser, ChangeQueryBuilder.Factory queryBuilder, CurrentUser currentUser,
ChangeQueryRewriter queryRewriter, Provider<ReviewDb> db, ChangeQueryRewriter queryRewriter, Provider<ReviewDb> db,
GitRepositoryManager repoManager) { GitRepositoryManager repoManager,
ChangeControl.Factory changeControlFactory) {
this.eventFactory = eventFactory; this.eventFactory = eventFactory;
this.queryBuilder = queryBuilder.create(currentUser); this.queryBuilder = queryBuilder.create(currentUser);
this.queryRewriter = queryRewriter; this.queryRewriter = queryRewriter;
this.db = db; this.db = db;
this.repoManager = repoManager; this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.maxLimit = currentUser.getCapabilities() this.maxLimit = currentUser.getCapabilities()
.getRange(GlobalCapability.QUERY_LIMIT) .getRange(GlobalCapability.QUERY_LIMIT)
.getMax(); .getMax();
@@ -267,6 +273,8 @@ public class QueryProcessor {
List<ChangeData> results = queryChanges(queryString); List<ChangeData> results = queryChanges(queryString);
ChangeAttribute c = null; ChangeAttribute c = null;
for (ChangeData d : results) { for (ChangeData d : results) {
LabelTypes labelTypes = changeControlFactory.controlFor(d.getChange())
.getLabelTypes();
c = eventFactory.asChangeAttribute(d.getChange()); c = eventFactory.asChangeAttribute(d.getChange());
eventFactory.extend(c, d.getChange()); eventFactory.extend(c, d.getChange());
eventFactory.addTrackingIds(c, d.trackingIds(db)); eventFactory.addTrackingIds(c, d.trackingIds(db));
@@ -287,10 +295,11 @@ public class QueryProcessor {
if (includeFiles) { if (includeFiles) {
eventFactory.addPatchSets(c, d.patches(db), eventFactory.addPatchSets(c, d.patches(db),
includeApprovals ? d.approvalsMap(db).asMap() : null, includeApprovals ? d.approvalsMap(db).asMap() : null,
includeFiles, d.change(db)); includeFiles, d.change(db), labelTypes);
} else { } else {
eventFactory.addPatchSets(c, d.patches(db), 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); PatchSet current = d.currentPatchSet(db);
if (current != null) { if (current != null) {
c.currentPatchSet = eventFactory.asPatchSetAttribute(current); c.currentPatchSet = eventFactory.asPatchSetAttribute(current);
eventFactory.addApprovals(c.currentPatchSet, // eventFactory.addApprovals(c.currentPatchSet,
d.currentApprovals(db)); d.currentApprovals(db), labelTypes);
if (includeFiles) { if (includeFiles) {
eventFactory.addPatchSetFileNames(c.currentPatchSet, eventFactory.addPatchSetFileNames(c.currentPatchSet,
@@ -342,6 +351,11 @@ public class QueryProcessor {
ErrorMessage m = new ErrorMessage(); ErrorMessage m = new ErrorMessage();
m.message = e.getMessage(); m.message = e.getMessage();
show(m); 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 { } finally {
try { try {

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.workflow; package com.google.gerrit.server.workflow;
import com.google.gerrit.common.data.LabelType; 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.LabelValue;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.PermissionRange;
@@ -42,7 +41,6 @@ public class FunctionState {
Collection<PatchSetApproval> all); Collection<PatchSetApproval> all);
} }
private final LabelTypes labelTypes;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final Map<String, Collection<PatchSetApproval>> approvals = private final Map<String, Collection<PatchSetApproval>> approvals =
@@ -52,11 +50,9 @@ public class FunctionState {
private final Change change; private final Change change;
@Inject @Inject
FunctionState(final LabelTypes labelTypes, FunctionState(final IdentifiedUser.GenericFactory userFactory,
final IdentifiedUser.GenericFactory userFactory,
@Assisted final ChangeControl c, @Assisted final PatchSet.Id psId, @Assisted final ChangeControl c, @Assisted final PatchSet.Id psId,
@Assisted final Collection<PatchSetApproval> all) { @Assisted final Collection<PatchSetApproval> all) {
this.labelTypes = labelTypes;
this.userFactory = userFactory; this.userFactory = userFactory;
callerChangeControl = c; callerChangeControl = c;
@@ -68,7 +64,7 @@ public class FunctionState {
approvals.get(ca.getCategoryId().get()); approvals.get(ca.getCategoryId().get());
if (l == null) { if (l == null) {
l = new ArrayList<PatchSetApproval>(); l = new ArrayList<PatchSetApproval>();
LabelType lt = labelTypes.byId(ca.getCategoryId().get()); LabelType lt = c.getLabelTypes().byId(ca.getCategoryId().get());
if (lt != null) { if (lt != null) {
// TODO: Support arbitrary labels // TODO: Support arbitrary labels
approvals.put(lt.getName(), l); approvals.put(lt.getName(), l);
@@ -80,7 +76,7 @@ public class FunctionState {
} }
List<LabelType> getLabelTypes() { List<LabelType> getLabelTypes() {
return labelTypes.getLabelTypes(); return callerChangeControl.getLabelTypes().getLabelTypes();
} }
Change getChange() { Change getChange() {

View File

@@ -541,10 +541,10 @@ public class RefControlTest extends TestCase {
RulesCache rulesCache = null; RulesCache rulesCache = null;
all.put(local.getProject().getNameKey(), new ProjectState( all.put(local.getProject().getNameKey(), new ProjectState(
projectCache, allProjectsName, projectControlFactory, projectCache, allProjectsName, projectControlFactory,
envFactory, mgr, rulesCache, local)); envFactory, mgr, rulesCache, null, local));
all.put(parent.getProject().getNameKey(), new ProjectState( all.put(parent.getProject().getNameKey(), new ProjectState(
projectCache, allProjectsName, projectControlFactory, projectCache, allProjectsName, projectControlFactory,
envFactory, mgr, rulesCache, parent)); envFactory, mgr, rulesCache, null, parent));
return all.get(local.getProject().getNameKey()); return all.get(local.getProject().getNameKey());
} }