diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java index b6539f1331..d701afa160 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -119,7 +119,7 @@ public class BatchProgramModule extends FactoryModule { bind(new TypeLiteral>() {}) .annotatedWith(GitReceivePackGroups.class) .toInstance(Collections. emptySet()); - factory(ChangeControl.AssistedFactory.class); + bind(ChangeControl.Factory.class); factory(ProjectControl.AssistedFactory.class); install(new BatchGitModule()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 646817e269..f51eeb10c8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -351,7 +351,7 @@ public class ChangeUtil { // Try isolated changeId if (!id.contains("~")) { - return asChangeControls(query.byKeyPrefix(id)); + return asChangeControls(query.byKeyPrefix(id), user); } // Try change triplet @@ -359,17 +359,18 @@ public class ChangeUtil { if (triplet.isPresent()) { return asChangeControls(query.byBranchKey( triplet.get().branch(), - triplet.get().id())); + triplet.get().id()), + user); } return Collections.emptyList(); } - private List asChangeControls(List cds) - throws OrmException { + private List asChangeControls(List cds, + CurrentUser user) throws OrmException { List ctls = new ArrayList<>(cds.size()); for (ChangeData cd : cds) { - ctls.add(cd.changeControl(user.get())); + ctls.add(cd.changeControl(user)); } return ctls; } 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 97a377e057..f6045c168a 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 @@ -103,7 +103,7 @@ public class ChangeEdits implements @Override public ChangeEditResource parse(ChangeResource rsrc, IdString id) throws ResourceNotFoundException, AuthException, IOException, - InvalidChangeOperationException { + InvalidChangeOperationException, OrmException { Optional edit = editUtil.byChange(rsrc.getChange()); if (!edit.isPresent()) { throw new ResourceNotFoundException(id); @@ -556,7 +556,7 @@ public class ChangeEdits implements @Override public BinaryResult apply(ChangeResource rsrc) throws AuthException, - IOException, ResourceNotFoundException { + IOException, ResourceNotFoundException, OrmException { Optional edit = editUtil.byChange(rsrc.getChange()); if (edit.isPresent()) { String msg = edit.get().getEditCommit().getFullMessage(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java index 495e6954e0..7c1e959822 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.change.DeleteChangeEdit.Input; import com.google.gerrit.server.edit.ChangeEdit; import com.google.gerrit.server.edit.ChangeEditUtil; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -41,7 +42,8 @@ public class DeleteChangeEdit implements RestModifyView { @Override public Response apply(ChangeResource rsrc, Input input) - throws AuthException, ResourceNotFoundException, IOException { + throws AuthException, ResourceNotFoundException, IOException, + OrmException { Optional edit = editUtil.byChange(rsrc.getChange()); if (edit.isPresent()) { editUtil.delete(edit.get()); 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 d6b0f7c978..c8db0edd79 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 @@ -138,7 +138,7 @@ public class Revisions implements ChildCollection loadEdit(ChangeResource change, RevId revid) - throws AuthException, IOException { + throws AuthException, IOException, OrmException { Optional edit = editUtil.byChange(change.getChange()); if (edit.isPresent()) { PatchSet ps = new PatchSet(new PatchSet.Id(change.getId(), 0)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 9ba959c8cd..7650480b15 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -114,9 +114,10 @@ public class ChangeEditUtil { * @return edit for this change for this user, if present. * @throws AuthException * @throws IOException + * @throws OrmException */ public Optional byChange(Change change) - throws AuthException, IOException { + throws AuthException, IOException, OrmException { try { return byChange(changeControlFactory.controlFor(change, user.get())); } catch (NoSuchChangeException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java index 097ab6ea7b..af9e243857 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java @@ -32,6 +32,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -89,9 +90,10 @@ public class LabelNormalizer { * included in the output, nor are approvals where the user has no * permissions for that label. * @throws NoSuchChangeException + * @throws OrmException */ - public Result normalize(Change change, - Collection approvals) throws NoSuchChangeException { + public Result normalize(Change change, Collection approvals) + throws NoSuchChangeException, OrmException { return normalize( changeFactory.controlFor(change, userFactory.create(change.getOwner())), approvals); 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 744f57230d..b7ca69d4ef 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 @@ -219,7 +219,7 @@ public class PatchScriptFactory implements Callable { } private ObjectId toObjectId(PatchSet ps) throws NoSuchChangeException, - AuthException, NoSuchChangeException, IOException { + AuthException, NoSuchChangeException, IOException, OrmException { if (ps.getId().get() == 0) { return getEditRev(); } @@ -236,7 +236,7 @@ public class PatchScriptFactory implements Callable { } private ObjectId getEditRev() throws AuthException, - NoSuchChangeException, IOException { + NoSuchChangeException, IOException, OrmException { edit = editReader.byChange(change); if (edit.isPresent()) { return edit.get().getRef().getObjectId(); 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 29ab220197..bc35d279c2 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 @@ -37,7 +37,7 @@ public class AccessControlModule extends FactoryModule { .annotatedWith(GitReceivePackGroups.class) // .toProvider(GitReceivePackGroupsProvider.class).in(SINGLETON); - factory(ChangeControl.AssistedFactory.class); + bind(ChangeControl.Factory.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 1c9365d82a..8b5695e725 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,8 +35,6 @@ 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 java.io.IOException; import java.util.Collection; @@ -65,7 +63,7 @@ public class ChangeControl { } public ChangeControl controlFor(Change change, CurrentUser user) - throws NoSuchChangeException { + throws NoSuchChangeException, OrmException { final Project.NameKey projectKey = change.getProject(); try { return projectControl.controlFor(projectKey, user).controlFor(change); @@ -106,34 +104,45 @@ public class ChangeControl { } } - public interface AssistedFactory { - ChangeControl create(RefControl refControl, Change change); - ChangeControl create(RefControl refControl, ChangeNotes notes); + public static class Factory { + private final ReviewDb db; + private final ChangeData.Factory changeDataFactory; + private final ChangeNotes.Factory notesFactory; + private final ApprovalsUtil approvalsUtil; + + @Inject + Factory(ReviewDb db, + ChangeData.Factory changeDataFactory, + ChangeNotes.Factory notesFactory, + ApprovalsUtil approvalsUtil) { + this.db = db; + this.changeDataFactory = changeDataFactory; + this.notesFactory = notesFactory; + this.approvalsUtil = approvalsUtil; + } + + @SuppressWarnings("unused") + ChangeControl create(RefControl refControl, Change change) + throws OrmException { + return create(refControl, notesFactory.create(db, change)); + } + + ChangeControl create(RefControl refControl, ChangeNotes notes) { + return new ChangeControl(changeDataFactory, approvalsUtil, refControl, + notes); + } } private final ChangeData.Factory changeDataFactory; + private final ApprovalsUtil approvalsUtil; private final RefControl refControl; private final ChangeNotes notes; - private final ApprovalsUtil approvalsUtil; - @AssistedInject - ChangeControl( - ChangeData.Factory changeDataFactory, - ChangeNotes.Factory notesFactory, - ApprovalsUtil approvalsUtil, - ReviewDb db, - @Assisted RefControl refControl, - @Assisted Change change) { - this(changeDataFactory, approvalsUtil, refControl, - notesFactory.create(db, change)); - } - - @AssistedInject ChangeControl( ChangeData.Factory changeDataFactory, ApprovalsUtil approvalsUtil, - @Assisted RefControl refControl, - @Assisted ChangeNotes notes) { + RefControl refControl, + ChangeNotes notes) { this.changeDataFactory = changeDataFactory; this.approvalsUtil = approvalsUtil; this.refControl = refControl; 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 02a1d45739..367dc1464e 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 @@ -44,6 +44,7 @@ import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; @@ -149,7 +150,7 @@ public class ProjectControl { private final CurrentUser user; private final ProjectState state; private final GitRepositoryManager repoManager; - private final ChangeControl.AssistedFactory changeControlFactory; + private final ChangeControl.Factory changeControlFactory; private final PermissionCollection.Factory permissionFilter; private final Collection contributorAgreements; private final TagCache tagCache; @@ -167,7 +168,7 @@ public class ProjectControl { ProjectCache pc, PermissionCollection.Factory permissionFilter, GitRepositoryManager repoManager, - ChangeControl.AssistedFactory changeControlFactory, + ChangeControl.Factory changeControlFactory, TagCache tagCache, ChangeCache changeCache, @CanonicalWebUrl @Nullable String canonicalWebUrl, @@ -193,7 +194,7 @@ public class ProjectControl { return r; } - public ChangeControl controlFor(final Change change) { + public ChangeControl controlFor(Change change) throws OrmException { return changeControlFactory.create(controlForRef(change.getDest()), change); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java index 0bd4f512de..f29a3510fd 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java @@ -25,17 +25,21 @@ import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.reviewdb.client.Account; 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.account.AccountManager; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.schema.SchemaCreator; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gerrit.testutil.InMemoryModule; import com.google.gerrit.testutil.InMemoryRepositoryManager; import com.google.inject.Guice; import com.google.inject.Inject; import com.google.inject.Injector; +import com.google.inject.Provider; import com.google.inject.util.Providers; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; @@ -55,6 +59,7 @@ public class ProjectControlTest { @Inject private InMemoryRepositoryManager repoManager; @Inject private ProjectControl.GenericFactory projectControlFactory; @Inject private SchemaCreator schemaCreator; + @Inject private ThreadLocalRequestContext requestContext; private LifecycleManager lifecycle; private ReviewDb db; @@ -81,6 +86,18 @@ public class ProjectControlTest { project = new ProjectConfig(name); project.load(inMemoryRepo); repo = new TestRepository<>(inMemoryRepo); + + requestContext.setContext(new RequestContext() { + @Override + public CurrentUser getUser() { + return user; + } + + @Override + public Provider getReviewDbProvider() { + return Providers.of(db); + } + }); } @After @@ -91,6 +108,7 @@ public class ProjectControlTest { if (lifecycle != null) { lifecycle.stop(); } + requestContext.setContext(null); if (db != null) { db.close(); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 2e5d22ef5f..e8c62e93c0 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -40,7 +40,6 @@ import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.errors.InvalidNameException; -import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountProjectWatch; import com.google.gerrit.reviewdb.client.Change; @@ -49,43 +48,29 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.PrologEnvironment; import com.google.gerrit.rules.RulesCache; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.StarredChangesUtil; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.CapabilityControl; -import com.google.gerrit.server.account.FakeRealm; -import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupMembership; import com.google.gerrit.server.account.ListGroupMembership; -import com.google.gerrit.server.account.Realm; -import com.google.gerrit.server.change.ChangeKindCache; -import com.google.gerrit.server.change.ChangeKindCacheImpl; -import com.google.gerrit.server.change.MergeabilityCache; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsNameProvider; -import com.google.gerrit.server.config.AnonymousCowardName; -import com.google.gerrit.server.config.AnonymousCowardNameProvider; -import com.google.gerrit.server.config.CanonicalWebUrl; -import com.google.gerrit.server.config.CanonicalWebUrlProvider; -import com.google.gerrit.server.config.DisableReverseDnsLookup; -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.MergeUtil; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.group.SystemGroupBackend; -import com.google.gerrit.server.patch.PatchListCache; -import com.google.gerrit.server.query.change.ChangeData; -import com.google.gerrit.testutil.FakeAccountCache; +import com.google.gerrit.server.schema.SchemaCreator; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; +import com.google.gerrit.testutil.InMemoryDatabase; +import com.google.gerrit.testutil.InMemoryModule; import com.google.gerrit.testutil.InMemoryRepositoryManager; import com.google.inject.Guice; +import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Provider; import com.google.inject.util.Providers; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -245,8 +230,13 @@ public class RefControlTest { private InMemoryRepositoryManager repoManager; private ProjectCache projectCache; private PermissionCollection.Factory sectionSorter; - private CapabilityControl.Factory capabilityControlFactory; - private ChangeControl.AssistedFactory changeControlFactory; + private ChangeControl.Factory changeControlFactory; + private ReviewDb db; + + @Inject private CapabilityControl.Factory capabilityControlFactory; + @Inject private SchemaCreator schemaCreator; + @Inject private InMemoryDatabase schemaFactory; + @Inject private ThreadLocalRequestContext requestContext; @Before public void setUp() throws Exception { @@ -317,43 +307,11 @@ public class RefControlTest { } }; - Injector injector = Guice.createInjector(new FactoryModule() { - @SuppressWarnings({"rawtypes", "unchecked"}) - @Override - protected void configure() { - Provider nullProvider = Providers.of(null); - bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance( - new Config()); - bind(ReviewDb.class).toProvider(nullProvider); - bind(GitRepositoryManager.class).toInstance(repoManager); - bind(PatchListCache.class).toProvider(nullProvider); - bind(Realm.class).to(FakeRealm.class); + Injector injector = Guice.createInjector(new InMemoryModule()); + injector.injectMembers(this); - factory(CapabilityControl.Factory.class); - factory(ChangeControl.AssistedFactory.class); - factory(ChangeData.Factory.class); - factory(MergeUtil.Factory.class); - bind(ProjectCache.class).toInstance(projectCache); - bind(AccountCache.class).toInstance(new FakeAccountCache()); - bind(GroupBackend.class).to(SystemGroupBackend.class); - bind(String.class).annotatedWith(CanonicalWebUrl.class) - .toProvider(CanonicalWebUrlProvider.class); - bind(Boolean.class).annotatedWith(DisableReverseDnsLookup.class) - .toInstance(Boolean.FALSE); - bind(String.class).annotatedWith(AnonymousCowardName.class) - .toProvider(AnonymousCowardNameProvider.class); - bind(ChangeKindCache.class).to(ChangeKindCacheImpl.NoCache.class); - bind(MergeabilityCache.class) - .to(MergeabilityCache.NotImplemented.class); - bind(StarredChangesUtil.class) - .toProvider(Providers. of(null)); - } - }); - - capabilityControlFactory = - injector.getInstance(CapabilityControl.Factory.class); - changeControlFactory = - injector.getInstance(ChangeControl.AssistedFactory.class); + db = schemaFactory.open(); + schemaCreator.create(db); Cache c = CacheBuilder.newBuilder().build(); @@ -367,6 +325,29 @@ public class RefControlTest { local.load(newRepository(localKey)); add(local); local.getProject().setParentName(parentKey); + + requestContext.setContext(new RequestContext() { + @Override + public CurrentUser getUser() { + return null; + } + + @Override + public Provider getReviewDbProvider() { + return Providers.of(db); + } + }); + + changeControlFactory = injector.getInstance(ChangeControl.Factory.class); + } + + @After + public void tearDown() { + requestContext.setContext(null); + if (db != null) { + db.close(); + } + InMemoryDatabase.drop(schemaFactory); } @Test