diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java index 788c6f6d35..fce8637154 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java @@ -34,7 +34,7 @@ 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.IdentifiedUser; -import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.Revisions; import com.google.gerrit.server.extensions.webui.UiActions; @@ -76,6 +76,7 @@ class PatchSetDetailFactory extends Handler { private final ReviewDb db; private final PatchListCache patchListCache; private final ChangeControl.Factory changeControlFactory; + private final ChangesCollection changes; private final Revisions revisions; private Project.NameKey projectKey; @@ -93,6 +94,7 @@ class PatchSetDetailFactory extends Handler { PatchSetDetailFactory(final PatchSetInfoFactory psif, final ReviewDb db, final PatchListCache patchListCache, final ChangeControl.Factory changeControlFactory, + final ChangesCollection changes, final Revisions revisions, @Assisted("psIdBase") @Nullable final PatchSet.Id psIdBase, @Assisted("psIdNew") final PatchSet.Id psIdNew, @@ -101,6 +103,7 @@ class PatchSetDetailFactory extends Handler { this.db = db; this.patchListCache = patchListCache; this.changeControlFactory = changeControlFactory; + this.changes = changes; this.revisions = revisions; this.psIdBase = psIdBase; @@ -179,7 +182,7 @@ class PatchSetDetailFactory extends Handler { detail.setCommands(Lists.newArrayList(Iterables.transform( UiActions.sorted(UiActions.plugins(UiActions.from( revisions, - new RevisionResource(new ChangeResource(control), patchSet), + new RevisionResource(changes.parse(control), patchSet), Providers.of(user)))), new Function() { @Override diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java index 18af5a2e8a..b2c4608b70 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java @@ -20,6 +20,7 @@ import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.extensions.api.changes.AddReviewerInput; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; @@ -33,6 +34,7 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.change.MergeabilityChecker; import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.config.AllProjectsNameProvider; @@ -41,7 +43,6 @@ import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.mail.CreateChangeSender; 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.ProjectControl; import com.google.gerrit.server.project.SetParent; @@ -78,11 +79,11 @@ public class ReviewProjectAccess extends ProjectAccessHandler { private final IdentifiedUser user; private final PatchSetInfoFactory patchSetInfoFactory; private final Provider reviewersProvider; - private final ChangeControl.GenericFactory changeFactory; private final MergeabilityChecker mergeabilityChecker; private final ChangeHooks hooks; private final CreateChangeSender.Factory createChangeSenderFactory; private final ProjectCache projectCache; + private final ChangesCollection changes; @Inject ReviewProjectAccess(final ProjectControl.Factory projectControlFactory, @@ -90,11 +91,11 @@ public class ReviewProjectAccess extends ProjectAccessHandler { MetaDataUpdate.User metaDataUpdateFactory, ReviewDb db, IdentifiedUser user, PatchSetInfoFactory patchSetInfoFactory, Provider reviewersProvider, - ChangeControl.GenericFactory changeFactory, MergeabilityChecker mergeabilityChecker, ChangeHooks hooks, CreateChangeSender.Factory createChangeSenderFactory, ProjectCache projectCache, AllProjectsNameProvider allProjects, + ChangesCollection changes, Provider setParent, @Assisted("projectName") Project.NameKey projectName, @@ -109,11 +110,11 @@ public class ReviewProjectAccess extends ProjectAccessHandler { this.user = user; this.patchSetInfoFactory = patchSetInfoFactory; this.reviewersProvider = reviewersProvider; - this.changeFactory = changeFactory; this.mergeabilityChecker = mergeabilityChecker; this.hooks = hooks; this.createChangeSenderFactory = createChangeSenderFactory; this.projectCache = projectCache; + this.changes = changes; } @Override @@ -165,9 +166,15 @@ public class ReviewProjectAccess extends ProjectAccessHandler { } catch (Exception err) { log.error("Cannot send email for new change " + change.getId(), err); } - addProjectOwnersAsReviewers(change); + ChangeResource rsrc; + try { + rsrc = changes.parse(changeId); + } catch (ResourceNotFoundException e) { + throw new IOException(e); + } + addProjectOwnersAsReviewers(rsrc); if (parentProjectUpdate) { - addAdministratorsAsReviewers(change); + addAdministratorsAsReviewers(rsrc); } return changeId; } @@ -186,12 +193,10 @@ public class ReviewProjectAccess extends ProjectAccessHandler { db.patchSetAncestors().insert(toInsert); } - private void addProjectOwnersAsReviewers(Change change) { + private void addProjectOwnersAsReviewers(ChangeResource rsrc) { final String projectOwners = groupBackend.get(SystemGroupBackend.PROJECT_OWNERS).getName(); try { - ChangeResource rsrc = - new ChangeResource(changeFactory.controlFor(change, user)); AddReviewerInput input = new AddReviewerInput(); input.reviewer = projectOwners; reviewersProvider.get().apply(rsrc, input); @@ -201,15 +206,13 @@ public class ReviewProjectAccess extends ProjectAccessHandler { } } - private void addAdministratorsAsReviewers(Change change) { + private void addAdministratorsAsReviewers(ChangeResource rsrc) { List adminRules = projectCache.getAllProjects().getConfig() .getAccessSection(AccessSection.GLOBAL_CAPABILITIES) .getPermission(GlobalCapability.ADMINISTRATE_SERVER).getRules(); for (PermissionRule r : adminRules) { try { - ChangeResource rsrc = - new ChangeResource(changeFactory.controlFor(change, user)); AddReviewerInput input = new AddReviewerInput(); input.reviewer = r.getGroup().getUUID().get(); reviewersProvider.get().apply(rsrc, input); 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 64c350580a..6adefa8d87 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 @@ -133,11 +133,12 @@ public class ChangeJson { private final IdentifiedUser.GenericFactory userFactory; private final ProjectControl.GenericFactory projectControlFactory; private final PatchSetInfoFactory patchSetInfoFactory; + private final ChangesCollection changes; private final FileInfoJson fileInfoJson; private final AccountInfo.Loader.Factory accountLoaderFactory; private final DynamicMap downloadSchemes; private final DynamicMap downloadCommands; - private final DynamicMap> changes; + private final DynamicMap> changeViews; private final Revisions revisions; private final PatchListCache patchListCache; @@ -156,11 +157,12 @@ public class ChangeJson { IdentifiedUser.GenericFactory uf, ProjectControl.GenericFactory pcf, PatchSetInfoFactory psi, + ChangesCollection changes, FileInfoJson fileInfoJson, AccountInfo.Loader.Factory ailf, DynamicMap downloadSchemes, DynamicMap downloadCommands, - DynamicMap> changes, + DynamicMap> changeViews, Revisions revisions, PatchListCache patchListCache) { this.db = db; @@ -170,11 +172,12 @@ public class ChangeJson { this.userFactory = uf; this.projectControlFactory = pcf; this.patchSetInfoFactory = psi; + this.changes = changes; this.fileInfoJson = fileInfoJson; this.accountLoaderFactory = ailf; this.downloadSchemes = downloadSchemes; this.downloadCommands = downloadCommands; - this.changes = changes; + this.changeViews = changeViews; this.revisions = revisions; this.patchListCache = patchListCache; @@ -322,8 +325,8 @@ public class ChangeJson { if (has(CURRENT_ACTIONS) && userProvider.get().isIdentifiedUser()) { out.actions = Maps.newTreeMap(); for (UiAction.Description d : UiActions.from( - changes, - new ChangeResource(control(cd)), + changeViews, + changes.parse(control(cd)), userProvider)) { out.actions.put(d.getId(), new ActionInfo(d)); } @@ -821,7 +824,7 @@ public class ChangeJson { out.actions = Maps.newTreeMap(); for (UiAction.Description d : UiActions.from( revisions, - new RevisionResource(new ChangeResource(control(cd)), in), + new RevisionResource(changes.parse(control(cd)), in), userProvider)) { out.actions.put(d.getId(), new ActionInfo(d)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java index b0562a9d22..37553b21e6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java @@ -35,7 +35,7 @@ public class ChangeResource implements RestResource, HasETag { private final ChangeControl control; - public ChangeResource(ChangeControl control) { + ChangeResource(ChangeControl control) { this.control = control; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java index b7d8819a48..f7038549df 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java @@ -83,6 +83,16 @@ public class ChangesCollection implements return new ChangeResource(control); } + public ChangeResource parse(Change.Id id) + throws ResourceNotFoundException, OrmException { + return parse(TopLevelResource.INSTANCE, + IdString.fromUrl(Integer.toString(id.get()))); + } + + public ChangeResource parse(ChangeControl control) throws OrmException { + return new ChangeResource(control); + } + private List findChanges(String id) throws OrmException, ResourceNotFoundException { // Try legacy id diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java index c1f8598482..6c1202fde2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java @@ -37,7 +37,6 @@ import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.WorkQueue.Executor; import com.google.gerrit.server.index.ChangeIndexer; -import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gwtorm.server.OrmException; @@ -64,8 +63,8 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener { private final ThreadLocalRequestContext tl; private final SchemaFactory schemaFactory; - private final ChangeControl.GenericFactory changeControlFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory; + private final ChangesCollection changes; private final Provider mergeable; private final ChangeIndexer indexer; private final ListeningExecutorService executor; @@ -75,16 +74,15 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener { @Inject public MergeabilityChecker(ThreadLocalRequestContext tl, SchemaFactory schemaFactory, - ChangeControl.GenericFactory changeControlFactory, IdentifiedUser.GenericFactory identifiedUserFactory, - Provider mergeable, ChangeIndexer indexer, - @MergeabilityChecksExecutor Executor executor, + ChangesCollection changes, Provider mergeable, + ChangeIndexer indexer, @MergeabilityChecksExecutor Executor executor, MergeabilityCheckQueue mergeabilityCheckQueue, MetaDataUpdate.Server metaDataUpdateFactory) { this.tl = tl; this.schemaFactory = schemaFactory; - this.changeControlFactory = changeControlFactory; this.identifiedUserFactory = identifiedUserFactory; + this.changes = changes; this.mergeable = mergeable; this.indexer = indexer; this.executor = MoreExecutors.listeningDecorator(executor); @@ -263,8 +261,8 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener { PatchSet ps = db.patchSets().get(change.currentPatchSetId()); Mergeable m = mergeable.get(); m.setForce(force); - MergeableInfo info = m.apply(new RevisionResource(new ChangeResource( - changeControlFactory.controlFor(change, context.getCurrentUser())), ps)); + MergeableInfo info = m.apply( + new RevisionResource(changes.parse(change.getId()), ps)); return change.isMergeable() != info.mergeable; } catch (ResourceConflictException e) { // change is closed diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index 2ee3ad0d9d..70936afd51 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -73,6 +73,7 @@ public class PostReview implements RestModifyView } private final Provider db; + private final ChangesCollection changes; private final ChangeIndexer indexer; private final AccountsCollection accounts; private final EmailReviewComments.Factory email; @@ -87,11 +88,13 @@ public class PostReview implements RestModifyView @Inject PostReview(Provider db, + ChangesCollection changes, ChangeIndexer indexer, AccountsCollection accounts, EmailReviewComments.Factory email, ChangeHooks hooks) { this.db = db; + this.changes = changes; this.indexer = indexer; this.accounts = accounts; this.email = email; @@ -193,7 +196,7 @@ public class PostReview implements RestModifyView } ChangeControl target = caller.forUser(accounts.parse(in.onBehalfOf)); - return new RevisionResource(new ChangeResource(target), rev.getPatchSet()); + return new RevisionResource(changes.parse(target), rev.getPatchSet()); } private void checkLabels(RevisionResource revision, boolean strict, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index b7b34b1135..293aeb444c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -69,7 +69,7 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.change.ChangeInserter; -import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.change.MergeabilityChecker; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.change.PatchSetInserter.ChangeKind; @@ -269,6 +269,7 @@ public class ReceiveCommits { private final CommitValidators.Factory commitValidatorsFactory; private final TagCache tagCache; private final AccountCache accountCache; + private final ChangesCollection changes; private final ChangeInserter.Factory changeInserterFactory; private final WorkQueue workQueue; private final ListeningExecutorService changeUpdateExector; @@ -329,6 +330,7 @@ public class ReceiveCommits { final TagCache tagCache, final AccountCache accountCache, final ChangeCache changeCache, + final ChangesCollection changes, final ChangeInserter.Factory changeInserterFactory, final CommitValidators.Factory commitValidatorsFactory, @CanonicalWebUrl final String canonicalWebUrl, @@ -364,6 +366,7 @@ public class ReceiveCommits { this.canonicalWebUrl = canonicalWebUrl; this.tagCache = tagCache; this.accountCache = accountCache; + this.changes = changes; this.changeInserterFactory = changeInserterFactory; this.commitValidatorsFactory = commitValidatorsFactory; this.workQueue = workQueue; @@ -1561,7 +1564,7 @@ public class ReceiveCommits { private void submit(ChangeControl changeCtl, PatchSet ps) throws OrmException, IOException { Submit submit = submitProvider.get(); - RevisionResource rsrc = new RevisionResource(new ChangeResource(changeCtl), ps); + RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps); Change c = submit.submit(rsrc, currentUser); if (c == null) { addError("Submitting change " + changeCtl.getChange().getChangeId() diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 14a8637281..ec485bba29 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -42,7 +42,7 @@ import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeJson.ChangeInfo; -import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.change.PostReview; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.project.ChangeControl; @@ -79,6 +79,7 @@ public abstract class AbstractQueryChangesTest { @Inject protected AccountManager accountManager; @Inject protected ChangeControl.GenericFactory changeControlFactory; @Inject protected ChangeInserter.Factory changeFactory; + @Inject protected ChangesCollection changes; @Inject protected CreateProject.Factory projectFactory; @Inject protected IdentifiedUser.RequestFactory userFactory; @Inject protected InMemoryDatabase schemaFactory; @@ -365,7 +366,7 @@ public abstract class AbstractQueryChangesTest { input.message = "toplevel"; input.labels = ImmutableMap. of("Code-Review", (short) 1); postReview.apply(new RevisionResource( - new ChangeResource(ctl), ins.getPatchSet()), input); + changes.parse(ctl), ins.getPatchSet()), input); assertTrue(query("label:Code-Review=-2").isEmpty()); assertTrue(query("label:Code-Review-2").isEmpty()); @@ -479,7 +480,7 @@ public abstract class AbstractQueryChangesTest { ReviewInput input = new ReviewInput(); input.message = "toplevel"; postReview.apply(new RevisionResource( - new ChangeResource(ctl1), ins1.getPatchSet()), input); + changes.parse(ctl1), ins1.getPatchSet()), input); change1 = db.changes().get(change1.getId()); assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2)); @@ -512,7 +513,7 @@ public abstract class AbstractQueryChangesTest { ReviewInput input = new ReviewInput(); input.message = "toplevel"; postReview.apply(new RevisionResource( - new ChangeResource(ctl1), ins1.getPatchSet()), input); + changes.parse(ctl1), ins1.getPatchSet()), input); change1 = db.changes().get(change1.getId()); assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2)); @@ -610,7 +611,7 @@ public abstract class AbstractQueryChangesTest { input.comments = ImmutableMap.> of( "Foo.java", ImmutableList. of(comment)); postReview.apply(new RevisionResource( - new ChangeResource(ctl), ins.getPatchSet()), input); + changes.parse(ctl), ins.getPatchSet()), input); assertTrue(query("comment:foo").isEmpty()); assertResultEquals(change, queryOne("comment:toplevel")); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java index 9a201338a0..06f2a79410 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java @@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.ChangesCollection; import com.google.gerrit.server.change.DeleteReviewer; import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.change.ReviewerResource; @@ -88,6 +89,9 @@ public class SetReviewersCommand extends SshCommand { @Inject private ChangeControl.Factory changeControlFactory; + @Inject + private ChangesCollection changesCollection; + private Set toRemove = new HashSet(); private Set changes = new HashSet(); @@ -110,8 +114,7 @@ public class SetReviewersCommand extends SshCommand { } private boolean modifyOne(Change.Id changeId) throws Exception { - ChangeResource changeRsrc = - new ChangeResource(changeControlFactory.validateFor(changeId)); + ChangeResource changeRsrc = changesCollection.parse(changeId); boolean ok = true; // Remove reviewers