From d4967d7b90ef08d74d1349671b77b64a973c1710 Mon Sep 17 00:00:00 2001 From: Alice Kober-Sotzek Date: Wed, 31 Jan 2018 13:41:36 +0100 Subject: [PATCH] Simplify BatchUpdateTest and CommitsCollectionTest Both of these tests use exactly the setup provided by InMemoryTestEnvironment. Use that JUnit rule to avoid duplicate code and improve readability of those classes. The previous code in BatchUpdateTest and CommitsCollectionTest closed the created repository after use. This shouldn't be necessary as the Javadoc of InMemoryRepository states that closing the repository has no impact on its memory/GC. Change-Id: I64f62a3acb38b18030f00ebb41b662be7c602900 --- .../testing/InMemoryTestEnvironment.java | 4 +- .../server/project/CommitsCollectionTest.java | 99 ++++++------------- .../gerrit/server/update/BatchUpdateTest.java | 85 +++------------- 3 files changed, 46 insertions(+), 142 deletions(-) diff --git a/java/com/google/gerrit/testing/InMemoryTestEnvironment.java b/java/com/google/gerrit/testing/InMemoryTestEnvironment.java index bffbcd6fda..cebd139612 100644 --- a/java/com/google/gerrit/testing/InMemoryTestEnvironment.java +++ b/java/com/google/gerrit/testing/InMemoryTestEnvironment.java @@ -121,7 +121,9 @@ public final class InMemoryTestEnvironment implements MethodRule { schemaCreator.create(underlyingDb); } db = schemaFactory.open(); - setApiUser(accountManager.authenticate(AuthRequest.forUser("user")).getAccountId()); + + // The first user is added to the "Administrators" group. See AccountManager#create(). + setApiUser(accountManager.authenticate(AuthRequest.forUser("admin")).getAccountId()); // Inject target members after setting API user, so it can @Inject a ReviewDb if it wants. injector.injectMembers(target); diff --git a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java index 267c62263b..e3aec4586d 100644 --- a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java +++ b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java @@ -19,118 +19,61 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.common.collect.ImmutableList; import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GlobalCapability; +import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.data.Permission; -import com.google.gerrit.lifecycle.LifecycleManager; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; 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.account.GroupCache; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.project.testing.Util; import com.google.gerrit.server.restapi.project.CommitsCollection; -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.testing.InMemoryDatabase; -import com.google.gerrit.testing.InMemoryModule; import com.google.gerrit.testing.InMemoryRepositoryManager; -import com.google.inject.Guice; +import com.google.gerrit.testing.InMemoryTestEnvironment; 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; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; -import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; /** Unit tests for {@link CommitsCollection}. */ public class CommitsCollectionTest { + @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); + @Inject private AccountManager accountManager; - @Inject private IdentifiedUser.GenericFactory userFactory; - @Inject private InMemoryDatabase schemaFactory; @Inject private InMemoryRepositoryManager repoManager; - @Inject private SchemaCreator schemaCreator; - @Inject private ThreadLocalRequestContext requestContext; @Inject protected ProjectCache projectCache; @Inject protected MetaDataUpdate.Server metaDataUpdateFactory; @Inject protected AllProjectsName allProjects; - @Inject protected GroupCache groupCache; @Inject private CommitsCollection commits; - private LifecycleManager lifecycle; - private ReviewDb db; private TestRepository repo; private ProjectConfig project; - private IdentifiedUser user; - private AccountGroup.UUID admins; @Before public void setUp() throws Exception { - Injector injector = Guice.createInjector(new InMemoryModule()); - injector.injectMembers(this); - lifecycle = new LifecycleManager(); - lifecycle.add(injector); - lifecycle.start(); - - db = schemaFactory.open(); - schemaCreator.create(db); - // Need to create at least one user to be admin before creating a "normal" - // registered user. - // See AccountManager#create(). - accountManager.authenticate(AuthRequest.forUser("admin")).getAccountId(); - admins = groupCache.get(new AccountGroup.NameKey("Administrators")).orElse(null).getGroupUUID(); setUpPermissions(); - Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId(); - user = userFactory.create(userId); + Account.Id user = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId(); + testEnvironment.setApiUser(user); Project.NameKey name = new Project.NameKey("project"); InMemoryRepository inMemoryRepo = repoManager.createRepository(name); 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 - public void tearDown() { - if (repo != null) { - repo.getRepository().close(); - } - if (lifecycle != null) { - lifecycle.stop(); - } - requestContext.setContext(null); - if (db != null) { - db.close(); - } - InMemoryDatabase.drop(schemaFactory); } @Test @@ -256,6 +199,8 @@ public class CommitsCollectionTest { } private void setUpPermissions() throws Exception { + ImmutableList admins = getAdmins(); + // Remove read permissions for all users besides admin, because by default // Anonymous user group has ALLOW READ permission in refs/*. // This method is idempotent, so is safe to call on every test setup. @@ -263,6 +208,24 @@ public class CommitsCollectionTest { for (AccessSection sec : pc.getAccessSections()) { sec.removePermission(Permission.READ); } - allow(pc, Permission.READ, admins, "refs/*"); + for (AccountGroup.UUID admin : admins) { + allow(pc, Permission.READ, admin, "refs/*"); + } + } + + private ImmutableList getAdmins() { + Permission adminPermission = + projectCache + .getAllProjects() + .getConfig() + .getAccessSection(AccessSection.GLOBAL_CAPABILITIES) + .getPermission(GlobalCapability.ADMINISTRATE_SERVER); + + return adminPermission + .getRules() + .stream() + .map(PermissionRule::getGroup) + .map(GroupReference::getUUID) + .collect(ImmutableList.toImmutableList()); } } diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java index 2b235a15ec..67672d3116 100644 --- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java +++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java @@ -17,98 +17,37 @@ package com.google.gerrit.server.update; import static org.junit.Assert.assertEquals; import com.google.gerrit.common.TimeUtil; -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.schema.SchemaCreator; -import com.google.gerrit.server.util.RequestContext; -import com.google.gerrit.server.util.ThreadLocalRequestContext; -import com.google.gerrit.testing.InMemoryDatabase; -import com.google.gerrit.testing.InMemoryModule; -import com.google.gerrit.testing.InMemoryRepositoryManager; -import com.google.gwtorm.server.SchemaFactory; -import com.google.inject.Guice; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.testing.InMemoryTestEnvironment; 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; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; -import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; public class BatchUpdateTest { - @Inject private AccountManager accountManager; - @Inject private IdentifiedUser.GenericFactory userFactory; - @Inject private SchemaFactory schemaFactory; - @Inject private InMemoryRepositoryManager repoManager; - @Inject private SchemaCreator schemaCreator; - @Inject private ThreadLocalRequestContext requestContext; + @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); + + @Inject private GitRepositoryManager repoManager; @Inject private BatchUpdate.Factory batchUpdateFactory; + @Inject private ReviewDb db; + @Inject private Provider user; - // Only for use in setting up/tearing down injector; other users should use schemaFactory. - @Inject private InMemoryDatabase inMemoryDatabase; - - private LifecycleManager lifecycle; - private ReviewDb db; - private TestRepository repo; private Project.NameKey project; - private IdentifiedUser user; + private TestRepository repo; @Before public void setUp() throws Exception { - Injector injector = Guice.createInjector(new InMemoryModule()); - injector.injectMembers(this); - lifecycle = new LifecycleManager(); - lifecycle.add(injector); - lifecycle.start(); - - try (ReviewDb underlyingDb = inMemoryDatabase.getDatabase().open()) { - schemaCreator.create(underlyingDb); - } - db = schemaFactory.open(); - Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId(); - user = userFactory.create(userId); - project = new Project.NameKey("test"); - InMemoryRepository inMemoryRepo = repoManager.createRepository(project); + Repository inMemoryRepo = repoManager.createRepository(project); repo = new TestRepository<>(inMemoryRepo); - - requestContext.setContext( - new RequestContext() { - @Override - public CurrentUser getUser() { - return user; - } - - @Override - public Provider getReviewDbProvider() { - return Providers.of(db); - } - }); - } - - @After - public void tearDown() { - if (repo != null) { - repo.getRepository().close(); - } - if (lifecycle != null) { - lifecycle.stop(); - } - requestContext.setContext(null); - if (db != null) { - db.close(); - } - InMemoryDatabase.drop(inMemoryDatabase); } @Test @@ -116,7 +55,7 @@ public class BatchUpdateTest { final RevCommit masterCommit = repo.branch("master").commit().create(); final RevCommit branchCommit = repo.branch("branch").commit().parent(masterCommit).create(); - try (BatchUpdate bu = batchUpdateFactory.create(db, project, user, TimeUtil.nowTs())) { + try (BatchUpdate bu = batchUpdateFactory.create(db, project, user.get(), TimeUtil.nowTs())) { bu.addRepoOnlyOp( new RepoOnlyOp() { @Override