diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java index 2dbd33bdb8..3b3dbb8a0c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java +++ b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java @@ -48,7 +48,6 @@ import java.util.Map; public final class StoredValues { public static final StoredValue REVIEW_DB = create(ReviewDb.class); - public static final StoredValue CHANGE = create(Change.class); public static final StoredValue CHANGE_DATA = create(ChangeData.class); public static final StoredValue PATCH_SET = create(PatchSet.class); public static final StoredValue CHANGE_CONTROL = create(ChangeControl.class); @@ -56,7 +55,7 @@ public final class StoredValues { public static final StoredValue PATCH_SET_INFO = new StoredValue() { @Override public PatchSetInfo createValue(Prolog engine) { - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); PatchSet ps = StoredValues.PATCH_SET.get(engine); PrologEnvironment env = (PrologEnvironment) engine.control; PatchSetInfoFactory patchInfoFactory = @@ -75,7 +74,7 @@ public final class StoredValues { PrologEnvironment env = (PrologEnvironment) engine.control; PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine); PatchListCache plCache = env.getArgs().getPatchListCache(); - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); Project.NameKey projectKey = change.getProject(); ObjectId a = null; ObjectId b = ObjectId.fromString(psInfo.getRevId()); @@ -96,7 +95,7 @@ public final class StoredValues { public Repository createValue(Prolog engine) { PrologEnvironment env = (PrologEnvironment) engine.control; GitRepositoryManager gitMgr = env.getArgs().getGitRepositoryManager(); - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); Project.NameKey projectKey = change.getProject(); final Repository repo; try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java index 3b1191c2d4..a4946ebc3a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java @@ -43,6 +43,7 @@ public class AccessControlModule extends FactoryModule { .annotatedWith(GitReceivePackGroups.class) // .toProvider(GitReceivePackGroupsProvider.class).in(SINGLETON); + factory(ChangeControl.AssistedFactory.class); factory(ProjectControl.AssistedFactory.class); } } 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 a868e615d4..f73a2f70ee 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 @@ -35,6 +35,8 @@ 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.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; import com.googlecode.prolog_cafe.lang.IntegerTerm; import com.googlecode.prolog_cafe.lang.ListTerm; @@ -167,6 +169,10 @@ public class ChangeControl { } } + interface AssistedFactory { + ChangeControl create(RefControl refControl, Change change); + } + /** * Exception thrown when the label term of a submit record * unexpectedly didn't contain a user term. @@ -181,18 +187,25 @@ public class ChangeControl { } private final ApprovalsUtil approvalsUtil; + private final ChangeData.Factory changeDataFactory; private final RefControl refControl; private final Change change; - ChangeControl(ApprovalsUtil au, RefControl r, Change c) { - this.approvalsUtil = au; - this.refControl = r; - this.change = c; + @AssistedInject + ChangeControl( + ApprovalsUtil approvalsUtil, + ChangeData.Factory changeDataFactory, + @Assisted RefControl refControl, + @Assisted Change change) { + this.approvalsUtil = approvalsUtil; + this.changeDataFactory = changeDataFactory; + this.refControl = refControl; + this.change = change; } public ChangeControl forUser(final CurrentUser who) { - return new ChangeControl(approvalsUtil, getRefControl().forUser(who), - getChange()); + return new ChangeControl(approvalsUtil, changeDataFactory, + getRefControl().forUser(who), change); } public RefControl getRefControl() { @@ -327,9 +340,7 @@ public class ChangeControl { public boolean isReviewer(ReviewDb db, @Nullable ChangeData cd) throws OrmException { if (getCurrentUser().isIdentifiedUser()) { - Collection results = cd != null - ? cd.reviewers().values() - : approvalsUtil.getReviewers(db, change.getId()).values(); + Collection results = changeData(db, cd).reviewers().values(); IdentifiedUser user = (IdentifiedUser) getCurrentUser(); return results.contains(user.getAccountId()); } @@ -410,6 +421,7 @@ public class ChangeControl { return ruleError("Patch set " + patchSet.getPatchSetId() + " is not current"); } + cd = changeData(db, cd); if ((change.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) && !allowDraft) { return cannotSubmitDraft(db, patchSet, cd); @@ -449,7 +461,7 @@ public class ChangeControl { } private List cannotSubmitDraft(ReviewDb db, PatchSet patchSet, - ChangeData cd) { + @Nullable ChangeData cd) { try { if (!isDraftVisible(db, cd)) { return ruleError("Patch set " + patchSet.getPatchSetId() + " not found"); @@ -555,6 +567,7 @@ public class ChangeControl { public SubmitTypeRecord getSubmitTypeRecord(ReviewDb db, PatchSet patchSet, @Nullable ChangeData cd) { + cd = changeData(db, cd); try { if (change.getStatus() == Change.Status.DRAFT && !isDraftVisible(db, cd)) { return typeRuleError("Patch set " + patchSet.getPatchSetId() @@ -657,6 +670,10 @@ public class ChangeControl { return rec; } + private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) { + return cd != null ? cd : changeDataFactory.create(db, change); + } + private void appliedBy(SubmitRecord.Label label, Term status) throws UserTermExpected { if (status.isStructure() && status.arity() == 1) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java index 5a9bb7bf58..f71c4c9f73 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java @@ -30,7 +30,6 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.InternalUser; @@ -146,7 +145,7 @@ public class ProjectControl { private final CurrentUser user; private final ProjectState state; private final GitRepositoryManager repoManager; - private final ApprovalsUtil approvalsUtil; + private final ChangeControl.AssistedFactory changeControlFactory; private final PermissionCollection.Factory permissionFilter; private final Collection contributorAgreements; @@ -162,12 +161,12 @@ public class ProjectControl { ProjectCache pc, PermissionCollection.Factory permissionFilter, GitRepositoryManager repoManager, - ApprovalsUtil approvalsUtil, + ChangeControl.AssistedFactory changeControlFactory, @CanonicalWebUrl @Nullable String canonicalWebUrl, @Assisted CurrentUser who, @Assisted ProjectState ps) { this.repoManager = repoManager; - this.approvalsUtil = approvalsUtil; + this.changeControlFactory = changeControlFactory; this.uploadGroups = uploadGroups; this.receiveGroups = receiveGroups; this.permissionFilter = permissionFilter; @@ -185,8 +184,7 @@ public class ProjectControl { } public ChangeControl controlFor(final Change change) { - return new ChangeControl(approvalsUtil, controlForRef(change.getDest()), - change); + return changeControlFactory.create(controlForRef(change.getDest()), change); } public RefControl controlForRef(Branch.NameKey ref) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java index 2ae420149e..f7c3c9dcb8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java @@ -14,8 +14,9 @@ package com.google.gerrit.server.project; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.collect.Lists; -import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -70,7 +71,7 @@ public class SubmitRuleEvaluator { */ public SubmitRuleEvaluator(ReviewDb db, PatchSet patchSet, ProjectControl projectControl, - ChangeControl changeControl, Change change, @Nullable ChangeData cd, + ChangeControl changeControl, Change change, ChangeData cd, boolean fastEvalLabels, String userRuleLocatorName, String userRuleWrapperName, String filterRuleLocatorName, String filterRuleWrapperName) { @@ -95,7 +96,7 @@ public class SubmitRuleEvaluator { */ public SubmitRuleEvaluator(ReviewDb db, PatchSet patchSet, ProjectControl projectControl, - ChangeControl changeControl, Change change, @Nullable ChangeData cd, + ChangeControl changeControl, Change change, ChangeData cd, boolean fastEvalLabels, String userRuleLocatorName, String userRuleWrapperName, String filterRuleLocatorName, String filterRuleWrapperName, @@ -105,7 +106,7 @@ public class SubmitRuleEvaluator { this.projectControl = projectControl; this.changeControl = changeControl; this.change = change; - this.cd = cd; + this.cd = checkNotNull(cd, "ChangeData"); this.fastEvalLabels = fastEvalLabels; this.userRuleLocatorName = userRuleLocatorName; this.userRuleWrapperName = userRuleWrapperName; @@ -182,7 +183,6 @@ public class SubmitRuleEvaluator { + getProjectName(), err); } env.set(StoredValues.REVIEW_DB, db); - env.set(StoredValues.CHANGE, change); env.set(StoredValues.CHANGE_DATA, cd); env.set(StoredValues.PATCH_SET, patchSet); env.set(StoredValues.CHANGE_CONTROL, changeControl); diff --git a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java index 8cabd55587..8e9126232e 100644 --- a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java +++ b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java @@ -4,9 +4,7 @@ package gerrit; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; -import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.StoredValues; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; @@ -40,20 +38,11 @@ class PRED__load_commit_labels_1 extends Predicate.P1 { Term listHead = Prolog.Nil; try { - ReviewDb db = StoredValues.REVIEW_DB.get(engine); - PatchSet patchSet = StoredValues.PATCH_SET.get(engine); - ChangeData cd = StoredValues.CHANGE_DATA.getOrNull(engine); + ChangeData cd = StoredValues.CHANGE_DATA.get(engine); LabelTypes types = StoredValues.CHANGE_CONTROL.get(engine).getLabelTypes(); - Iterable approvals; - if (cd != null) { - approvals = cd.currentApprovals(); - } else { - approvals = db.patchSetApprovals().byPatchSet(patchSet.getId()); - } - - for (PatchSetApproval a : approvals) { + for (PatchSetApproval a : cd.currentApprovals()) { if (a.getValue() == 0) { continue; } diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java index 51396ed9c4..67c2c93d80 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java +++ b/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java @@ -36,7 +36,7 @@ public class PRED_change_branch_1 extends Predicate.P1 { engine.setB0(); Term a1 = arg1.dereference(); - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); Branch.NameKey name = change.getDest(); if (!a1.unify(SymbolTerm.create(name.get()), engine.trail)) { @@ -44,4 +44,4 @@ public class PRED_change_branch_1 extends Predicate.P1 { } return cont; } -} \ No newline at end of file +} diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java index b127fff254..efe1c82dcf 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java +++ b/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java @@ -40,7 +40,7 @@ public class PRED_change_owner_1 extends Predicate.P1 { engine.setB0(); Term a1 = arg1.dereference(); - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); Account.Id ownerId = change.getOwner(); if (!a1.unify(new StructureTerm(user, new IntegerTerm(ownerId.get())), engine.trail)) { @@ -48,4 +48,4 @@ public class PRED_change_owner_1 extends Predicate.P1 { } return cont; } -} \ No newline at end of file +} diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java index fb9f86533a..9a1d8b6da9 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java +++ b/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java @@ -36,7 +36,7 @@ public class PRED_change_project_1 extends Predicate.P1 { engine.setB0(); Term a1 = arg1.dereference(); - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); Project.NameKey name = change.getProject(); if (!a1.unify(SymbolTerm.create(name.get()), engine.trail)) { @@ -44,4 +44,4 @@ public class PRED_change_project_1 extends Predicate.P1 { } return cont; } -} \ No newline at end of file +} diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java index 34885f9141..e6748dea74 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java +++ b/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java @@ -36,7 +36,7 @@ public class PRED_change_topic_1 extends Predicate.P1 { Term a1 = arg1.dereference(); Term topicTerm = Prolog.Nil; - Change change = StoredValues.CHANGE.get(engine); + Change change = StoredValues.CHANGE_DATA.get(engine).getChange(); String topic = change.getTopic(); if (topic != null) { topicTerm = SymbolTerm.create(topic); @@ -47,4 +47,4 @@ public class PRED_change_topic_1 extends Predicate.P1 { } return cont; } -} \ No newline at end of file +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java index cf7ace25c1..d5db854673 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java @@ -85,7 +85,6 @@ public class GerritCommonTest extends PrologTestCase { new Account.Id(2), new Branch.NameKey(localKey, "refs/heads/master"), TimeUtil.nowTs()); - env.set(StoredValues.CHANGE, change); env.set(StoredValues.CHANGE_CONTROL, util.user(local).controlFor(change)); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java index 8680af6a02..6e59b7cf12 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java @@ -31,7 +31,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project.NameKey; import com.google.gerrit.rules.PrologEnvironment; import com.google.gerrit.rules.RulesCache; -import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupMembership; @@ -42,9 +41,12 @@ import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.patch.PatchListCache; +import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.testutil.InMemoryRepositoryManager; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.util.Providers; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; @@ -119,6 +121,7 @@ public class Util { private final Map all; private final ProjectCache projectCache; private final CapabilityControl.Factory capabilityControlFactory; + private final ChangeControl.AssistedFactory changeControlFactory; private final PermissionCollection.Factory sectionSorter; private final GitRepositoryManager repoManager; @@ -190,8 +193,13 @@ public class Util { protected void configure() { bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance( new Config()); + bind(GitRepositoryManager.class).toInstance(repoManager); + bind(PatchListCache.class) + .toProvider(Providers. of(null)); factory(CapabilityControl.Factory.class); + factory(ChangeControl.AssistedFactory.class); + factory(ChangeData.Factory.class); bind(ProjectCache.class).toInstance(projectCache); } }); @@ -201,6 +209,8 @@ public class Util { sectionSorter = new PermissionCollection.Factory(new SectionSortCache(c)); capabilityControlFactory = injector.getInstance(CapabilityControl.Factory.class); + changeControlFactory = + injector.getInstance(ChangeControl.AssistedFactory.class); } public ProjectConfig getParentConfig() { @@ -234,7 +244,7 @@ public class Util { return new ProjectControl(Collections. emptySet(), Collections. emptySet(), projectCache, - sectionSorter, null, new ApprovalsUtil(), canonicalWebUrl, + sectionSorter, null, changeControlFactory, canonicalWebUrl, new MockUser(name, memberOf), newProjectState(local)); }