Merge "Make ChangeControl package-private"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
index 1e3e502..1ca98b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
@@ -214,7 +214,8 @@
@Override
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());
if (!edit.isPresent()) {
return Response.none();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 8dc53bc..19c2157 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -83,6 +83,7 @@
import com.google.gerrit.extensions.config.DownloadCommand;
import com.google.gerrit.extensions.config.DownloadScheme;
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.index.query.QueryResult;
import com.google.gerrit.reviewdb.client.Account;
@@ -116,11 +117,12 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
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.PermissionBackend;
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.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -228,7 +230,6 @@
private final ApprovalsUtil approvalsUtil;
private final RemoveReviewerControl removeReviewerControl;
private final TrackingFooters trackingFooters;
- private final ChangeControl.GenericFactory changeControlFactory;
private boolean lazyLoad = true;
private AccountLoader accountLoader;
private FixInput fix;
@@ -261,7 +262,6 @@
ApprovalsUtil approvalsUtil,
RemoveReviewerControl removeReviewerControl,
TrackingFooters trackingFooters,
- ChangeControl.GenericFactory changeControlFactory,
@Assisted Iterable<ListChangesOption> options) {
this.db = db;
this.userProvider = user;
@@ -287,7 +287,6 @@
this.indexes = indexes;
this.approvalsUtil = approvalsUtil;
this.removeReviewerControl = removeReviewerControl;
- this.changeControlFactory = changeControlFactory;
this.options = Sets.immutableEnumSet(options);
this.trackingFooters = trackingFooters;
}
@@ -347,6 +346,7 @@
| OrmException
| IOException
| PermissionBackendException
+ | NoSuchProjectException
| RuntimeException e) {
if (!has(CHECK)) {
Throwables.throwIfInstanceOf(e, OrmException.class);
@@ -425,6 +425,7 @@
| OrmException
| IOException
| PermissionBackendException
+ | NoSuchProjectException
| RuntimeException e) {
if (has(CHECK)) {
i = checkOnly(cd);
@@ -491,7 +492,7 @@
private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
throws PatchListNotAvailableException, GpgException, OrmException, IOException,
- PermissionBackendException {
+ PermissionBackendException, NoSuchProjectException {
ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
@@ -506,11 +507,7 @@
}
}
- PermissionBackend.WithUser withUser = permissionBackend.user(user).database(db);
- PermissionBackend.ForChange perm =
- lazyLoad
- ? withUser.change(cd)
- : withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
+ PermissionBackend.ForChange perm = permissionBackendForChange(user, cd);
Change in = cd.change();
out.project = in.getProject().get();
out.branch = in.getDest().getShortName();
@@ -596,19 +593,15 @@
src = null;
}
- ChangeControl ctl = null;
- if (needMessages || needRevisions) {
- ctl = changeControlFactory.controlFor(db.get(), cd.change(), userProvider.get());
- }
if (needMessages) {
- out.messages = messages(ctl, cd);
+ out.messages = messages(cd);
}
finish(out);
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
if (needRevisions) {
- out.revisions = revisions(ctl, cd, src, limitToPsId, out);
+ out.revisions = revisions(cd, src, limitToPsId, out);
if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) {
@@ -1103,8 +1096,7 @@
return result;
}
- private Collection<ChangeMessageInfo> messages(ChangeControl ctl, ChangeData cd)
- throws OrmException {
+ private Collection<ChangeMessageInfo> messages(ChangeData cd) throws OrmException {
List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes());
if (messages.isEmpty()) {
return Collections.emptyList();
@@ -1113,26 +1105,24 @@
List<ChangeMessageInfo> result = Lists.newArrayListWithCapacity(messages.size());
for (ChangeMessage message : messages) {
PatchSet.Id patchNum = message.getPatchSetId();
- if (patchNum == null || ctl.isVisible(db.get())) {
- ChangeMessageInfo cmi = new ChangeMessageInfo();
- cmi.id = message.getKey().get();
- cmi.author = accountLoader.get(message.getAuthor());
- cmi.date = message.getWrittenOn();
- cmi.message = message.getMessage();
- cmi.tag = message.getTag();
- cmi._revisionNumber = patchNum != null ? patchNum.get() : null;
- Account.Id realAuthor = message.getRealAuthor();
- if (realAuthor != null) {
- cmi.realAuthor = accountLoader.get(realAuthor);
- }
- result.add(cmi);
+ ChangeMessageInfo cmi = new ChangeMessageInfo();
+ cmi.id = message.getKey().get();
+ cmi.author = accountLoader.get(message.getAuthor());
+ cmi.date = message.getWrittenOn();
+ cmi.message = message.getMessage();
+ cmi.tag = message.getTag();
+ cmi._revisionNumber = patchNum != null ? patchNum.get() : null;
+ Account.Id realAuthor = message.getRealAuthor();
+ if (realAuthor != null) {
+ cmi.realAuthor = accountLoader.get(realAuthor);
}
+ result.add(cmi);
}
return result;
}
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
// which CCs are removable.
//
@@ -1228,13 +1218,14 @@
}
private Map<String, RevisionInfo> revisions(
- ChangeControl ctl,
ChangeData cd,
Map<PatchSet.Id, PatchSet> map,
Optional<PatchSet.Id> limitToPsId,
ChangeInfo changeInfo)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException {
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
Map<String, RevisionInfo> res = new LinkedHashMap<>();
+ Boolean isWorldReadable = null;
try (Repository repo = openRepoIfNecessary(cd.project());
RevWalk rw = newRevWalk(repo)) {
for (PatchSet in : map.values()) {
@@ -1247,8 +1238,13 @@
} else {
want = id.equals(cd.change().currentPatchSetId());
}
- if (want && ctl.isVisible(db.get())) {
- res.put(in.getRevision().get(), toRevisionInfo(cd, in, repo, rw, false, changeInfo));
+ if (want) {
+ if (isWorldReadable == null) {
+ isWorldReadable = isWorldReadable(cd);
+ }
+ res.put(
+ in.getRevision().get(),
+ toRevisionInfo(cd, in, repo, rw, false, changeInfo, isWorldReadable));
}
}
return res;
@@ -1283,11 +1279,12 @@
}
public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException {
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
try (Repository repo = openRepoIfNecessary(cd.project());
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();
return rev;
}
@@ -1299,8 +1296,10 @@
@Nullable Repository repo,
@Nullable RevWalk rw,
boolean fillCommit,
- @Nullable ChangeInfo changeInfo)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException {
+ @Nullable ChangeInfo changeInfo,
+ boolean isWorldReadable)
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
Change c = cd.change();
RevisionInfo out = new RevisionInfo();
out.isCurrent = in.getId().equals(c.currentPatchSetId());
@@ -1308,7 +1307,7 @@
out.ref = in.getRefName();
out.created = in.getCreatedOn();
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.description = in.getDescription();
@@ -1398,10 +1397,9 @@
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<>();
-
- ChangeControl ctl = changeControlFactory.controlFor(db.get(), cd.change(), anonymous);
for (DynamicMap.Entry<DownloadScheme> e : downloadSchemes) {
String schemeName = e.getExportName();
DownloadScheme scheme = e.getProvider().get();
@@ -1409,8 +1407,7 @@
|| (scheme.isAuthRequired() && !userProvider.get().isIdentifiedUser())) {
continue;
}
-
- if (!scheme.isAuthSupported() && !ctl.isVisible(db.get())) {
+ if (!scheme.isAuthSupported() && !isWorldReadable) {
continue;
}
@@ -1464,6 +1461,27 @@
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
abstract static class LabelWithStatus {
private static LabelWithStatus create(LabelInfo label, SubmitRecord.Label.Status status) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index c743769..91f7720 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -119,7 +120,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException, PermissionBackendException,
- IOException {
+ IOException, NoSuchProjectException {
Account.Id reviewerId = reviewer.getId();
if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
throw new ResourceNotFoundException();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index 10164ce..8c6c3cc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -41,6 +41,7 @@
import com.google.gerrit.server.mail.send.DeleteVoteSender;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
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.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -162,7 +163,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws OrmException, AuthException, ResourceNotFoundException, IOException,
- PermissionBackendException {
+ PermissionBackendException, NoSuchProjectException {
change = ctx.getChange();
PatchSet.Id psId = change.currentPatchSetId();
ps = psUtil.current(db.get(), ctx.getNotes());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
index dc180cc..c167e31 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
@@ -43,6 +43,7 @@
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -146,7 +147,8 @@
@Override
public Response<?> apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceNotFoundException, OrmException,
- RepositoryNotFoundException, IOException, PatchListNotAvailableException {
+ RepositoryNotFoundException, IOException, PatchListNotAvailableException,
+ PermissionBackendException {
checkOptions();
if (reviewed) {
return Response.ok(reviewed(resource));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java
index c91748c..25902b9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java
@@ -47,6 +47,7 @@
import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes;
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.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
@@ -122,7 +123,7 @@
@Override
public Response<DiffInfo> apply(FileResource resource)
throws ResourceConflictException, ResourceNotFoundException, OrmException, AuthException,
- InvalidChangeOperationException, IOException {
+ InvalidChangeOperationException, IOException, PermissionBackendException {
DiffPreferencesInfo prefs = new DiffPreferencesInfo();
if (whitespace != null) {
prefs.ignoreWhitespace = whitespace;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java
index 6002f75..27c5d49 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetPureRevert.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -53,7 +52,6 @@
private final ChangeNotes.Factory notesFactory;
private final Provider<ReviewDb> dbProvider;
private final PatchSetUtil psUtil;
- private final ChangeControl.GenericFactory changeControlFactory;
@Option(
name = "--claimed-original",
@@ -70,15 +68,13 @@
ProjectCache projectCache,
ChangeNotes.Factory notesFactory,
Provider<ReviewDb> dbProvider,
- PatchSetUtil psUtil,
- ChangeControl.GenericFactory changeControlFactory) {
+ PatchSetUtil psUtil) {
this.mergeUtilFactory = mergeUtilFactory;
this.repoManager = repoManager;
this.projectCache = projectCache;
this.notesFactory = notesFactory;
this.dbProvider = dbProvider;
this.psUtil = psUtil;
- this.changeControlFactory = changeControlFactory;
}
@Override
@@ -88,10 +84,6 @@
PatchSet currentPatchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
if (currentPatchSet == null) {
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());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
index a6583b1..a60bf9c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RelatedChangesSorter.PatchSetData;
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.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -64,14 +65,15 @@
@Override
public RelatedInfo apply(RevisionResource rsrc)
- throws RepositoryNotFoundException, IOException, OrmException, NoSuchProjectException {
+ throws RepositoryNotFoundException, IOException, OrmException, NoSuchProjectException,
+ PermissionBackendException {
RelatedInfo relatedInfo = new RelatedInfo();
relatedInfo.changes = getRelated(rsrc);
return relatedInfo;
}
private List<ChangeAndCommit> getRelated(RevisionResource rsrc)
- throws OrmException, IOException, NoSuchProjectException {
+ throws OrmException, IOException, NoSuchProjectException, PermissionBackendException {
Set<String> groups = getAllGroups(rsrc.getNotes());
if (groups.isEmpty()) {
return Collections.emptyList();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
index 5edef0a..aa10af8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
@@ -35,7 +35,6 @@
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.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
@@ -73,7 +72,6 @@
private final PatchSetUtil psUtil;
private final NotifyUtil notifyUtil;
private final ProjectCache projectCache;
- private final ChangeControl.GenericFactory changeControlFactory;
@Inject
PutMessage(
@@ -86,8 +84,7 @@
@GerritPersonIdent PersonIdent gerritIdent,
PatchSetUtil psUtil,
NotifyUtil notifyUtil,
- ProjectCache projectCache,
- ChangeControl.GenericFactory changeControlFactory) {
+ ProjectCache projectCache) {
super(retryHelper);
this.repositoryManager = repositoryManager;
this.currentUserProvider = currentUserProvider;
@@ -98,7 +95,6 @@
this.psUtil = psUtil;
this.notifyUtil = notifyUtil;
this.projectCache = projectCache;
- this.changeControlFactory = changeControlFactory;
}
@Override
@@ -109,10 +105,6 @@
PatchSet ps = psUtil.current(db.get(), resource.getNotes());
if (ps == null) {
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) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
index 7d92973..59ab190 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
@@ -35,13 +35,12 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RebaseUtil.Base;
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.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
@@ -74,8 +73,7 @@
private final RebaseUtil rebaseUtil;
private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider;
- private final Provider<CurrentUser> userProvider;
- private final ChangeControl.GenericFactory changeControlFactory;
+ private final PermissionBackend permissionBackend;
@Inject
public Rebase(
@@ -85,16 +83,14 @@
RebaseUtil rebaseUtil,
ChangeJson.Factory json,
Provider<ReviewDb> dbProvider,
- Provider<CurrentUser> userProvider,
- ChangeControl.GenericFactory changeControlFactory) {
+ PermissionBackend permissionBackend) {
super(retryHelper);
this.repoManager = repoManager;
this.rebaseFactory = rebaseFactory;
this.rebaseUtil = rebaseUtil;
this.json = json;
this.dbProvider = dbProvider;
- this.userProvider = userProvider;
- this.changeControlFactory = changeControlFactory;
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -132,7 +128,8 @@
private ObjectId findBaseRev(
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();
if (input == null || input.base == null) {
return rebaseUtil.findBaseRevision(rsrc.getPatchSet(), destRefKey, repo, rw);
@@ -150,20 +147,21 @@
return destRef.getObjectId();
}
- @SuppressWarnings("resource")
- ReviewDb db = dbProvider.get();
Base base = rebaseUtil.parseBase(rsrc, str);
if (base == null) {
throw new ResourceConflictException("base revision is missing: " + str);
}
PatchSet.Id baseId = base.patchSet().getId();
- ChangeControl baseCtl = changeControlFactory.controlFor(base.notes(), userProvider.get());
- if (!baseCtl.isVisible(db)) {
- throw new AuthException("base revision not accessible: " + str);
- } else if (change.getId().equals(baseId.getParentKey())) {
+ if (change.getId().equals(baseId.getParentKey())) {
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();
if (!baseChange.getProject().equals(change.getProject())) {
throw new ResourceConflictException(
@@ -228,18 +226,12 @@
extends RetryingRestModifyView<ChangeResource, RebaseInput, ChangeInfo> {
private final PatchSetUtil psUtil;
private final Rebase rebase;
- private final ChangeControl.GenericFactory changeControlFactory;
@Inject
- CurrentRevision(
- RetryHelper retryHelper,
- PatchSetUtil psUtil,
- Rebase rebase,
- ChangeControl.GenericFactory changeControlFactory) {
+ CurrentRevision(RetryHelper retryHelper, PatchSetUtil psUtil, Rebase rebase) {
super(retryHelper);
this.psUtil = psUtil;
this.rebase = rebase;
- this.changeControlFactory = changeControlFactory;
}
@Override
@@ -250,10 +242,6 @@
PatchSet ps = psUtil.current(rebase.dbProvider.get(), rsrc.getNotes());
if (ps == null) {
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);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RelatedChangesSorter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RelatedChangesSorter.java
index 86d6e81..68f637e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RelatedChangesSorter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RelatedChangesSorter.java
@@ -24,16 +24,21 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
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.PatchSet;
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.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.ProjectControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayDeque;
@@ -55,23 +60,27 @@
@Singleton
class RelatedChangesSorter {
private final GitRepositoryManager repoManager;
- private final ProjectControl.GenericFactory projectControlFactory;
+ private final PermissionBackend permissionBackend;
+ private final Provider<ReviewDb> dbProvider;
@Inject
RelatedChangesSorter(
- GitRepositoryManager repoManager, ProjectControl.GenericFactory projectControlFactory) {
+ GitRepositoryManager repoManager,
+ PermissionBackend permissionBackend,
+ Provider<ReviewDb> dbProvider) {
this.repoManager = repoManager;
- this.projectControlFactory = projectControlFactory;
+ this.permissionBackend = permissionBackend;
+ this.dbProvider = dbProvider;
}
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");
// Map of all patch sets, keyed by commit SHA-1.
Map<String, PatchSetData> byId = collectById(in);
PatchSetData start = byId.get(startPs.getRevision().get());
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.
ListMultimap<PatchSetData, PatchSetData> parents =
@@ -98,9 +107,9 @@
}
}
- Collection<PatchSetData> ancestors = walkAncestors(ctl, parents, start);
+ Collection<PatchSetData> ancestors = walkAncestors(perm, parents, start);
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);
result.addAll(Lists.reverse(descendants));
result.addAll(ancestors);
@@ -133,14 +142,16 @@
}
private static Collection<PatchSetData> walkAncestors(
- ProjectControl ctl, ListMultimap<PatchSetData, PatchSetData> parents, PatchSetData start)
- throws OrmException {
+ PermissionBackend.WithUser perm,
+ ListMultimap<PatchSetData, PatchSetData> parents,
+ PatchSetData start)
+ throws PermissionBackendException {
LinkedHashSet<PatchSetData> result = new LinkedHashSet<>();
Deque<PatchSetData> pending = new ArrayDeque<>();
pending.add(start);
while (!pending.isEmpty()) {
PatchSetData psd = pending.remove();
- if (result.contains(psd) || !isVisible(psd, ctl)) {
+ if (result.contains(psd) || !isVisible(psd, perm)) {
continue;
}
result.add(psd);
@@ -150,24 +161,25 @@
}
private static List<PatchSetData> walkDescendants(
- ProjectControl ctl,
+ PermissionBackend.WithUser perm,
ListMultimap<PatchSetData, PatchSetData> children,
PatchSetData start,
List<PatchSetData> otherPatchSetsOfStart,
Iterable<PatchSetData> ancestors)
- throws OrmException {
+ throws PermissionBackendException {
Set<Change.Id> alreadyEmittedChanges = new HashSet<>();
addAllChangeIds(alreadyEmittedChanges, ancestors);
// Prefer descendants found by following the original patch set passed in.
List<PatchSetData> result =
- walkDescendentsImpl(ctl, alreadyEmittedChanges, children, ImmutableList.of(start));
+ walkDescendentsImpl(perm, alreadyEmittedChanges, children, ImmutableList.of(start));
addAllChangeIds(alreadyEmittedChanges, result);
// Then, go back and add new indirect descendants found by following any
// other patch sets of start. These show up after all direct descendants,
// 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;
}
@@ -179,11 +191,11 @@
}
private static List<PatchSetData> walkDescendentsImpl(
- ProjectControl ctl,
+ PermissionBackend.WithUser perm,
Set<Change.Id> alreadyEmittedChanges,
ListMultimap<PatchSetData, PatchSetData> children,
List<PatchSetData> start)
- throws OrmException {
+ throws PermissionBackendException {
if (start.isEmpty()) {
return ImmutableList.of();
}
@@ -194,7 +206,7 @@
pending.addAll(start);
while (!pending.isEmpty()) {
PatchSetData psd = pending.remove();
- if (seen.contains(psd) || !isVisible(psd, ctl)) {
+ if (seen.contains(psd) || !isVisible(psd, perm)) {
continue;
}
seen.add(psd);
@@ -225,10 +237,14 @@
return result;
}
- private static boolean isVisible(PatchSetData psd, ProjectControl ctl) throws OrmException {
- // Reuse existing project control rather than lazily creating a new one for
- // each ChangeData.
- return ctl.controlFor(psd.data().notes()).isPatchVisible(psd.patchSet(), psd.data());
+ private static boolean isVisible(PatchSetData psd, PermissionBackend.WithUser perm)
+ throws PermissionBackendException {
+ try {
+ perm.change(psd.data()).check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
@AutoValue
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
index ef039dd..084bc25 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
@@ -28,7 +28,9 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.edit.ChangeEdit;
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.inject.Inject;
import com.google.inject.Provider;
@@ -46,7 +48,7 @@
private final Provider<ReviewDb> dbProvider;
private final ChangeEditUtil editUtil;
private final PatchSetUtil psUtil;
- private final ChangeControl.GenericFactory changeControlFactory;
+ private final PermissionBackend permissionBackend;
@Inject
Revisions(
@@ -54,12 +56,12 @@
Provider<ReviewDb> dbProvider,
ChangeEditUtil editUtil,
PatchSetUtil psUtil,
- ChangeControl.GenericFactory changeControlFactory) {
+ PermissionBackend permissionBackend) {
this.views = views;
this.dbProvider = dbProvider;
this.editUtil = editUtil;
this.psUtil = psUtil;
- this.changeControlFactory = changeControlFactory;
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -74,7 +76,8 @@
@Override
public RevisionResource parse(ChangeResource change, IdString id)
- throws ResourceNotFoundException, AuthException, OrmException, IOException {
+ throws ResourceNotFoundException, AuthException, OrmException, IOException,
+ PermissionBackendException {
if (id.get().equals("current")) {
PatchSet ps = psUtil.current(dbProvider.get(), change.getNotes());
if (ps != null && visible(change)) {
@@ -100,10 +103,17 @@
}
}
- private boolean visible(ChangeResource change) throws OrmException {
- return changeControlFactory
- .controlFor(change.getNotes(), change.getUser())
- .isVisible(dbProvider.get());
+ private boolean visible(ChangeResource change) throws PermissionBackendException {
+ try {
+ permissionBackend
+ .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)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index cab61b3..84ba88e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -55,7 +55,6 @@
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.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -507,20 +506,17 @@
private final Submit submit;
private final ChangeJson.Factory json;
private final PatchSetUtil psUtil;
- private final ChangeControl.GenericFactory changeControlFactory;
@Inject
CurrentRevision(
Provider<ReviewDb> dbProvider,
Submit submit,
ChangeJson.Factory json,
- PatchSetUtil psUtil,
- ChangeControl.GenericFactory changeControlFactory) {
+ PatchSetUtil psUtil) {
this.dbProvider = dbProvider;
this.submit = submit;
this.json = json;
this.psUtil = psUtil;
- this.changeControlFactory = changeControlFactory;
}
@Override
@@ -530,10 +526,6 @@
PatchSet ps = psUtil.current(dbProvider.get(), rsrc.getNotes());
if (ps == null) {
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);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
index 1415f3b..ef69616 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -73,7 +74,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
index 3bba164..e9ae356 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -67,7 +68,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
index 0437623..c25deab 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -66,7 +67,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
index 1676c2c..77cd1a8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -77,7 +78,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
index be308a9..95d7132 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.change.ChangeJson;
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.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -80,12 +81,14 @@
}
public RevisionInfo revisionInfo(Project project, PatchSet ps)
- throws OrmException, PatchListNotAvailableException, GpgException, IOException {
+ throws OrmException, PatchListNotAvailableException, GpgException, IOException,
+ PermissionBackendException {
return revisionInfo(project.getNameKey(), 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());
return changeJson.getRevisionInfo(cd, ps);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
index d785f38..fc6881d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -70,7 +71,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
index 9914563..28e07a9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -81,7 +82,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
index 475e4b5..76779ca 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -67,7 +68,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
index 5f20293..8944698 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchListObjectTooLargeException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
@@ -81,7 +82,11 @@
}
} catch (PatchListObjectTooLargeException e) {
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);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
index 4e0c3ae..7a17148 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
@@ -38,9 +38,7 @@
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.ChangeControl;
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.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -106,8 +104,6 @@
private final Map<QueryKey, List<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
- private final ChangeControl.GenericFactory changeControlFactory;
- private final ProjectCache projectCache;
private MergeOpRepoManager orm;
private boolean closeOrm;
@@ -119,17 +115,13 @@
Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider,
PermissionBackend permissionBackend,
- SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
- ChangeControl.GenericFactory changeControlFactory,
- ProjectCache projectCache) {
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.cfg = cfg;
this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
this.permissionBackend = permissionBackend;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
- this.changeControlFactory = changeControlFactory;
- this.projectCache = projectCache;
queryCache = new HashMap<>();
heads = new HashMap<>();
}
@@ -160,7 +152,7 @@
}
}
- private SubmitType submitType(CurrentUser user, ChangeData cd, PatchSet ps, boolean visible)
+ private SubmitType submitType(CurrentUser user, ChangeData cd, PatchSet ps)
throws OrmException, IOException {
// Submit type prolog rules mean that the submit type can depend on the
// submitting user and the content of the change.
@@ -172,10 +164,6 @@
// 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
// would be submitted together with the visible ones.
- if (!visible) {
- return projectCache.checkedGet(cd.project()).getProject().getSubmitType();
- }
-
SubmitTypeRecord str =
ps == cd.currentPatchSet()
? cd.submitTypeRecord()
@@ -247,18 +235,7 @@
// is visible, we use the most recent one. Otherwise, use the current
// patch set.
PatchSet ps = cd.currentPatchSet();
- boolean visiblePatchSet = visible;
- 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 (submitType(user, cd, ps) == SubmitType.CHERRY_PICK) {
if (visible) {
visibleChanges.add(cd);
} else {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
index 2a22c1c..79c0cdb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
@@ -50,7 +50,6 @@
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.validators.OnSubmitValidators;
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.ProjectState;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -107,7 +106,6 @@
final AccountCache accountCache;
final ApprovalsUtil approvalsUtil;
- final ChangeControl.GenericFactory changeControlFactory;
final ChangeMerged changeMerged;
final ChangeMessagesUtil cmUtil;
final EmailMerge.Factory mergedSenderFactory;
@@ -146,7 +144,6 @@
Arguments(
AccountCache accountCache,
ApprovalsUtil approvalsUtil,
- ChangeControl.GenericFactory changeControlFactory,
ChangeMerged changeMerged,
ChangeMessagesUtil cmUtil,
EmailMerge.Factory mergedSenderFactory,
@@ -178,7 +175,6 @@
@Assisted boolean dryrun) {
this.accountCache = accountCache;
this.approvalsUtil = approvalsUtil;
- this.changeControlFactory = changeControlFactory;
this.changeMerged = changeMerged;
this.mergedSenderFactory = mergedSenderFactory;
this.repoManager = repoManager;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 384d4fd..fe158f8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -37,7 +37,9 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException;
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.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
@@ -90,7 +92,7 @@
private final DiffPreferencesInfo diffPrefs;
private final ChangeEditUtil editReader;
private final Provider<CurrentUser> userProvider;
- private final ChangeControl.GenericFactory changeControlFactory;
+ private final PermissionBackend permissionBackend;
private Optional<ChangeEdit> edit;
private final Change.Id changeId;
@@ -113,7 +115,7 @@
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
Provider<CurrentUser> userProvider,
- ChangeControl.GenericFactory changeControlFactory,
+ PermissionBackend permissionBackend,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted("patchSetA") @Nullable PatchSet.Id patchSetA,
@@ -128,7 +130,7 @@
this.commentsUtil = commentsUtil;
this.editReader = editReader;
this.userProvider = userProvider;
- this.changeControlFactory = changeControlFactory;
+ this.permissionBackend = permissionBackend;
this.fileName = fileName;
this.psa = patchSetA;
@@ -149,7 +151,7 @@
CommentsUtil commentsUtil,
ChangeEditUtil editReader,
Provider<CurrentUser> userProvider,
- ChangeControl.GenericFactory changeControlFactory,
+ PermissionBackend permissionBackend,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted int parentNum,
@@ -164,7 +166,7 @@
this.commentsUtil = commentsUtil;
this.editReader = editReader;
this.userProvider = userProvider;
- this.changeControlFactory = changeControlFactory;
+ this.permissionBackend = permissionBackend;
this.fileName = fileName;
this.psa = null;
@@ -187,7 +189,7 @@
@Override
public PatchScript call()
throws OrmException, LargeObjectException, AuthException, InvalidChangeOperationException,
- IOException {
+ IOException, PermissionBackendException {
if (parentNum < 0) {
validatePatchSetId(psa);
}
@@ -195,10 +197,16 @@
PatchSet psEntityA = psa != null ? psUtil.get(db, notes, psa) : null;
PatchSet psEntityB = psb.get() == 0 ? new PatchSet(psb) : psUtil.get(db, notes, psb);
-
- ChangeControl ctl = changeControlFactory.controlFor(notes, userProvider.get());
- if ((psEntityA != null && !ctl.isVisible(db)) || (psEntityB != null && !ctl.isVisible(db))) {
- throw new NoSuchChangeException(changeId);
+ if (psEntityA != null || psEntityB != null) {
+ try {
+ permissionBackend
+ .user(userProvider)
+ .change(notes)
+ .database(db)
+ .check(ChangePermission.READ);
+ } catch (AuthException e) {
+ throw new NoSuchChangeException(changeId);
+ }
}
try (Repository git = repoManager.openRepository(notes.getProjectName())) {
@@ -212,8 +220,7 @@
final PatchScriptBuilder b = newBuilder(list, git);
final PatchListEntry content = list.get(fileName);
- loadCommentsAndHistory(
- ctl, content.getChangeType(), content.getOldName(), content.getNewName());
+ loadCommentsAndHistory(content.getChangeType(), content.getOldName(), content.getNewName());
return b.toPatchScript(content, comments, history);
} catch (PatchListNotAvailableException e) {
@@ -285,8 +292,7 @@
}
}
- private void loadCommentsAndHistory(
- ChangeControl ctl, ChangeType changeType, String oldName, String newName)
+ private void loadCommentsAndHistory(ChangeType changeType, String oldName, String newName)
throws OrmException {
Map<Patch.Key, Patch> byKey = new HashMap<>();
@@ -298,9 +304,6 @@
//
history = new ArrayList<>();
for (PatchSet ps : psUtil.byChange(db, notes)) {
- if (!ctl.isVisible(db)) {
- continue;
- }
String name = fileName;
if (psa != null) {
switch (changeType) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 2203f77..91433ee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -14,10 +14,8 @@
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.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.Sets;
@@ -27,7 +25,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
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.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -46,65 +43,15 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.io.IOException;
import java.util.Collection;
import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
-import java.util.function.Predicate;
/** Access control management for a user accessing a single change. */
-public class ChangeControl {
+class ChangeControl {
@Singleton
- public static class GenericFactory {
- 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 {
+ static class Factory {
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
@@ -152,7 +99,7 @@
this.patchSetUtil = patchSetUtil;
}
- public ChangeControl forUser(CurrentUser who) {
+ ChangeControl forUser(CurrentUser who) {
if (getUser().equals(who)) {
return this;
}
@@ -160,40 +107,27 @@
changeDataFactory, approvalsUtil, getRefControl().forUser(who), notes, patchSetUtil);
}
- public RefControl getRefControl() {
+ private RefControl getRefControl() {
return refControl;
}
- public CurrentUser getUser() {
+ private CurrentUser getUser() {
return getRefControl().getUser();
}
- public ProjectControl getProjectControl() {
+ private ProjectControl getProjectControl() {
return getRefControl().getProjectControl();
}
- public Project getProject() {
- return getProjectControl().getProject();
- }
-
- public Change.Id getId() {
- return notes.getChangeId();
- }
-
- public Change getChange() {
+ private Change getChange() {
return notes.getChange();
}
- public ChangeNotes getNotes() {
+ private ChangeNotes getNotes() {
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? */
private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
return false;
@@ -202,36 +136,10 @@
}
/** Can the user see this change? Does not account for draft status */
- public boolean isRefVisible() {
+ private boolean isRefVisible() {
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? */
private boolean canAbandon(ReviewDb db) throws OrmException {
return (isOwner() // owner (aka creator) of the change can abandon
@@ -243,7 +151,7 @@
}
/** Can this user delete this change? */
- public boolean canDelete(Change.Status status) {
+ private boolean canDelete(Change.Status status) {
switch (status) {
case NEW:
case ABANDONED:
@@ -285,7 +193,7 @@
}
/** 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) {
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackendModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackendModule.java
index 8fa049d..bdfc67f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackendModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DefaultPermissionBackendModule.java
@@ -32,7 +32,6 @@
// TODO(sop) Hide ProjectControl, RefControl, ChangeControl related bindings.
bind(ProjectControl.GenericFactory.class);
factory(ProjectControl.AssistedFactory.class);
- bind(ChangeControl.GenericFactory.class);
bind(ChangeControl.Factory.class);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
index ded2bf8..9c9be7c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RemoveReviewerControl.java
@@ -29,27 +29,29 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.io.IOException;
@Singleton
public class RemoveReviewerControl {
private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
- private final ChangeControl.GenericFactory changeControlFactory;
+ private final ProjectControl.GenericFactory projectControlFactory;
@Inject
RemoveReviewerControl(
PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider,
- ChangeControl.GenericFactory changeControlFactory) {
+ ProjectControl.GenericFactory projectControlFactory) {
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
- this.changeControlFactory = changeControlFactory;
+ this.projectControlFactory = projectControlFactory;
}
/** @throws AuthException if this user is not allowed to remove this approval. */
public void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
- throws PermissionBackendException, AuthException, NoSuchChangeException, OrmException {
+ throws PermissionBackendException, AuthException, NoSuchProjectException, OrmException,
+ IOException {
if (canRemoveReviewerWithoutPermissionCheck(
notes.getChange(), currentUser, approval.getAccountId(), approval.getValue())) {
return;
@@ -65,7 +67,7 @@
/** @return true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer(
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)) {
return true;
}
@@ -78,7 +80,7 @@
private boolean canRemoveReviewerWithoutPermissionCheck(
Change change, CurrentUser currentUser, Account.Id reviewer, int value)
- throws NoSuchChangeException, OrmException {
+ throws NoSuchProjectException, OrmException, IOException {
if (!change.getStatus().isOpen()) {
return false;
}
@@ -94,11 +96,10 @@
// Users with the remove reviewer permission, the branch owner, project
// owner and site admin can remove anyone
- ChangeControl changeControl =
- changeControlFactory.controlFor(dbProvider.get(), change, currentUser);
- if (changeControl.getRefControl().isOwner() // branch owner
- || changeControl.getProjectControl().isOwner() // project owner
- || changeControl.getProjectControl().isAdmin()) { // project admin
+ ProjectControl ctl = projectControlFactory.controlFor(change.getProject(), currentUser);
+ if (ctl.controlForRef(change.getDest()).isOwner() // branch owner
+ || ctl.isOwner() // project owner
+ || ctl.isAdmin()) { // project admin
return true;
}
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 2a71258..dfcc999 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -72,7 +72,6 @@
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
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.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
@@ -326,7 +325,7 @@
ChangeData cd =
new ChangeData(
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));
return cd;
}
@@ -349,7 +348,6 @@
private final TrackingFooters trackingFooters;
private final GetPureRevert pureRevert;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
- private final ChangeControl.GenericFactory changeControlFactory;
// Required assisted injected fields.
private final ReviewDb db;
@@ -375,7 +373,6 @@
private Collection<Comment> publishedComments;
private Collection<RobotComment> robotComments;
private CurrentUser visibleTo;
- private ChangeControl changeControl;
private List<ChangeMessage> messages;
private Optional<ChangedLines> changedLines;
private SubmitTypeRecord submitTypeRecord;
@@ -420,7 +417,6 @@
TrackingFooters trackingFooters,
GetPureRevert pureRevert,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
- ChangeControl.GenericFactory changeControlFactory,
@Assisted ReviewDb db,
@Assisted Project.NameKey project,
@Assisted Change.Id id,
@@ -443,7 +439,6 @@
this.trackingFooters = trackingFooters;
this.pureRevert = pureRevert;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
- this.changeControlFactory = changeControlFactory;
// 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
@@ -554,22 +549,8 @@
return visibleTo == user;
}
- private ChangeControl changeControl() throws OrmException {
- // TODO(hiesel): Remove this method.
- 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;
+ void cacheVisibleTo(CurrentUser user) {
+ visibleTo = user;
}
public Change change() throws OrmException {
@@ -980,15 +961,8 @@
return null;
}
PatchSet ps = currentPatchSet();
- try {
- if (ps == null || !changeControl().isVisible(db)) {
- return null;
- }
- } catch (OrmException e) {
- if (e.getCause() instanceof NoSuchChangeException) {
- return null;
- }
- throw e;
+ if (ps == null) {
+ return null;
}
try (Repository repo = repoManager.openRepository(project())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index 7bbb27b..17296bc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -23,28 +23,23 @@
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.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData> {
protected final Provider<ReviewDb> db;
protected final ChangeNotes.Factory notesFactory;
- protected final ChangeControl.GenericFactory changeControlFactory;
protected final CurrentUser user;
protected final PermissionBackend permissionBackend;
public ChangeIsVisibleToPredicate(
Provider<ReviewDb> db,
ChangeNotes.Factory notesFactory,
- ChangeControl.GenericFactory changeControlFactory,
CurrentUser user,
PermissionBackend permissionBackend) {
super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user));
this.db = db;
this.notesFactory = notesFactory;
- this.changeControlFactory = changeControlFactory;
this.user = user;
this.permissionBackend = permissionBackend;
}
@@ -59,15 +54,7 @@
return false;
}
- ChangeControl changeControl;
ChangeNotes notes = notesFactory.createFromIndexedChange(change);
- try {
- changeControl = changeControlFactory.controlFor(notes, user);
- } catch (NoSuchChangeException e) {
- // Ignored
- return false;
- }
-
boolean visible;
try {
visible =
@@ -80,7 +67,7 @@
throw new OrmException("unable to check permissions", e);
}
if (visible) {
- cd.cacheVisibleTo(changeControl);
+ cd.cacheVisibleTo(user);
return true;
}
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 1f28dbd..c7a85f0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -70,7 +70,6 @@
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListCache;
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.ProjectCache;
import com.google.gwtorm.server.OrmException;
@@ -196,7 +195,6 @@
final AllProjectsName allProjectsName;
final AllUsersName allUsersName;
final PermissionBackend permissionBackend;
- final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory;
final ChangeIndex index;
final ChangeIndexRewriter rewriter;
@@ -233,7 +231,6 @@
IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self,
PermissionBackend permissionBackend,
- ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory,
ChangeData.Factory changeDataFactory,
CommentsUtil commentsUtil,
@@ -263,7 +260,6 @@
userFactory,
self,
permissionBackend,
- changeControlGenericFactory,
notesFactory,
changeDataFactory,
commentsUtil,
@@ -295,7 +291,6 @@
IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self,
PermissionBackend permissionBackend,
- ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory,
ChangeData.Factory changeDataFactory,
CommentsUtil commentsUtil,
@@ -324,7 +319,6 @@
this.self = self;
this.permissionBackend = permissionBackend;
this.notesFactory = notesFactory;
- this.changeControlGenericFactory = changeControlGenericFactory;
this.changeDataFactory = changeDataFactory;
this.commentsUtil = commentsUtil;
this.accountResolver = accountResolver;
@@ -357,7 +351,6 @@
userFactory,
Providers.of(otherUser),
permissionBackend,
- changeControlGenericFactory,
notesFactory,
changeDataFactory,
commentsUtil,
@@ -926,8 +919,7 @@
}
public Predicate<ChangeData> visibleto(CurrentUser user) {
- return new ChangeIsVisibleToPredicate(
- args.db, args.notesFactory, args.changeControlGenericFactory, user, args.permissionBackend);
+ return new ChangeIsVisibleToPredicate(args.db, args.notesFactory, user, args.permissionBackend);
}
public Predicate<ChangeData> is_visible() throws QueryParseException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index eb6cf77..b190cd2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -34,7 +34,6 @@
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.ArrayList;
@@ -61,7 +60,6 @@
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> userProvider;
- private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory;
private final DynamicMap<ChangeAttributeFactory> attributeFactories;
private final PermissionBackend permissionBackend;
@@ -82,7 +80,6 @@
ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter,
Provider<ReviewDb> db,
- ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory,
DynamicMap<ChangeAttributeFactory> attributeFactories,
PermissionBackend permissionBackend) {
@@ -96,7 +93,6 @@
() -> limitsFactory.create(userProvider.get()).getQueryLimit());
this.db = db;
this.userProvider = userProvider;
- this.changeControlFactory = changeControlFactory;
this.notesFactory = notesFactory;
this.attributeFactories = attributeFactories;
this.permissionBackend = permissionBackend;
@@ -142,8 +138,7 @@
protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) {
return new AndChangeSource(
pred,
- new ChangeIsVisibleToPredicate(
- db, notesFactory, changeControlFactory, userProvider.get(), permissionBackend),
+ new ChangeIsVisibleToPredicate(db, notesFactory, userProvider.get(), permissionBackend),
start);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
index c9ddfb7..f8bd2e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -24,7 +24,6 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
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.util.LabelVote;
import com.google.inject.Provider;
@@ -38,7 +37,6 @@
protected static class Args {
protected final ProjectCache projectCache;
protected final PermissionBackend permissionBackend;
- protected final ChangeControl.GenericFactory ccFactory;
protected final IdentifiedUser.GenericFactory userFactory;
protected final Provider<ReviewDb> dbProvider;
protected final String value;
@@ -48,7 +46,6 @@
protected Args(
ProjectCache projectCache,
PermissionBackend permissionBackend,
- ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory,
Provider<ReviewDb> dbProvider,
String value,
@@ -56,7 +53,6 @@
AccountGroup.UUID group) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
- this.ccFactory = ccFactory;
this.userFactory = userFactory;
this.dbProvider = dbProvider;
this.value = value;
@@ -87,14 +83,7 @@
super(
predicates(
new Args(
- a.projectCache,
- a.permissionBackend,
- a.changeControlGenericFactory,
- a.userFactory,
- a.db,
- value,
- accounts,
- group)));
+ a.projectCache, a.permissionBackend, a.userFactory, a.db, value, accounts, group)));
this.value = value;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index dde577d..185517a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.data.QueryStatsAttribute;
import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gson.Gson;
import com.google.gwtorm.server.OrmException;
@@ -84,7 +83,6 @@
private final EventFactory eventFactory;
private final TrackingFooters trackingFooters;
private final CurrentUser user;
- private final ChangeControl.GenericFactory changeControlFactory;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private OutputFormat outputFormat = OutputFormat.TEXT;
@@ -110,7 +108,6 @@
EventFactory eventFactory,
TrackingFooters trackingFooters,
CurrentUser user,
- ChangeControl.GenericFactory changeControlFactory,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db;
this.repoManager = repoManager;
@@ -119,7 +116,6 @@
this.eventFactory = eventFactory;
this.trackingFooters = trackingFooters;
this.user = user;
- this.changeControlFactory = changeControlFactory;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
@@ -278,13 +274,12 @@
}
}
- ChangeControl ctl = changeControlFactory.controlFor(db, d.change(), user);
if (includePatchSets) {
eventFactory.addPatchSets(
db,
rw,
c,
- ctl.getVisiblePatchSets(d.patchSets(), db),
+ d.patchSets(),
includeApprovals ? d.approvals().asMap() : null,
includeFiles,
d.change(),
@@ -293,7 +288,7 @@
if (includeCurrentPatchSet) {
PatchSet current = d.currentPatchSet();
- if (current != null && ctl.isVisible(d.db())) {
+ if (current != null) {
c.currentPatchSet = eventFactory.asPatchSetAttribute(db, rw, d.change(), current);
eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes);
@@ -313,7 +308,7 @@
db,
rw,
c,
- ctl.getVisiblePatchSets(d.patchSets(), db),
+ d.patchSets(),
includeApprovals ? d.approvals().asMap() : null,
includeFiles,
d.change(),
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index a194336..b525504 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -27,8 +27,7 @@
new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(
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, null, indexes, null, null, null, null, null, null, null, null));
}
@Operator
