Merge "Make ChangeControl package-private"

This commit is contained in:
Patrick Hiesel
2017-10-10 11:34:06 +00:00
committed by Gerrit Code Review
35 changed files with 266 additions and 392 deletions

View File

@@ -214,7 +214,8 @@ public class ChangeEdits
@Override @Override
public Response<EditInfo> apply(ChangeResource rsrc) public Response<EditInfo> apply(ChangeResource rsrc)
throws AuthException, IOException, ResourceNotFoundException, OrmException { throws AuthException, IOException, ResourceNotFoundException, OrmException,
PermissionBackendException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser()); Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser());
if (!edit.isPresent()) { if (!edit.isPresent()) {
return Response.none(); return Response.none();

View File

@@ -83,6 +83,7 @@ import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.config.DownloadCommand; import com.google.gerrit.extensions.config.DownloadCommand;
import com.google.gerrit.extensions.config.DownloadScheme; import com.google.gerrit.extensions.config.DownloadScheme;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.index.query.QueryResult; import com.google.gerrit.index.query.QueryResult;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
@@ -116,11 +117,12 @@ import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.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.RemoveReviewerControl; import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.project.SubmitRuleOptions; import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -228,7 +230,6 @@ public class ChangeJson {
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final RemoveReviewerControl removeReviewerControl; private final RemoveReviewerControl removeReviewerControl;
private final TrackingFooters trackingFooters; private final TrackingFooters trackingFooters;
private final ChangeControl.GenericFactory changeControlFactory;
private boolean lazyLoad = true; private boolean lazyLoad = true;
private AccountLoader accountLoader; private AccountLoader accountLoader;
private FixInput fix; private FixInput fix;
@@ -261,7 +262,6 @@ public class ChangeJson {
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
RemoveReviewerControl removeReviewerControl, RemoveReviewerControl removeReviewerControl,
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
ChangeControl.GenericFactory changeControlFactory,
@Assisted Iterable<ListChangesOption> options) { @Assisted Iterable<ListChangesOption> options) {
this.db = db; this.db = db;
this.userProvider = user; this.userProvider = user;
@@ -287,7 +287,6 @@ public class ChangeJson {
this.indexes = indexes; this.indexes = indexes;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.removeReviewerControl = removeReviewerControl; this.removeReviewerControl = removeReviewerControl;
this.changeControlFactory = changeControlFactory;
this.options = Sets.immutableEnumSet(options); this.options = Sets.immutableEnumSet(options);
this.trackingFooters = trackingFooters; this.trackingFooters = trackingFooters;
} }
@@ -347,6 +346,7 @@ public class ChangeJson {
| OrmException | OrmException
| IOException | IOException
| PermissionBackendException | PermissionBackendException
| NoSuchProjectException
| RuntimeException e) { | RuntimeException e) {
if (!has(CHECK)) { if (!has(CHECK)) {
Throwables.throwIfInstanceOf(e, OrmException.class); Throwables.throwIfInstanceOf(e, OrmException.class);
@@ -425,6 +425,7 @@ public class ChangeJson {
| OrmException | OrmException
| IOException | IOException
| PermissionBackendException | PermissionBackendException
| NoSuchProjectException
| RuntimeException e) { | RuntimeException e) {
if (has(CHECK)) { if (has(CHECK)) {
i = checkOnly(cd); i = checkOnly(cd);
@@ -491,7 +492,7 @@ public class ChangeJson {
private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId) private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
throws PatchListNotAvailableException, GpgException, OrmException, IOException, throws PatchListNotAvailableException, GpgException, OrmException, IOException,
PermissionBackendException { PermissionBackendException, NoSuchProjectException {
ChangeInfo out = new ChangeInfo(); ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get(); CurrentUser user = userProvider.get();
@@ -506,11 +507,7 @@ public class ChangeJson {
} }
} }
PermissionBackend.WithUser withUser = permissionBackend.user(user).database(db); PermissionBackend.ForChange perm = permissionBackendForChange(user, cd);
PermissionBackend.ForChange perm =
lazyLoad
? withUser.change(cd)
: withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
Change in = cd.change(); Change in = cd.change();
out.project = in.getProject().get(); out.project = in.getProject().get();
out.branch = in.getDest().getShortName(); out.branch = in.getDest().getShortName();
@@ -596,19 +593,15 @@ public class ChangeJson {
src = null; src = null;
} }
ChangeControl ctl = null;
if (needMessages || needRevisions) {
ctl = changeControlFactory.controlFor(db.get(), cd.change(), userProvider.get());
}
if (needMessages) { if (needMessages) {
out.messages = messages(ctl, cd); out.messages = messages(cd);
} }
finish(out); finish(out);
// This block must come after the ChangeInfo is mostly populated, since // This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is. // it will be passed to ActionVisitors as-is.
if (needRevisions) { if (needRevisions) {
out.revisions = revisions(ctl, cd, src, limitToPsId, out); out.revisions = revisions(cd, src, limitToPsId, out);
if (out.revisions != null) { if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) { for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) { if (entry.getValue().isCurrent) {
@@ -1103,8 +1096,7 @@ public class ChangeJson {
return result; return result;
} }
private Collection<ChangeMessageInfo> messages(ChangeControl ctl, ChangeData cd) private Collection<ChangeMessageInfo> messages(ChangeData cd) throws OrmException {
throws OrmException {
List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes()); List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes());
if (messages.isEmpty()) { if (messages.isEmpty()) {
return Collections.emptyList(); return Collections.emptyList();
@@ -1113,26 +1105,24 @@ public class ChangeJson {
List<ChangeMessageInfo> result = Lists.newArrayListWithCapacity(messages.size()); List<ChangeMessageInfo> result = Lists.newArrayListWithCapacity(messages.size());
for (ChangeMessage message : messages) { for (ChangeMessage message : messages) {
PatchSet.Id patchNum = message.getPatchSetId(); PatchSet.Id patchNum = message.getPatchSetId();
if (patchNum == null || ctl.isVisible(db.get())) { ChangeMessageInfo cmi = new ChangeMessageInfo();
ChangeMessageInfo cmi = new ChangeMessageInfo(); cmi.id = message.getKey().get();
cmi.id = message.getKey().get(); cmi.author = accountLoader.get(message.getAuthor());
cmi.author = accountLoader.get(message.getAuthor()); cmi.date = message.getWrittenOn();
cmi.date = message.getWrittenOn(); cmi.message = message.getMessage();
cmi.message = message.getMessage(); cmi.tag = message.getTag();
cmi.tag = message.getTag(); cmi._revisionNumber = patchNum != null ? patchNum.get() : null;
cmi._revisionNumber = patchNum != null ? patchNum.get() : null; Account.Id realAuthor = message.getRealAuthor();
Account.Id realAuthor = message.getRealAuthor(); if (realAuthor != null) {
if (realAuthor != null) { cmi.realAuthor = accountLoader.get(realAuthor);
cmi.realAuthor = accountLoader.get(realAuthor);
}
result.add(cmi);
} }
result.add(cmi);
} }
return result; return result;
} }
private Collection<AccountInfo> removableReviewers(ChangeData cd, ChangeInfo out) private Collection<AccountInfo> removableReviewers(ChangeData cd, ChangeInfo out)
throws PermissionBackendException, NoSuchChangeException, OrmException { throws PermissionBackendException, NoSuchProjectException, OrmException, IOException {
// Although this is called removableReviewers, this method also determines // Although this is called removableReviewers, this method also determines
// which CCs are removable. // which CCs are removable.
// //
@@ -1228,13 +1218,14 @@ public class ChangeJson {
} }
private Map<String, RevisionInfo> revisions( private Map<String, RevisionInfo> revisions(
ChangeControl ctl,
ChangeData cd, ChangeData cd,
Map<PatchSet.Id, PatchSet> map, Map<PatchSet.Id, PatchSet> map,
Optional<PatchSet.Id> limitToPsId, Optional<PatchSet.Id> limitToPsId,
ChangeInfo changeInfo) ChangeInfo changeInfo)
throws PatchListNotAvailableException, GpgException, OrmException, IOException { throws PatchListNotAvailableException, GpgException, OrmException, IOException,
PermissionBackendException {
Map<String, RevisionInfo> res = new LinkedHashMap<>(); Map<String, RevisionInfo> res = new LinkedHashMap<>();
Boolean isWorldReadable = null;
try (Repository repo = openRepoIfNecessary(cd.project()); try (Repository repo = openRepoIfNecessary(cd.project());
RevWalk rw = newRevWalk(repo)) { RevWalk rw = newRevWalk(repo)) {
for (PatchSet in : map.values()) { for (PatchSet in : map.values()) {
@@ -1247,8 +1238,13 @@ public class ChangeJson {
} else { } else {
want = id.equals(cd.change().currentPatchSetId()); want = id.equals(cd.change().currentPatchSetId());
} }
if (want && ctl.isVisible(db.get())) { if (want) {
res.put(in.getRevision().get(), toRevisionInfo(cd, in, repo, rw, false, changeInfo)); if (isWorldReadable == null) {
isWorldReadable = isWorldReadable(cd);
}
res.put(
in.getRevision().get(),
toRevisionInfo(cd, in, repo, rw, false, changeInfo, isWorldReadable));
} }
} }
return res; return res;
@@ -1283,11 +1279,12 @@ public class ChangeJson {
} }
public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in) public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in)
throws PatchListNotAvailableException, GpgException, OrmException, IOException { throws PatchListNotAvailableException, GpgException, OrmException, IOException,
PermissionBackendException {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
try (Repository repo = openRepoIfNecessary(cd.project()); try (Repository repo = openRepoIfNecessary(cd.project());
RevWalk rw = newRevWalk(repo)) { RevWalk rw = newRevWalk(repo)) {
RevisionInfo rev = toRevisionInfo(cd, in, repo, rw, true, null); RevisionInfo rev = toRevisionInfo(cd, in, repo, rw, true, null, isWorldReadable(cd));
accountLoader.fill(); accountLoader.fill();
return rev; return rev;
} }
@@ -1299,8 +1296,10 @@ public class ChangeJson {
@Nullable Repository repo, @Nullable Repository repo,
@Nullable RevWalk rw, @Nullable RevWalk rw,
boolean fillCommit, boolean fillCommit,
@Nullable ChangeInfo changeInfo) @Nullable ChangeInfo changeInfo,
throws PatchListNotAvailableException, GpgException, OrmException, IOException { boolean isWorldReadable)
throws PatchListNotAvailableException, GpgException, OrmException, IOException,
PermissionBackendException {
Change c = cd.change(); Change c = cd.change();
RevisionInfo out = new RevisionInfo(); RevisionInfo out = new RevisionInfo();
out.isCurrent = in.getId().equals(c.currentPatchSetId()); out.isCurrent = in.getId().equals(c.currentPatchSetId());
@@ -1308,7 +1307,7 @@ public class ChangeJson {
out.ref = in.getRefName(); out.ref = in.getRefName();
out.created = in.getCreatedOn(); out.created = in.getCreatedOn();
out.uploader = accountLoader.get(in.getUploader()); out.uploader = accountLoader.get(in.getUploader());
out.fetch = makeFetchMap(cd, in); out.fetch = makeFetchMap(cd, in, isWorldReadable);
out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in); out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in);
out.description = in.getDescription(); out.description = in.getDescription();
@@ -1398,10 +1397,9 @@ public class ChangeJson {
return info; return info;
} }
private Map<String, FetchInfo> makeFetchMap(ChangeData cd, PatchSet in) throws OrmException { private Map<String, FetchInfo> makeFetchMap(ChangeData cd, PatchSet in, boolean isWorldReadable)
throws OrmException, PermissionBackendException {
Map<String, FetchInfo> r = new LinkedHashMap<>(); Map<String, FetchInfo> r = new LinkedHashMap<>();
ChangeControl ctl = changeControlFactory.controlFor(db.get(), cd.change(), anonymous);
for (DynamicMap.Entry<DownloadScheme> e : downloadSchemes) { for (DynamicMap.Entry<DownloadScheme> e : downloadSchemes) {
String schemeName = e.getExportName(); String schemeName = e.getExportName();
DownloadScheme scheme = e.getProvider().get(); DownloadScheme scheme = e.getProvider().get();
@@ -1409,8 +1407,7 @@ public class ChangeJson {
|| (scheme.isAuthRequired() && !userProvider.get().isIdentifiedUser())) { || (scheme.isAuthRequired() && !userProvider.get().isIdentifiedUser())) {
continue; continue;
} }
if (!scheme.isAuthSupported() && !isWorldReadable) {
if (!scheme.isAuthSupported() && !ctl.isVisible(db.get())) {
continue; continue;
} }
@@ -1464,6 +1461,27 @@ public class ChangeJson {
label.all.add(approval); label.all.add(approval);
} }
/**
* @return {@link PermissionBackend.ForChange} constructed from either an index-backed or a
* database-backed {@link ChangeData} depending on {@code lazyload}.
*/
private PermissionBackend.ForChange permissionBackendForChange(CurrentUser user, ChangeData cd)
throws OrmException {
PermissionBackend.WithUser withUser = permissionBackend.user(user).database(db);
return lazyLoad
? withUser.change(cd)
: withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
}
private boolean isWorldReadable(ChangeData cd) throws OrmException, PermissionBackendException {
try {
permissionBackendForChange(anonymous, cd).check(ChangePermission.READ);
return true;
} catch (AuthException ae) {
return false;
}
}
@AutoValue @AutoValue
abstract static class LabelWithStatus { abstract static class LabelWithStatus {
private static LabelWithStatus create(LabelInfo label, SubmitRecord.Label.Status status) { private static LabelWithStatus create(LabelInfo label, SubmitRecord.Label.Status status) {

View File

@@ -39,6 +39,7 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
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.RemoveReviewerControl; import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.BatchUpdateOp;
@@ -119,7 +120,7 @@ public class DeleteReviewerOp implements BatchUpdateOp {
@Override @Override
public boolean updateChange(ChangeContext ctx) public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException, throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException,
IOException { IOException, NoSuchProjectException {
Account.Id reviewerId = reviewer.getId(); Account.Id reviewerId = reviewer.getId();
if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) { if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
throw new ResourceNotFoundException(); throw new ResourceNotFoundException();

View File

@@ -41,6 +41,7 @@ import com.google.gerrit.server.extensions.events.VoteDeleted;
import com.google.gerrit.server.mail.send.DeleteVoteSender; import com.google.gerrit.server.mail.send.DeleteVoteSender;
import com.google.gerrit.server.mail.send.ReplyToChangeSender; import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.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.project.RemoveReviewerControl; import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -162,7 +163,7 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
@Override @Override
public boolean updateChange(ChangeContext ctx) public boolean updateChange(ChangeContext ctx)
throws OrmException, AuthException, ResourceNotFoundException, IOException, throws OrmException, AuthException, ResourceNotFoundException, IOException,
PermissionBackendException { PermissionBackendException, NoSuchProjectException {
change = ctx.getChange(); change = ctx.getChange();
PatchSet.Id psId = change.currentPatchSetId(); PatchSet.Id psId = change.currentPatchSetId();
ps = psUtil.current(db.get(), ctx.getNotes()); ps = psUtil.current(db.get(), ctx.getNotes());

View File

@@ -43,6 +43,7 @@ import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey; import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -146,7 +147,8 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
@Override @Override
public Response<?> apply(RevisionResource resource) public Response<?> apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceNotFoundException, OrmException, throws AuthException, BadRequestException, ResourceNotFoundException, OrmException,
RepositoryNotFoundException, IOException, PatchListNotAvailableException { RepositoryNotFoundException, IOException, PatchListNotAvailableException,
PermissionBackendException {
checkOptions(); checkOptions();
if (reviewed) { if (reviewed) {
return Response.ok(reviewed(resource)); return Response.ok(reviewed(resource));

View File

@@ -47,6 +47,7 @@ import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.git.LargeObjectException; import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchScriptFactory; import com.google.gerrit.server.patch.PatchScriptFactory;
import com.google.gerrit.server.permissions.PermissionBackendException;
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.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
@@ -122,7 +123,7 @@ public class GetDiff implements RestReadView<FileResource> {
@Override @Override
public Response<DiffInfo> apply(FileResource resource) public Response<DiffInfo> apply(FileResource resource)
throws ResourceConflictException, ResourceNotFoundException, OrmException, AuthException, throws ResourceConflictException, ResourceNotFoundException, OrmException, AuthException,
InvalidChangeOperationException, IOException { InvalidChangeOperationException, IOException, PermissionBackendException {
DiffPreferencesInfo prefs = new DiffPreferencesInfo(); DiffPreferencesInfo prefs = new DiffPreferencesInfo();
if (whitespace != null) { if (whitespace != null) {
prefs.ignoreWhitespace = whitespace; prefs.ignoreWhitespace = whitespace;

View File

@@ -26,7 +26,6 @@ import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -53,7 +52,6 @@ public class GetPureRevert implements RestReadView<ChangeResource> {
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final ChangeControl.GenericFactory changeControlFactory;
@Option( @Option(
name = "--claimed-original", name = "--claimed-original",
@@ -70,15 +68,13 @@ public class GetPureRevert implements RestReadView<ChangeResource> {
ProjectCache projectCache, ProjectCache projectCache,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
PatchSetUtil psUtil, PatchSetUtil psUtil) {
ChangeControl.GenericFactory changeControlFactory) {
this.mergeUtilFactory = mergeUtilFactory; this.mergeUtilFactory = mergeUtilFactory;
this.repoManager = repoManager; this.repoManager = repoManager;
this.projectCache = projectCache; this.projectCache = projectCache;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.psUtil = psUtil; this.psUtil = psUtil;
this.changeControlFactory = changeControlFactory;
} }
@Override @Override
@@ -88,10 +84,6 @@ public class GetPureRevert implements RestReadView<ChangeResource> {
PatchSet currentPatchSet = psUtil.current(dbProvider.get(), rsrc.getNotes()); PatchSet currentPatchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
if (currentPatchSet == null) { if (currentPatchSet == null) {
throw new ResourceConflictException("current revision is missing"); throw new ResourceConflictException("current revision is missing");
} else if (!changeControlFactory
.controlFor(rsrc.getNotes(), rsrc.getUser())
.isVisible(dbProvider.get())) {
throw new AuthException("current revision not accessible");
} }
return getPureRevert(rsrc.getNotes()); return getPureRevert(rsrc.getNotes());
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RelatedChangesSorter.PatchSetData; import com.google.gerrit.server.change.RelatedChangesSorter.PatchSetData;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException; 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.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -64,14 +65,15 @@ public class GetRelated implements RestReadView<RevisionResource> {
@Override @Override
public RelatedInfo apply(RevisionResource rsrc) public RelatedInfo apply(RevisionResource rsrc)
throws RepositoryNotFoundException, IOException, OrmException, NoSuchProjectException { throws RepositoryNotFoundException, IOException, OrmException, NoSuchProjectException,
PermissionBackendException {
RelatedInfo relatedInfo = new RelatedInfo(); RelatedInfo relatedInfo = new RelatedInfo();
relatedInfo.changes = getRelated(rsrc); relatedInfo.changes = getRelated(rsrc);
return relatedInfo; return relatedInfo;
} }
private List<ChangeAndCommit> getRelated(RevisionResource rsrc) private List<ChangeAndCommit> getRelated(RevisionResource rsrc)
throws OrmException, IOException, NoSuchProjectException { throws OrmException, IOException, NoSuchProjectException, PermissionBackendException {
Set<String> groups = getAllGroups(rsrc.getNotes()); Set<String> groups = getAllGroups(rsrc.getNotes());
if (groups.isEmpty()) { if (groups.isEmpty()) {
return Collections.emptyList(); return Collections.emptyList();

View File

@@ -35,7 +35,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryHelper;
@@ -73,7 +72,6 @@ public class PutMessage
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final NotifyUtil notifyUtil; private final NotifyUtil notifyUtil;
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final ChangeControl.GenericFactory changeControlFactory;
@Inject @Inject
PutMessage( PutMessage(
@@ -86,8 +84,7 @@ public class PutMessage
@GerritPersonIdent PersonIdent gerritIdent, @GerritPersonIdent PersonIdent gerritIdent,
PatchSetUtil psUtil, PatchSetUtil psUtil,
NotifyUtil notifyUtil, NotifyUtil notifyUtil,
ProjectCache projectCache, ProjectCache projectCache) {
ChangeControl.GenericFactory changeControlFactory) {
super(retryHelper); super(retryHelper);
this.repositoryManager = repositoryManager; this.repositoryManager = repositoryManager;
this.currentUserProvider = currentUserProvider; this.currentUserProvider = currentUserProvider;
@@ -98,7 +95,6 @@ public class PutMessage
this.psUtil = psUtil; this.psUtil = psUtil;
this.notifyUtil = notifyUtil; this.notifyUtil = notifyUtil;
this.projectCache = projectCache; this.projectCache = projectCache;
this.changeControlFactory = changeControlFactory;
} }
@Override @Override
@@ -109,10 +105,6 @@ public class PutMessage
PatchSet ps = psUtil.current(db.get(), resource.getNotes()); PatchSet ps = psUtil.current(db.get(), resource.getNotes());
if (ps == null) { if (ps == null) {
throw new ResourceConflictException("current revision is missing"); throw new ResourceConflictException("current revision is missing");
} else if (!changeControlFactory
.controlFor(resource.getNotes(), resource.getUser())
.isVisible(db.get())) {
throw new AuthException("current revision not accessible");
} }
if (input == null) { if (input == null) {

View File

@@ -35,13 +35,12 @@ import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RebaseUtil.Base; import com.google.gerrit.server.change.RebaseUtil.Base;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
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.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper; import com.google.gerrit.server.update.RetryHelper;
@@ -74,8 +73,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
private final RebaseUtil rebaseUtil; private final RebaseUtil rebaseUtil;
private final ChangeJson.Factory json; private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final Provider<CurrentUser> userProvider; private final PermissionBackend permissionBackend;
private final ChangeControl.GenericFactory changeControlFactory;
@Inject @Inject
public Rebase( public Rebase(
@@ -85,16 +83,14 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
RebaseUtil rebaseUtil, RebaseUtil rebaseUtil,
ChangeJson.Factory json, ChangeJson.Factory json,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
Provider<CurrentUser> userProvider, PermissionBackend permissionBackend) {
ChangeControl.GenericFactory changeControlFactory) {
super(retryHelper); super(retryHelper);
this.repoManager = repoManager; this.repoManager = repoManager;
this.rebaseFactory = rebaseFactory; this.rebaseFactory = rebaseFactory;
this.rebaseUtil = rebaseUtil; this.rebaseUtil = rebaseUtil;
this.json = json; this.json = json;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.userProvider = userProvider; this.permissionBackend = permissionBackend;
this.changeControlFactory = changeControlFactory;
} }
@Override @Override
@@ -132,7 +128,8 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
private ObjectId findBaseRev( private ObjectId findBaseRev(
Repository repo, RevWalk rw, RevisionResource rsrc, RebaseInput input) Repository repo, RevWalk rw, RevisionResource rsrc, RebaseInput input)
throws RestApiException, OrmException, IOException, NoSuchChangeException { throws RestApiException, OrmException, IOException, NoSuchChangeException, AuthException,
PermissionBackendException {
Branch.NameKey destRefKey = rsrc.getChange().getDest(); Branch.NameKey destRefKey = rsrc.getChange().getDest();
if (input == null || input.base == null) { if (input == null || input.base == null) {
return rebaseUtil.findBaseRevision(rsrc.getPatchSet(), destRefKey, repo, rw); return rebaseUtil.findBaseRevision(rsrc.getPatchSet(), destRefKey, repo, rw);
@@ -150,20 +147,21 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
return destRef.getObjectId(); return destRef.getObjectId();
} }
@SuppressWarnings("resource")
ReviewDb db = dbProvider.get();
Base base = rebaseUtil.parseBase(rsrc, str); Base base = rebaseUtil.parseBase(rsrc, str);
if (base == null) { if (base == null) {
throw new ResourceConflictException("base revision is missing: " + str); throw new ResourceConflictException("base revision is missing: " + str);
} }
PatchSet.Id baseId = base.patchSet().getId(); PatchSet.Id baseId = base.patchSet().getId();
ChangeControl baseCtl = changeControlFactory.controlFor(base.notes(), userProvider.get()); if (change.getId().equals(baseId.getParentKey())) {
if (!baseCtl.isVisible(db)) {
throw new AuthException("base revision not accessible: " + str);
} else if (change.getId().equals(baseId.getParentKey())) {
throw new ResourceConflictException("cannot rebase change onto itself"); throw new ResourceConflictException("cannot rebase change onto itself");
} }
permissionBackend
.user(rsrc.getUser())
.database(dbProvider)
.change(base.notes())
.check(ChangePermission.READ);
Change baseChange = base.notes().getChange(); Change baseChange = base.notes().getChange();
if (!baseChange.getProject().equals(change.getProject())) { if (!baseChange.getProject().equals(change.getProject())) {
throw new ResourceConflictException( throw new ResourceConflictException(
@@ -228,18 +226,12 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
extends RetryingRestModifyView<ChangeResource, RebaseInput, ChangeInfo> { extends RetryingRestModifyView<ChangeResource, RebaseInput, ChangeInfo> {
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final Rebase rebase; private final Rebase rebase;
private final ChangeControl.GenericFactory changeControlFactory;
@Inject @Inject
CurrentRevision( CurrentRevision(RetryHelper retryHelper, PatchSetUtil psUtil, Rebase rebase) {
RetryHelper retryHelper,
PatchSetUtil psUtil,
Rebase rebase,
ChangeControl.GenericFactory changeControlFactory) {
super(retryHelper); super(retryHelper);
this.psUtil = psUtil; this.psUtil = psUtil;
this.rebase = rebase; this.rebase = rebase;
this.changeControlFactory = changeControlFactory;
} }
@Override @Override
@@ -250,10 +242,6 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
PatchSet ps = psUtil.current(rebase.dbProvider.get(), rsrc.getNotes()); PatchSet ps = psUtil.current(rebase.dbProvider.get(), rsrc.getNotes());
if (ps == null) { if (ps == null) {
throw new ResourceConflictException("current revision is missing"); throw new ResourceConflictException("current revision is missing");
} else if (!changeControlFactory
.controlFor(rsrc.getNotes(), rsrc.getUser())
.isVisible(rebase.dbProvider.get())) {
throw new AuthException("current revision not accessible");
} }
return rebase.applyImpl(updateFactory, new RevisionResource(rsrc, ps), input); return rebase.applyImpl(updateFactory, new RevisionResource(rsrc, ps), input);
} }

View File

@@ -24,16 +24,21 @@ import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder; import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
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.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayDeque; import java.util.ArrayDeque;
@@ -55,23 +60,27 @@ import org.eclipse.jgit.revwalk.RevWalk;
@Singleton @Singleton
class RelatedChangesSorter { class RelatedChangesSorter {
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final ProjectControl.GenericFactory projectControlFactory; private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
@Inject @Inject
RelatedChangesSorter( RelatedChangesSorter(
GitRepositoryManager repoManager, ProjectControl.GenericFactory projectControlFactory) { GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider) {
this.repoManager = repoManager; this.repoManager = repoManager;
this.projectControlFactory = projectControlFactory; this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
} }
public List<PatchSetData> sort(List<ChangeData> in, PatchSet startPs, CurrentUser user) public List<PatchSetData> sort(List<ChangeData> in, PatchSet startPs, CurrentUser user)
throws OrmException, IOException, NoSuchProjectException { throws OrmException, IOException, NoSuchProjectException, PermissionBackendException {
checkArgument(!in.isEmpty(), "Input may not be empty"); checkArgument(!in.isEmpty(), "Input may not be empty");
// Map of all patch sets, keyed by commit SHA-1. // Map of all patch sets, keyed by commit SHA-1.
Map<String, PatchSetData> byId = collectById(in); Map<String, PatchSetData> byId = collectById(in);
PatchSetData start = byId.get(startPs.getRevision().get()); PatchSetData start = byId.get(startPs.getRevision().get());
checkArgument(start != null, "%s not found in %s", startPs, in); checkArgument(start != null, "%s not found in %s", startPs, in);
ProjectControl ctl = projectControlFactory.controlFor(start.data().project(), user); PermissionBackend.WithUser perm = permissionBackend.user(user).database(dbProvider);
// Map of patch set -> immediate parent. // Map of patch set -> immediate parent.
ListMultimap<PatchSetData, PatchSetData> parents = ListMultimap<PatchSetData, PatchSetData> parents =
@@ -98,9 +107,9 @@ class RelatedChangesSorter {
} }
} }
Collection<PatchSetData> ancestors = walkAncestors(ctl, parents, start); Collection<PatchSetData> ancestors = walkAncestors(perm, parents, start);
List<PatchSetData> descendants = List<PatchSetData> descendants =
walkDescendants(ctl, children, start, otherPatchSetsOfStart, ancestors); walkDescendants(perm, children, start, otherPatchSetsOfStart, ancestors);
List<PatchSetData> result = new ArrayList<>(ancestors.size() + descendants.size() - 1); List<PatchSetData> result = new ArrayList<>(ancestors.size() + descendants.size() - 1);
result.addAll(Lists.reverse(descendants)); result.addAll(Lists.reverse(descendants));
result.addAll(ancestors); result.addAll(ancestors);
@@ -133,14 +142,16 @@ class RelatedChangesSorter {
} }
private static Collection<PatchSetData> walkAncestors( private static Collection<PatchSetData> walkAncestors(
ProjectControl ctl, ListMultimap<PatchSetData, PatchSetData> parents, PatchSetData start) PermissionBackend.WithUser perm,
throws OrmException { ListMultimap<PatchSetData, PatchSetData> parents,
PatchSetData start)
throws PermissionBackendException {
LinkedHashSet<PatchSetData> result = new LinkedHashSet<>(); LinkedHashSet<PatchSetData> result = new LinkedHashSet<>();
Deque<PatchSetData> pending = new ArrayDeque<>(); Deque<PatchSetData> pending = new ArrayDeque<>();
pending.add(start); pending.add(start);
while (!pending.isEmpty()) { while (!pending.isEmpty()) {
PatchSetData psd = pending.remove(); PatchSetData psd = pending.remove();
if (result.contains(psd) || !isVisible(psd, ctl)) { if (result.contains(psd) || !isVisible(psd, perm)) {
continue; continue;
} }
result.add(psd); result.add(psd);
@@ -150,24 +161,25 @@ class RelatedChangesSorter {
} }
private static List<PatchSetData> walkDescendants( private static List<PatchSetData> walkDescendants(
ProjectControl ctl, PermissionBackend.WithUser perm,
ListMultimap<PatchSetData, PatchSetData> children, ListMultimap<PatchSetData, PatchSetData> children,
PatchSetData start, PatchSetData start,
List<PatchSetData> otherPatchSetsOfStart, List<PatchSetData> otherPatchSetsOfStart,
Iterable<PatchSetData> ancestors) Iterable<PatchSetData> ancestors)
throws OrmException { throws PermissionBackendException {
Set<Change.Id> alreadyEmittedChanges = new HashSet<>(); Set<Change.Id> alreadyEmittedChanges = new HashSet<>();
addAllChangeIds(alreadyEmittedChanges, ancestors); addAllChangeIds(alreadyEmittedChanges, ancestors);
// Prefer descendants found by following the original patch set passed in. // Prefer descendants found by following the original patch set passed in.
List<PatchSetData> result = List<PatchSetData> result =
walkDescendentsImpl(ctl, alreadyEmittedChanges, children, ImmutableList.of(start)); walkDescendentsImpl(perm, alreadyEmittedChanges, children, ImmutableList.of(start));
addAllChangeIds(alreadyEmittedChanges, result); addAllChangeIds(alreadyEmittedChanges, result);
// Then, go back and add new indirect descendants found by following any // Then, go back and add new indirect descendants found by following any
// other patch sets of start. These show up after all direct descendants, // other patch sets of start. These show up after all direct descendants,
// because we wouldn't know where in the walk to insert them. // because we wouldn't know where in the walk to insert them.
result.addAll(walkDescendentsImpl(ctl, alreadyEmittedChanges, children, otherPatchSetsOfStart)); result.addAll(
walkDescendentsImpl(perm, alreadyEmittedChanges, children, otherPatchSetsOfStart));
return result; return result;
} }
@@ -179,11 +191,11 @@ class RelatedChangesSorter {
} }
private static List<PatchSetData> walkDescendentsImpl( private static List<PatchSetData> walkDescendentsImpl(
ProjectControl ctl, PermissionBackend.WithUser perm,
Set<Change.Id> alreadyEmittedChanges, Set<Change.Id> alreadyEmittedChanges,
ListMultimap<PatchSetData, PatchSetData> children, ListMultimap<PatchSetData, PatchSetData> children,
List<PatchSetData> start) List<PatchSetData> start)
throws OrmException { throws PermissionBackendException {
if (start.isEmpty()) { if (start.isEmpty()) {
return ImmutableList.of(); return ImmutableList.of();
} }
@@ -194,7 +206,7 @@ class RelatedChangesSorter {
pending.addAll(start); pending.addAll(start);
while (!pending.isEmpty()) { while (!pending.isEmpty()) {
PatchSetData psd = pending.remove(); PatchSetData psd = pending.remove();
if (seen.contains(psd) || !isVisible(psd, ctl)) { if (seen.contains(psd) || !isVisible(psd, perm)) {
continue; continue;
} }
seen.add(psd); seen.add(psd);
@@ -225,10 +237,14 @@ class RelatedChangesSorter {
return result; return result;
} }
private static boolean isVisible(PatchSetData psd, ProjectControl ctl) throws OrmException { private static boolean isVisible(PatchSetData psd, PermissionBackend.WithUser perm)
// Reuse existing project control rather than lazily creating a new one for throws PermissionBackendException {
// each ChangeData. try {
return ctl.controlFor(psd.data().notes()).isPatchVisible(psd.patchSet(), psd.data()); perm.change(psd.data()).check(ChangePermission.READ);
return true;
} catch (AuthException e) {
return false;
}
} }
@AutoValue @AutoValue

View File

@@ -28,7 +28,9 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.edit.ChangeEdit; import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -46,7 +48,7 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final ChangeEditUtil editUtil; private final ChangeEditUtil editUtil;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final ChangeControl.GenericFactory changeControlFactory; private final PermissionBackend permissionBackend;
@Inject @Inject
Revisions( Revisions(
@@ -54,12 +56,12 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
ChangeEditUtil editUtil, ChangeEditUtil editUtil,
PatchSetUtil psUtil, PatchSetUtil psUtil,
ChangeControl.GenericFactory changeControlFactory) { PermissionBackend permissionBackend) {
this.views = views; this.views = views;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.editUtil = editUtil; this.editUtil = editUtil;
this.psUtil = psUtil; this.psUtil = psUtil;
this.changeControlFactory = changeControlFactory; this.permissionBackend = permissionBackend;
} }
@Override @Override
@@ -74,7 +76,8 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
@Override @Override
public RevisionResource parse(ChangeResource change, IdString id) public RevisionResource parse(ChangeResource change, IdString id)
throws ResourceNotFoundException, AuthException, OrmException, IOException { throws ResourceNotFoundException, AuthException, OrmException, IOException,
PermissionBackendException {
if (id.get().equals("current")) { if (id.get().equals("current")) {
PatchSet ps = psUtil.current(dbProvider.get(), change.getNotes()); PatchSet ps = psUtil.current(dbProvider.get(), change.getNotes());
if (ps != null && visible(change)) { if (ps != null && visible(change)) {
@@ -100,10 +103,17 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
} }
} }
private boolean visible(ChangeResource change) throws OrmException { private boolean visible(ChangeResource change) throws PermissionBackendException {
return changeControlFactory try {
.controlFor(change.getNotes(), change.getUser()) permissionBackend
.isVisible(dbProvider.get()); .user(change.getUser())
.change(change.getNotes())
.database(dbProvider)
.check(ChangePermission.READ);
return true;
} catch (AuthException e) {
return false;
}
} }
private List<RevisionResource> find(ChangeResource change, String id) private List<RevisionResource> find(ChangeResource change, String id)

View File

@@ -55,7 +55,6 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -507,20 +506,17 @@ public class Submit
private final Submit submit; private final Submit submit;
private final ChangeJson.Factory json; private final ChangeJson.Factory json;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final ChangeControl.GenericFactory changeControlFactory;
@Inject @Inject
CurrentRevision( CurrentRevision(
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
Submit submit, Submit submit,
ChangeJson.Factory json, ChangeJson.Factory json,
PatchSetUtil psUtil, PatchSetUtil psUtil) {
ChangeControl.GenericFactory changeControlFactory) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.submit = submit; this.submit = submit;
this.json = json; this.json = json;
this.psUtil = psUtil; this.psUtil = psUtil;
this.changeControlFactory = changeControlFactory;
} }
@Override @Override
@@ -530,10 +526,6 @@ public class Submit
PatchSet ps = psUtil.current(dbProvider.get(), rsrc.getNotes()); PatchSet ps = psUtil.current(dbProvider.get(), rsrc.getNotes());
if (ps == null) { if (ps == null) {
throw new ResourceConflictException("current revision is missing"); throw new ResourceConflictException("current revision is missing");
} else if (!changeControlFactory
.controlFor(rsrc.getNotes(), rsrc.getUser())
.isVisible(dbProvider.get())) {
throw new AuthException("current revision not accessible");
} }
Output out = submit.apply(new RevisionResource(rsrc, ps), input); Output out = submit.apply(new RevisionResource(rsrc, ps), input);

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -73,7 +74,11 @@ public class ChangeAbandoned {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -67,7 +68,11 @@ public class ChangeMerged {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -66,7 +67,11 @@ public class ChangeRestored {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -77,7 +78,11 @@ public class CommentAdded {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.change.ChangeJson; import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackendException;
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.inject.Inject; import com.google.inject.Inject;
@@ -80,12 +81,14 @@ public class EventUtil {
} }
public RevisionInfo revisionInfo(Project project, PatchSet ps) public RevisionInfo revisionInfo(Project project, PatchSet ps)
throws OrmException, PatchListNotAvailableException, GpgException, IOException { throws OrmException, PatchListNotAvailableException, GpgException, IOException,
PermissionBackendException {
return revisionInfo(project.getNameKey(), ps); return revisionInfo(project.getNameKey(), ps);
} }
public RevisionInfo revisionInfo(Project.NameKey project, PatchSet ps) public RevisionInfo revisionInfo(Project.NameKey project, PatchSet ps)
throws OrmException, PatchListNotAvailableException, GpgException, IOException { throws OrmException, PatchListNotAvailableException, GpgException, IOException,
PermissionBackendException {
ChangeData cd = changeDataFactory.create(db.get(), project, ps.getId().getParentKey()); ChangeData cd = changeDataFactory.create(db.get(), project, ps.getId().getParentKey());
return changeJson.getRevisionInfo(cd, ps); return changeJson.getRevisionInfo(cd, ps);
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -70,7 +71,11 @@ public class ReviewerAdded {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -81,7 +82,11 @@ public class ReviewerDeleted {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -67,7 +68,11 @@ public class RevisionCreated {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.GpgException; import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException; import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
@@ -81,7 +82,11 @@ public class VoteDeleted {
} }
} catch (PatchListObjectTooLargeException e) { } catch (PatchListObjectTooLargeException e) {
log.warn("Couldn't fire event: " + e.getMessage()); log.warn("Couldn't fire event: " + e.getMessage());
} catch (PatchListNotAvailableException | GpgException | IOException | OrmException e) { } catch (PatchListNotAvailableException
| GpgException
| IOException
| OrmException
| PermissionBackendException e) {
log.error("Couldn't fire event", e); log.error("Couldn't fire event", e);
} }
} }

View File

@@ -38,9 +38,7 @@ import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -106,8 +104,6 @@ public class MergeSuperSet {
private final Map<QueryKey, List<ChangeData>> queryCache; private final Map<QueryKey, List<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads; private final Map<Branch.NameKey, Optional<RevCommit>> heads;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final ProjectCache projectCache;
private MergeOpRepoManager orm; private MergeOpRepoManager orm;
private boolean closeOrm; private boolean closeOrm;
@@ -119,17 +115,13 @@ public class MergeSuperSet {
Provider<InternalChangeQuery> queryProvider, Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider, Provider<MergeOpRepoManager> repoManagerProvider,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory, SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
ChangeControl.GenericFactory changeControlFactory,
ProjectCache projectCache) {
this.cfg = cfg; this.cfg = cfg;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider; this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider; this.repoManagerProvider = repoManagerProvider;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
this.changeControlFactory = changeControlFactory;
this.projectCache = projectCache;
queryCache = new HashMap<>(); queryCache = new HashMap<>();
heads = new HashMap<>(); heads = new HashMap<>();
} }
@@ -160,7 +152,7 @@ public class MergeSuperSet {
} }
} }
private SubmitType submitType(CurrentUser user, ChangeData cd, PatchSet ps, boolean visible) private SubmitType submitType(CurrentUser user, ChangeData cd, PatchSet ps)
throws OrmException, IOException { throws OrmException, IOException {
// Submit type prolog rules mean that the submit type can depend on the // Submit type prolog rules mean that the submit type can depend on the
// submitting user and the content of the change. // submitting user and the content of the change.
@@ -172,10 +164,6 @@ public class MergeSuperSet {
// doesn't match that, we may pick the wrong submit type and produce a // doesn't match that, we may pick the wrong submit type and produce a
// misleading (but still nonzero) count of the non visible changes that // misleading (but still nonzero) count of the non visible changes that
// would be submitted together with the visible ones. // would be submitted together with the visible ones.
if (!visible) {
return projectCache.checkedGet(cd.project()).getProject().getSubmitType();
}
SubmitTypeRecord str = SubmitTypeRecord str =
ps == cd.currentPatchSet() ps == cd.currentPatchSet()
? cd.submitTypeRecord() ? cd.submitTypeRecord()
@@ -247,18 +235,7 @@ public class MergeSuperSet {
// is visible, we use the most recent one. Otherwise, use the current // is visible, we use the most recent one. Otherwise, use the current
// patch set. // patch set.
PatchSet ps = cd.currentPatchSet(); PatchSet ps = cd.currentPatchSet();
boolean visiblePatchSet = visible; if (submitType(user, cd, ps) == SubmitType.CHERRY_PICK) {
ChangeControl ctl = changeControlFactory.controlFor(cd.notes(), user);
if (!ctl.isPatchVisible(ps, cd)) {
Iterable<PatchSet> visiblePatchSets = ctl.getVisiblePatchSets(cd.patchSets(), db);
if (Iterables.isEmpty(visiblePatchSets)) {
visiblePatchSet = false;
} else {
ps = Iterables.getLast(visiblePatchSets);
}
}
if (submitType(user, cd, ps, visiblePatchSet) == SubmitType.CHERRY_PICK) {
if (visible) { if (visible) {
visibleChanges.add(cd); visibleChanges.add(cd);
} else { } else {

View File

@@ -50,7 +50,6 @@ import com.google.gerrit.server.git.SubmoduleOp;
import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.validators.OnSubmitValidators; import com.google.gerrit.server.git.validators.OnSubmitValidators;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -107,7 +106,6 @@ public abstract class SubmitStrategy {
final AccountCache accountCache; final AccountCache accountCache;
final ApprovalsUtil approvalsUtil; final ApprovalsUtil approvalsUtil;
final ChangeControl.GenericFactory changeControlFactory;
final ChangeMerged changeMerged; final ChangeMerged changeMerged;
final ChangeMessagesUtil cmUtil; final ChangeMessagesUtil cmUtil;
final EmailMerge.Factory mergedSenderFactory; final EmailMerge.Factory mergedSenderFactory;
@@ -146,7 +144,6 @@ public abstract class SubmitStrategy {
Arguments( Arguments(
AccountCache accountCache, AccountCache accountCache,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
ChangeControl.GenericFactory changeControlFactory,
ChangeMerged changeMerged, ChangeMerged changeMerged,
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
EmailMerge.Factory mergedSenderFactory, EmailMerge.Factory mergedSenderFactory,
@@ -178,7 +175,6 @@ public abstract class SubmitStrategy {
@Assisted boolean dryrun) { @Assisted boolean dryrun) {
this.accountCache = accountCache; this.accountCache = accountCache;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.changeControlFactory = changeControlFactory;
this.changeMerged = changeMerged; this.changeMerged = changeMerged;
this.mergedSenderFactory = mergedSenderFactory; this.mergedSenderFactory = mergedSenderFactory;
this.repoManager = repoManager; this.repoManager = repoManager;

View File

@@ -37,7 +37,9 @@ import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException; import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
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.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -90,7 +92,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
private final DiffPreferencesInfo diffPrefs; private final DiffPreferencesInfo diffPrefs;
private final ChangeEditUtil editReader; private final ChangeEditUtil editReader;
private final Provider<CurrentUser> userProvider; private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControlFactory; private final PermissionBackend permissionBackend;
private Optional<ChangeEdit> edit; private Optional<ChangeEdit> edit;
private final Change.Id changeId; private final Change.Id changeId;
@@ -113,7 +115,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
CommentsUtil commentsUtil, CommentsUtil commentsUtil,
ChangeEditUtil editReader, ChangeEditUtil editReader,
Provider<CurrentUser> userProvider, Provider<CurrentUser> userProvider,
ChangeControl.GenericFactory changeControlFactory, PermissionBackend permissionBackend,
@Assisted ChangeNotes notes, @Assisted ChangeNotes notes,
@Assisted String fileName, @Assisted String fileName,
@Assisted("patchSetA") @Nullable PatchSet.Id patchSetA, @Assisted("patchSetA") @Nullable PatchSet.Id patchSetA,
@@ -128,7 +130,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
this.commentsUtil = commentsUtil; this.commentsUtil = commentsUtil;
this.editReader = editReader; this.editReader = editReader;
this.userProvider = userProvider; this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory; this.permissionBackend = permissionBackend;
this.fileName = fileName; this.fileName = fileName;
this.psa = patchSetA; this.psa = patchSetA;
@@ -149,7 +151,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
CommentsUtil commentsUtil, CommentsUtil commentsUtil,
ChangeEditUtil editReader, ChangeEditUtil editReader,
Provider<CurrentUser> userProvider, Provider<CurrentUser> userProvider,
ChangeControl.GenericFactory changeControlFactory, PermissionBackend permissionBackend,
@Assisted ChangeNotes notes, @Assisted ChangeNotes notes,
@Assisted String fileName, @Assisted String fileName,
@Assisted int parentNum, @Assisted int parentNum,
@@ -164,7 +166,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
this.commentsUtil = commentsUtil; this.commentsUtil = commentsUtil;
this.editReader = editReader; this.editReader = editReader;
this.userProvider = userProvider; this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory; this.permissionBackend = permissionBackend;
this.fileName = fileName; this.fileName = fileName;
this.psa = null; this.psa = null;
@@ -187,7 +189,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
@Override @Override
public PatchScript call() public PatchScript call()
throws OrmException, LargeObjectException, AuthException, InvalidChangeOperationException, throws OrmException, LargeObjectException, AuthException, InvalidChangeOperationException,
IOException { IOException, PermissionBackendException {
if (parentNum < 0) { if (parentNum < 0) {
validatePatchSetId(psa); validatePatchSetId(psa);
} }
@@ -195,10 +197,16 @@ public class PatchScriptFactory implements Callable<PatchScript> {
PatchSet psEntityA = psa != null ? psUtil.get(db, notes, psa) : null; PatchSet psEntityA = psa != null ? psUtil.get(db, notes, psa) : null;
PatchSet psEntityB = psb.get() == 0 ? new PatchSet(psb) : psUtil.get(db, notes, psb); PatchSet psEntityB = psb.get() == 0 ? new PatchSet(psb) : psUtil.get(db, notes, psb);
if (psEntityA != null || psEntityB != null) {
ChangeControl ctl = changeControlFactory.controlFor(notes, userProvider.get()); try {
if ((psEntityA != null && !ctl.isVisible(db)) || (psEntityB != null && !ctl.isVisible(db))) { permissionBackend
throw new NoSuchChangeException(changeId); .user(userProvider)
.change(notes)
.database(db)
.check(ChangePermission.READ);
} catch (AuthException e) {
throw new NoSuchChangeException(changeId);
}
} }
try (Repository git = repoManager.openRepository(notes.getProjectName())) { try (Repository git = repoManager.openRepository(notes.getProjectName())) {
@@ -212,8 +220,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
final PatchScriptBuilder b = newBuilder(list, git); final PatchScriptBuilder b = newBuilder(list, git);
final PatchListEntry content = list.get(fileName); final PatchListEntry content = list.get(fileName);
loadCommentsAndHistory( loadCommentsAndHistory(content.getChangeType(), content.getOldName(), content.getNewName());
ctl, content.getChangeType(), content.getOldName(), content.getNewName());
return b.toPatchScript(content, comments, history); return b.toPatchScript(content, comments, history);
} catch (PatchListNotAvailableException e) { } catch (PatchListNotAvailableException e) {
@@ -285,8 +292,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
} }
} }
private void loadCommentsAndHistory( private void loadCommentsAndHistory(ChangeType changeType, String oldName, String newName)
ChangeControl ctl, ChangeType changeType, String oldName, String newName)
throws OrmException { throws OrmException {
Map<Patch.Key, Patch> byKey = new HashMap<>(); Map<Patch.Key, Patch> byKey = new HashMap<>();
@@ -298,9 +304,6 @@ public class PatchScriptFactory implements Callable<PatchScript> {
// //
history = new ArrayList<>(); history = new ArrayList<>();
for (PatchSet ps : psUtil.byChange(db, notes)) { for (PatchSet ps : psUtil.byChange(db, notes)) {
if (!ctl.isVisible(db)) {
continue;
}
String name = fileName; String name = fileName;
if (psa != null) { if (psa != null) {
switch (changeType) { switch (changeType) {

View File

@@ -14,10 +14,8 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF; import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
@@ -27,7 +25,6 @@ import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
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;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -46,65 +43,15 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection; import java.util.Collection;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.function.Predicate;
/** Access control management for a user accessing a single change. */ /** Access control management for a user accessing a single change. */
public class ChangeControl { class ChangeControl {
@Singleton @Singleton
public static class GenericFactory { static class Factory {
private final ProjectControl.GenericFactory projectControl;
private final ChangeNotes.Factory notesFactory;
@Inject
GenericFactory(ProjectControl.GenericFactory p, ChangeNotes.Factory n) {
projectControl = p;
notesFactory = n;
}
public ChangeControl controlFor(
ReviewDb db, Project.NameKey project, Change.Id changeId, CurrentUser user)
throws OrmException {
return controlFor(notesFactory.create(db, project, changeId), user);
}
public ChangeControl controlFor(ReviewDb db, Change change, CurrentUser user)
throws OrmException {
final Project.NameKey projectKey = change.getProject();
try {
return projectControl.controlFor(projectKey, user).controlFor(db, change);
} catch (NoSuchProjectException e) {
throw new NoSuchChangeException(change.getId(), e);
} catch (IOException e) {
// TODO: propagate this exception
throw new NoSuchChangeException(change.getId(), e);
}
}
public ChangeControl controlFor(ChangeNotes notes, CurrentUser user)
throws NoSuchChangeException {
try {
return projectControl.controlFor(notes.getProjectName(), user).controlFor(notes);
} catch (NoSuchProjectException | IOException e) {
throw new NoSuchChangeException(notes.getChangeId(), e);
}
}
public ChangeControl validateFor(Change.Id changeId, CurrentUser user) throws OrmException {
return validateFor(notesFactory.createChecked(changeId), user);
}
public ChangeControl validateFor(ChangeNotes notes, CurrentUser user) throws OrmException {
return controlFor(notes, user);
}
}
@Singleton
public static class Factory {
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
@@ -152,7 +99,7 @@ public class ChangeControl {
this.patchSetUtil = patchSetUtil; this.patchSetUtil = patchSetUtil;
} }
public ChangeControl forUser(CurrentUser who) { ChangeControl forUser(CurrentUser who) {
if (getUser().equals(who)) { if (getUser().equals(who)) {
return this; return this;
} }
@@ -160,39 +107,26 @@ public class ChangeControl {
changeDataFactory, approvalsUtil, getRefControl().forUser(who), notes, patchSetUtil); changeDataFactory, approvalsUtil, getRefControl().forUser(who), notes, patchSetUtil);
} }
public RefControl getRefControl() { private RefControl getRefControl() {
return refControl; return refControl;
} }
public CurrentUser getUser() { private CurrentUser getUser() {
return getRefControl().getUser(); return getRefControl().getUser();
} }
public ProjectControl getProjectControl() { private ProjectControl getProjectControl() {
return getRefControl().getProjectControl(); return getRefControl().getProjectControl();
} }
public Project getProject() { private Change getChange() {
return getProjectControl().getProject();
}
public Change.Id getId() {
return notes.getChangeId();
}
public Change getChange() {
return notes.getChange(); return notes.getChange();
} }
public ChangeNotes getNotes() { private ChangeNotes getNotes() {
return notes; return notes;
} }
/** Can this user see this change? */
public boolean isVisible(ReviewDb db) throws OrmException {
return isVisible(db, null);
}
/** Can this user see this change? */ /** Can this user see this change? */
private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException { private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
if (getChange().isPrivate() && !isPrivateVisible(db, cd)) { if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
@@ -202,36 +136,10 @@ public class ChangeControl {
} }
/** Can the user see this change? Does not account for draft status */ /** Can the user see this change? Does not account for draft status */
public boolean isRefVisible() { private boolean isRefVisible() {
return getRefControl().isVisible(); return getRefControl().isVisible();
} }
/** Can this user see the given patchset? */
public boolean isPatchVisible(PatchSet ps, ChangeData cd) throws OrmException {
// TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed
checkArgument(
cd.getId().equals(ps.getId().getParentKey()), "%s not for change %s", ps, cd.getId());
return isVisible(cd.db());
}
/**
* @return patches for the change visible to the current user.
* @throws OrmException an error occurred reading the database.
*/
public Collection<PatchSet> getVisiblePatchSets(Collection<PatchSet> patchSets, ReviewDb db)
throws OrmException {
// TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed
Predicate<? super PatchSet> predicate =
ps -> {
try {
return isVisible(db);
} catch (OrmException e) {
return false;
}
};
return patchSets.stream().filter(predicate).collect(toList());
}
/** Can this user abandon this change? */ /** Can this user abandon this change? */
private boolean canAbandon(ReviewDb db) throws OrmException { private boolean canAbandon(ReviewDb db) throws OrmException {
return (isOwner() // owner (aka creator) of the change can abandon return (isOwner() // owner (aka creator) of the change can abandon
@@ -243,7 +151,7 @@ public class ChangeControl {
} }
/** Can this user delete this change? */ /** Can this user delete this change? */
public boolean canDelete(Change.Status status) { private boolean canDelete(Change.Status status) {
switch (status) { switch (status) {
case NEW: case NEW:
case ABANDONED: case ABANDONED:
@@ -285,7 +193,7 @@ public class ChangeControl {
} }
/** Is the current patch set locked against state changes? */ /** Is the current patch set locked against state changes? */
boolean isPatchSetLocked(ReviewDb db) throws OrmException { private boolean isPatchSetLocked(ReviewDb db) throws OrmException {
if (getChange().getStatus() == Change.Status.MERGED) { if (getChange().getStatus() == Change.Status.MERGED) {
return false; return false;
} }

View File

@@ -32,7 +32,6 @@ public class DefaultPermissionBackendModule extends AbstractModule {
// TODO(sop) Hide ProjectControl, RefControl, ChangeControl related bindings. // TODO(sop) Hide ProjectControl, RefControl, ChangeControl related bindings.
bind(ProjectControl.GenericFactory.class); bind(ProjectControl.GenericFactory.class);
factory(ProjectControl.AssistedFactory.class); factory(ProjectControl.AssistedFactory.class);
bind(ChangeControl.GenericFactory.class);
bind(ChangeControl.Factory.class); bind(ChangeControl.Factory.class);
} }
} }

View File

@@ -29,27 +29,29 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
@Singleton @Singleton
public class RemoveReviewerControl { public class RemoveReviewerControl {
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final ChangeControl.GenericFactory changeControlFactory; private final ProjectControl.GenericFactory projectControlFactory;
@Inject @Inject
RemoveReviewerControl( RemoveReviewerControl(
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
ChangeControl.GenericFactory changeControlFactory) { ProjectControl.GenericFactory projectControlFactory) {
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.changeControlFactory = changeControlFactory; this.projectControlFactory = projectControlFactory;
} }
/** @throws AuthException if this user is not allowed to remove this approval. */ /** @throws AuthException if this user is not allowed to remove this approval. */
public void checkRemoveReviewer( public void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval) ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
throws PermissionBackendException, AuthException, NoSuchChangeException, OrmException { throws PermissionBackendException, AuthException, NoSuchProjectException, OrmException,
IOException {
if (canRemoveReviewerWithoutPermissionCheck( if (canRemoveReviewerWithoutPermissionCheck(
notes.getChange(), currentUser, approval.getAccountId(), approval.getValue())) { notes.getChange(), currentUser, approval.getAccountId(), approval.getValue())) {
return; return;
@@ -65,7 +67,7 @@ public class RemoveReviewerControl {
/** @return true if the user is allowed to remove this reviewer. */ /** @return true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer( public boolean testRemoveReviewer(
ChangeData cd, CurrentUser currentUser, Account.Id reviewer, int value) ChangeData cd, CurrentUser currentUser, Account.Id reviewer, int value)
throws PermissionBackendException, NoSuchChangeException, OrmException { throws PermissionBackendException, NoSuchProjectException, OrmException, IOException {
if (canRemoveReviewerWithoutPermissionCheck(cd.change(), currentUser, reviewer, value)) { if (canRemoveReviewerWithoutPermissionCheck(cd.change(), currentUser, reviewer, value)) {
return true; return true;
} }
@@ -78,7 +80,7 @@ public class RemoveReviewerControl {
private boolean canRemoveReviewerWithoutPermissionCheck( private boolean canRemoveReviewerWithoutPermissionCheck(
Change change, CurrentUser currentUser, Account.Id reviewer, int value) Change change, CurrentUser currentUser, Account.Id reviewer, int value)
throws NoSuchChangeException, OrmException { throws NoSuchProjectException, OrmException, IOException {
if (!change.getStatus().isOpen()) { if (!change.getStatus().isOpen()) {
return false; return false;
} }
@@ -94,11 +96,10 @@ public class RemoveReviewerControl {
// Users with the remove reviewer permission, the branch owner, project // Users with the remove reviewer permission, the branch owner, project
// owner and site admin can remove anyone // owner and site admin can remove anyone
ChangeControl changeControl = ProjectControl ctl = projectControlFactory.controlFor(change.getProject(), currentUser);
changeControlFactory.controlFor(dbProvider.get(), change, currentUser); if (ctl.controlForRef(change.getDest()).isOwner() // branch owner
if (changeControl.getRefControl().isOwner() // branch owner || ctl.isOwner() // project owner
|| changeControl.getProjectControl().isOwner() // project owner || ctl.isAdmin()) { // project admin
|| changeControl.getProjectControl().isAdmin()) { // project admin
return true; return true;
} }
return false; return false;

View File

@@ -72,7 +72,6 @@ import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey; import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
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.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
@@ -326,7 +325,7 @@ public class ChangeData {
ChangeData cd = ChangeData cd =
new ChangeData( new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, project, id, null, null); null, null, null, null, project, id, null, null);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId)); cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd; return cd;
} }
@@ -349,7 +348,6 @@ public class ChangeData {
private final TrackingFooters trackingFooters; private final TrackingFooters trackingFooters;
private final GetPureRevert pureRevert; private final GetPureRevert pureRevert;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private final ChangeControl.GenericFactory changeControlFactory;
// Required assisted injected fields. // Required assisted injected fields.
private final ReviewDb db; private final ReviewDb db;
@@ -375,7 +373,6 @@ public class ChangeData {
private Collection<Comment> publishedComments; private Collection<Comment> publishedComments;
private Collection<RobotComment> robotComments; private Collection<RobotComment> robotComments;
private CurrentUser visibleTo; private CurrentUser visibleTo;
private ChangeControl changeControl;
private List<ChangeMessage> messages; private List<ChangeMessage> messages;
private Optional<ChangedLines> changedLines; private Optional<ChangedLines> changedLines;
private SubmitTypeRecord submitTypeRecord; private SubmitTypeRecord submitTypeRecord;
@@ -420,7 +417,6 @@ public class ChangeData {
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
GetPureRevert pureRevert, GetPureRevert pureRevert,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory, SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
ChangeControl.GenericFactory changeControlFactory,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@Assisted Project.NameKey project, @Assisted Project.NameKey project,
@Assisted Change.Id id, @Assisted Change.Id id,
@@ -443,7 +439,6 @@ public class ChangeData {
this.trackingFooters = trackingFooters; this.trackingFooters = trackingFooters;
this.pureRevert = pureRevert; this.pureRevert = pureRevert;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
this.changeControlFactory = changeControlFactory;
// May be null in tests when created via createForTest above, in which case lazy-loading will // May be null in tests when created via createForTest above, in which case lazy-loading will
// intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers // intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers
@@ -554,22 +549,8 @@ public class ChangeData {
return visibleTo == user; return visibleTo == user;
} }
private ChangeControl changeControl() throws OrmException { void cacheVisibleTo(CurrentUser user) {
// TODO(hiesel): Remove this method. visibleTo = user;
if (changeControl == null) {
Change c = change();
try {
changeControl = changeControlFactory.controlFor(db, c, userFactory.create(c.getOwner()));
} catch (NoSuchChangeException e) {
throw new OrmException(e);
}
}
return changeControl;
}
void cacheVisibleTo(ChangeControl ctl) {
visibleTo = ctl.getUser();
changeControl = ctl;
} }
public Change change() throws OrmException { public Change change() throws OrmException {
@@ -980,15 +961,8 @@ public class ChangeData {
return null; return null;
} }
PatchSet ps = currentPatchSet(); PatchSet ps = currentPatchSet();
try { if (ps == null) {
if (ps == null || !changeControl().isVisible(db)) { return null;
return null;
}
} catch (OrmException e) {
if (e.getCause() instanceof NoSuchChangeException) {
return null;
}
throw e;
} }
try (Repository repo = repoManager.openRepository(project())) { try (Repository repo = repoManager.openRepository(project())) {

View File

@@ -23,28 +23,23 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider; import com.google.inject.Provider;
public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData> { public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData> {
protected final Provider<ReviewDb> db; protected final Provider<ReviewDb> db;
protected final ChangeNotes.Factory notesFactory; protected final ChangeNotes.Factory notesFactory;
protected final ChangeControl.GenericFactory changeControlFactory;
protected final CurrentUser user; protected final CurrentUser user;
protected final PermissionBackend permissionBackend; protected final PermissionBackend permissionBackend;
public ChangeIsVisibleToPredicate( public ChangeIsVisibleToPredicate(
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ChangeControl.GenericFactory changeControlFactory,
CurrentUser user, CurrentUser user,
PermissionBackend permissionBackend) { PermissionBackend permissionBackend) {
super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user)); super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user));
this.db = db; this.db = db;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.changeControlFactory = changeControlFactory;
this.user = user; this.user = user;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
} }
@@ -59,15 +54,7 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
return false; return false;
} }
ChangeControl changeControl;
ChangeNotes notes = notesFactory.createFromIndexedChange(change); ChangeNotes notes = notesFactory.createFromIndexedChange(change);
try {
changeControl = changeControlFactory.controlFor(notes, user);
} catch (NoSuchChangeException e) {
// Ignored
return false;
}
boolean visible; boolean visible;
try { try {
visible = visible =
@@ -80,7 +67,7 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
throw new OrmException("unable to check permissions", e); throw new OrmException("unable to check permissions", e);
} }
if (visible) { if (visible) {
cd.cacheVisibleTo(changeControl); cd.cacheVisibleTo(user);
return true; return true;
} }
return false; return false;

View File

@@ -70,7 +70,6 @@ import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ListChildProjects; import com.google.gerrit.server.project.ListChildProjects;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -196,7 +195,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
final AllProjectsName allProjectsName; final AllProjectsName allProjectsName;
final AllUsersName allUsersName; final AllUsersName allUsersName;
final PermissionBackend permissionBackend; final PermissionBackend permissionBackend;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory; final ChangeData.Factory changeDataFactory;
final ChangeIndex index; final ChangeIndex index;
final ChangeIndexRewriter rewriter; final ChangeIndexRewriter rewriter;
@@ -233,7 +231,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self, Provider<CurrentUser> self,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
CommentsUtil commentsUtil, CommentsUtil commentsUtil,
@@ -263,7 +260,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
userFactory, userFactory,
self, self,
permissionBackend, permissionBackend,
changeControlGenericFactory,
notesFactory, notesFactory,
changeDataFactory, changeDataFactory,
commentsUtil, commentsUtil,
@@ -295,7 +291,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self, Provider<CurrentUser> self,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
CommentsUtil commentsUtil, CommentsUtil commentsUtil,
@@ -324,7 +319,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.self = self; this.self = self;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.commentsUtil = commentsUtil; this.commentsUtil = commentsUtil;
this.accountResolver = accountResolver; this.accountResolver = accountResolver;
@@ -357,7 +351,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
userFactory, userFactory,
Providers.of(otherUser), Providers.of(otherUser),
permissionBackend, permissionBackend,
changeControlGenericFactory,
notesFactory, notesFactory,
changeDataFactory, changeDataFactory,
commentsUtil, commentsUtil,
@@ -926,8 +919,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
} }
public Predicate<ChangeData> visibleto(CurrentUser user) { public Predicate<ChangeData> visibleto(CurrentUser user) {
return new ChangeIsVisibleToPredicate( return new ChangeIsVisibleToPredicate(args.db, args.notesFactory, user, args.permissionBackend);
args.db, args.notesFactory, args.changeControlGenericFactory, user, args.permissionBackend);
} }
public Predicate<ChangeData> is_visible() throws QueryParseException { public Predicate<ChangeData> is_visible() throws QueryParseException {

View File

@@ -34,7 +34,6 @@ import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.index.change.IndexedChangeQuery; import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import java.util.ArrayList; import java.util.ArrayList;
@@ -61,7 +60,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final Provider<CurrentUser> userProvider; private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final DynamicMap<ChangeAttributeFactory> attributeFactories; private final DynamicMap<ChangeAttributeFactory> attributeFactories;
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
@@ -82,7 +80,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
ChangeIndexCollection indexes, ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter, ChangeIndexRewriter rewriter,
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
DynamicMap<ChangeAttributeFactory> attributeFactories, DynamicMap<ChangeAttributeFactory> attributeFactories,
PermissionBackend permissionBackend) { PermissionBackend permissionBackend) {
@@ -96,7 +93,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
() -> limitsFactory.create(userProvider.get()).getQueryLimit()); () -> limitsFactory.create(userProvider.get()).getQueryLimit());
this.db = db; this.db = db;
this.userProvider = userProvider; this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.attributeFactories = attributeFactories; this.attributeFactories = attributeFactories;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
@@ -142,8 +138,7 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) { protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) {
return new AndChangeSource( return new AndChangeSource(
pred, pred,
new ChangeIsVisibleToPredicate( new ChangeIsVisibleToPredicate(db, notesFactory, userProvider.get(), permissionBackend),
db, notesFactory, changeControlFactory, userProvider.get(), permissionBackend),
start); start);
} }
} }

View File

@@ -24,7 +24,6 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.LabelVote; import com.google.gerrit.server.util.LabelVote;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -38,7 +37,6 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
protected static class Args { protected static class Args {
protected final ProjectCache projectCache; protected final ProjectCache projectCache;
protected final PermissionBackend permissionBackend; protected final PermissionBackend permissionBackend;
protected final ChangeControl.GenericFactory ccFactory;
protected final IdentifiedUser.GenericFactory userFactory; protected final IdentifiedUser.GenericFactory userFactory;
protected final Provider<ReviewDb> dbProvider; protected final Provider<ReviewDb> dbProvider;
protected final String value; protected final String value;
@@ -48,7 +46,6 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
protected Args( protected Args(
ProjectCache projectCache, ProjectCache projectCache,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
String value, String value,
@@ -56,7 +53,6 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
AccountGroup.UUID group) { AccountGroup.UUID group) {
this.projectCache = projectCache; this.projectCache = projectCache;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.ccFactory = ccFactory;
this.userFactory = userFactory; this.userFactory = userFactory;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.value = value; this.value = value;
@@ -87,14 +83,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
super( super(
predicates( predicates(
new Args( new Args(
a.projectCache, a.projectCache, a.permissionBackend, a.userFactory, a.db, value, accounts, group)));
a.permissionBackend,
a.changeControlGenericFactory,
a.userFactory,
a.db,
value,
accounts,
group)));
this.value = value; this.value = value;
} }

View File

@@ -31,7 +31,6 @@ import com.google.gerrit.server.data.PatchSetAttribute;
import com.google.gerrit.server.data.QueryStatsAttribute; import com.google.gerrit.server.data.QueryStatsAttribute;
import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.events.EventFactory;
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.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gson.Gson; import com.google.gson.Gson;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -84,7 +83,6 @@ public class OutputStreamQuery {
private final EventFactory eventFactory; private final EventFactory eventFactory;
private final TrackingFooters trackingFooters; private final TrackingFooters trackingFooters;
private final CurrentUser user; private final CurrentUser user;
private final ChangeControl.GenericFactory changeControlFactory;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private OutputFormat outputFormat = OutputFormat.TEXT; private OutputFormat outputFormat = OutputFormat.TEXT;
@@ -110,7 +108,6 @@ public class OutputStreamQuery {
EventFactory eventFactory, EventFactory eventFactory,
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
CurrentUser user, CurrentUser user,
ChangeControl.GenericFactory changeControlFactory,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) { SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db; this.db = db;
this.repoManager = repoManager; this.repoManager = repoManager;
@@ -119,7 +116,6 @@ public class OutputStreamQuery {
this.eventFactory = eventFactory; this.eventFactory = eventFactory;
this.trackingFooters = trackingFooters; this.trackingFooters = trackingFooters;
this.user = user; this.user = user;
this.changeControlFactory = changeControlFactory;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
} }
@@ -278,13 +274,12 @@ public class OutputStreamQuery {
} }
} }
ChangeControl ctl = changeControlFactory.controlFor(db, d.change(), user);
if (includePatchSets) { if (includePatchSets) {
eventFactory.addPatchSets( eventFactory.addPatchSets(
db, db,
rw, rw,
c, c,
ctl.getVisiblePatchSets(d.patchSets(), db), d.patchSets(),
includeApprovals ? d.approvals().asMap() : null, includeApprovals ? d.approvals().asMap() : null,
includeFiles, includeFiles,
d.change(), d.change(),
@@ -293,7 +288,7 @@ public class OutputStreamQuery {
if (includeCurrentPatchSet) { if (includeCurrentPatchSet) {
PatchSet current = d.currentPatchSet(); PatchSet current = d.currentPatchSet();
if (current != null && ctl.isVisible(d.db())) { if (current != null) {
c.currentPatchSet = eventFactory.asPatchSetAttribute(db, rw, d.change(), current); c.currentPatchSet = eventFactory.asPatchSetAttribute(db, rw, d.change(), current);
eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes); eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes);
@@ -313,7 +308,7 @@ public class OutputStreamQuery {
db, db,
rw, rw,
c, c,
ctl.getVisiblePatchSets(d.patchSets(), db), d.patchSets(),
includeApprovals ? d.approvals().asMap() : null, includeApprovals ? d.approvals().asMap() : null,
includeFiles, includeFiles,
d.change(), d.change(),

View File

@@ -27,8 +27,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class), new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments( new ChangeQueryBuilder.Arguments(
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, null, indexes, null, null, null, null, null, null, null, null, null, null, null, null, indexes, null, null, null, null, null, null, null, null));
null));
} }
@Operator @Operator