Ensure reader always exists for VersionedMetaData.onLoad()

This simplifies implementations of onLoad() that may want to depend on
reader not being null. Since createInMemory() resulted in a null
reader, remove that method and replace it with usages of
InMemoryRepository (which is easy enough since it was only used in
tests).

Change-Id: I8b534ef041ff7f3f54b0e744d36e99a0c8c1bb24
This commit is contained in:
Dave Borowitz
2013-12-05 15:44:21 -08:00
parent cb8fc9b4b5
commit 826a7b6dda
6 changed files with 49 additions and 45 deletions

View File

@@ -2,10 +2,8 @@ CONSTANTS_SRC = [
'src/main/java/com/google/gerrit/server/documentation/Constants.java', 'src/main/java/com/google/gerrit/server/documentation/Constants.java',
] ]
SRCS = glob([ SRCS = glob(
'src/main/java/**/*.java', ['src/main/java/**/*.java'],
'src/test/java/com/google/gerrit/server/project/Util.java',
],
excludes = CONSTANTS_SRC, excludes = CONSTANTS_SRC,
) )
RESOURCES = glob(['src/main/resources/**/*']) RESOURCES = glob(['src/main/resources/**/*'])
@@ -80,7 +78,10 @@ java_sources(
visibility = ['PUBLIC'], visibility = ['PUBLIC'],
) )
TESTUTIL = glob(['src/test/java/com/google/gerrit/testutil/**/*.java']) TESTUTIL = glob([
'src/test/java/com/google/gerrit/testutil/**/*.java',
'src/test/java/com/google/gerrit/server/project/Util.java',
])
java_library( java_library(
name = 'testutil', name = 'testutil',
srcs = TESTUTIL, srcs = TESTUTIL,
@@ -129,11 +130,13 @@ java_test(
deps = [ deps = [
':prolog_test_case', ':prolog_test_case',
':server', ':server',
':testutil',
'//gerrit-common:server', '//gerrit-common:server',
'//gerrit-reviewdb:server', '//gerrit-reviewdb:server',
'//gerrit-server/src/main/prolog:common', '//gerrit-server/src/main/prolog:common',
'//lib:gwtorm', '//lib:gwtorm',
'//lib:junit', '//lib:junit',
'//lib/jgit:jgit',
'//lib/guice:guice', '//lib/guice:guice',
'//lib/prolog:prolog-cafe', '//lib/prolog:prolog-cafe',
], ],

View File

@@ -71,18 +71,6 @@ public abstract class VersionedMetaData {
return revision != null ? revision.copy() : null; return revision != null ? revision.copy() : null;
} }
/** Initialize in-memory as though the repository branch doesn't exist. */
public void createInMemory() {
try {
revision = null;
onLoad();
} catch (IOException err) {
throw new RuntimeException("Unexpected IOException", err);
} catch (ConfigInvalidException err) {
throw new RuntimeException("Unexpected ConfigInvalidException", err);
}
}
/** /**
* Load the current version from the branch. * Load the current version from the branch.
* <p> * <p>
@@ -116,19 +104,13 @@ public abstract class VersionedMetaData {
*/ */
public void load(Repository db, ObjectId id) throws IOException, public void load(Repository db, ObjectId id) throws IOException,
ConfigInvalidException { ConfigInvalidException {
if (id != null) { reader = db.newObjectReader();
reader = db.newObjectReader(); try {
try { revision = id != null ? new RevWalk(reader).parseCommit(id) : null;
revision = new RevWalk(reader).parseCommit(id);
onLoad();
} finally {
reader.release();
reader = null;
}
} else {
// The branch does not yet exist.
revision = null;
onLoad(); onLoad();
} finally {
reader.release();
reader = null;
} }
} }

View File

@@ -15,19 +15,20 @@
package com.google.gerrit.rules; package com.google.gerrit.rules;
import static com.google.gerrit.common.data.Permission.LABEL; import static com.google.gerrit.common.data.Permission.LABEL;
import static com.google.gerrit.server.project.Util.value;
import static com.google.gerrit.server.project.Util.category; import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.server.project.Util.grant;
import static com.google.gerrit.server.project.Util.value;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
import com.google.inject.AbstractModule; import com.google.inject.AbstractModule;
import org.junit.Before; import org.junit.Before;
@@ -67,7 +68,7 @@ public class GerritCommonTest extends PrologTestCase {
}); });
local = new ProjectConfig(localKey); local = new ProjectConfig(localKey);
local.createInMemory(); local.load(InMemoryRepositoryManager.newRepository(localKey));
Q.setRefPatterns(Arrays.asList("refs/heads/develop")); Q.setRefPatterns(Arrays.asList("refs/heads/develop"));
local.getLabelSections().put(V.getName(), V); local.getLabelSections().put(V.getName(), V);

View File

@@ -27,6 +27,7 @@ import static com.google.gerrit.server.project.Util.ADMIN;
import static com.google.gerrit.server.project.Util.DEVS; import static com.google.gerrit.server.project.Util.DEVS;
import static com.google.gerrit.server.project.Util.doNotInherit; import static com.google.gerrit.server.project.Util.doNotInherit;
import static com.google.gerrit.server.project.Util.grant; import static com.google.gerrit.server.project.Util.grant;
import static com.google.gerrit.testutil.InMemoryRepositoryManager.newRepository;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
@@ -61,7 +62,7 @@ public class RefControlTest {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
local = new ProjectConfig(localKey); local = new ProjectConfig(localKey);
local.createInMemory(); local.load(newRepository(localKey));
util.add(local); util.add(local);
} }
@@ -157,14 +158,14 @@ public class RefControlTest {
} }
@Test @Test
public void testInheritDuplicateSections() { public void testInheritDuplicateSections() throws Exception {
grant(util.getParentConfig(), READ, ADMIN, "refs/*"); grant(util.getParentConfig(), READ, ADMIN, "refs/*");
grant(local, READ, DEVS, "refs/heads/*"); grant(local, READ, DEVS, "refs/heads/*");
local.getProject().setParentName(util.getParentConfig().getProject().getName()); local.getProject().setParentName(util.getParentConfig().getProject().getName());
assertTrue("a can read", util.user(local, "a", ADMIN).isVisible()); assertTrue("a can read", util.user(local, "a", ADMIN).isVisible());
local = new ProjectConfig(new Project.NameKey("local")); local = new ProjectConfig(new Project.NameKey("local"));
local.createInMemory(); local.load(newRepository(localKey));
grant(local, READ, DEVS, "refs/*"); grant(local, READ, DEVS, "refs/*");
assertTrue("d can read", util.user(local, "d", DEVS).isVisible()); assertTrue("d can read", util.user(local, "d", DEVS).isVisible());
} }

View File

@@ -41,10 +41,13 @@ import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
import com.google.inject.Guice; import com.google.inject.Guice;
import com.google.inject.Injector; import com.google.inject.Injector;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
@@ -116,16 +119,22 @@ public class Util {
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final CapabilityControl.Factory capabilityControlFactory; private final CapabilityControl.Factory capabilityControlFactory;
private final PermissionCollection.Factory sectionSorter; private final PermissionCollection.Factory sectionSorter;
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjectsName = new AllProjectsName("parent"); private final AllProjectsName allProjectsName = new AllProjectsName("parent");
private final ProjectConfig parent = new ProjectConfig(allProjectsName); private final ProjectConfig parent = new ProjectConfig(allProjectsName);
public Util() { public Util() {
all = new HashMap<Project.NameKey, ProjectState>(); all = new HashMap<Project.NameKey, ProjectState>();
parent.createInMemory(); repoManager = new InMemoryRepositoryManager();
parent.getLabelSections().put(CR.getName(), CR); try {
Repository repo = repoManager.createRepository(allProjectsName);
add(parent); parent.load(repo);
parent.getLabelSections().put(CR.getName(), CR);
add(parent);
} catch (IOException | ConfigInvalidException e) {
throw new RuntimeException(e);
}
projectCache = new ProjectCache() { projectCache = new ProjectCache() {
@Override @Override
@@ -199,15 +208,19 @@ public class Util {
public void add(ProjectConfig pc) { public void add(ProjectConfig pc) {
PrologEnvironment.Factory envFactory = null; PrologEnvironment.Factory envFactory = null;
GitRepositoryManager mgr = null;
ProjectControl.AssistedFactory projectControlFactory = null; ProjectControl.AssistedFactory projectControlFactory = null;
RulesCache rulesCache = null; RulesCache rulesCache = null;
SitePaths sitePaths = null; SitePaths sitePaths = null;
List<CommentLinkInfo> commentLinks = null; List<CommentLinkInfo> commentLinks = null;
try {
repoManager.createRepository(pc.getProject().getNameKey());
} catch (IOException e) {
throw new RuntimeException(e);
}
all.put(pc.getProject().getNameKey(), new ProjectState(sitePaths, all.put(pc.getProject().getNameKey(), new ProjectState(sitePaths,
projectCache, allProjectsName, projectControlFactory, envFactory, mgr, projectCache, allProjectsName, projectControlFactory, envFactory,
rulesCache, commentLinks, pc)); repoManager, rulesCache, commentLinks, pc));
} }
public ProjectControl user(ProjectConfig local, AccountGroup.UUID... memberOf) { public ProjectControl user(ProjectConfig local, AccountGroup.UUID... memberOf) {

View File

@@ -31,6 +31,10 @@ import java.util.SortedSet;
/** Repository manager that uses in-memory repositories. */ /** Repository manager that uses in-memory repositories. */
public class InMemoryRepositoryManager implements GitRepositoryManager { public class InMemoryRepositoryManager implements GitRepositoryManager {
public static InMemoryRepository newRepository(Project.NameKey name) {
return new Repo(name);
}
private static class Description extends DfsRepositoryDescription { private static class Description extends DfsRepositoryDescription {
private String desc; private String desc;