opendev/gerrit
Code Issues Proposed changes

Merge "Remove GitRepositoryManager#openMetadataRepository"

Browse Source
This commit is contained in:
Dave Borowitz
2016-04-11 15:36:56 +00:00
committed by Gerrit Code Review
parent b50cd86eff 5c0f0825a3
commit 78cb56c393
19 changed files with 36 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -1074,7 +1074,7 @@ public class ChangeIT extends AbstractDaemonTest {
pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
"b.txt", "4711", r.getChangeId()).to("refs/for/master").assertOkStatus();
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
try (Repository repo = repoManager.openMetadataRepository(project);
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
RevCommit commitPatchSetCreation = rw.parseCommit(
repo.exactRef(ChangeNoteUtil.changeRefName(new Change.Id(c._number)))

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -119,7 +119,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
assume().that(notesMigration.enabled()).isTrue();
ChangeInfo c = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
try (Repository repo = repoManager.openMetadataRepository(project);
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(
repo.exactRef(ChangeNoteUtil.changeRefName(new Change.Id(c._number)))

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java
View File

@@ -107,7 +107,7 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
private Ref getDraftRef(TestAccount account, Change.Id changeId)
throws Exception {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
return repo.exactRef(RefNames.refsDraftComments(account.id, changeId));
}
}

6
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
View File

@@ -294,7 +294,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
private void assertChangeUpToDate(boolean expected, Change.Id id)
throws Exception {
try (Repository repo = repoManager.openMetadataRepository(project)) {
try (Repository repo = repoManager.openRepository(project)) {
Change c = unwrapDb().changes().get(id);
assertThat(c).isNotNull();
assertThat(c.getNoteDbState()).isNotNull();
@@ -305,7 +305,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
private void assertDraftsUpToDate(boolean expected, Change.Id changeId,
TestAccount account) throws Exception {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
Change c = unwrapDb().changes().get(changeId);
assertThat(c).isNotNull();
assertThat(c.getNoteDbState()).isNotNull();
@@ -316,7 +316,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
}
private ObjectId getMetaRef(Project.NameKey p, String name) throws Exception {
try (Repository repo = repoManager.openMetadataRepository(p)) {
try (Repository repo = repoManager.openRepository(p)) {
Ref ref = repo.exactRef(name);
return ref != null ? ref.getObjectId() : null;
}

3
gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
View File

@@ -133,8 +133,7 @@ public class RebuildNoteDb extends SiteProgram {
getChangesByProject();
AtomicBoolean ok = new AtomicBoolean(true);
Stopwatch sw = Stopwatch.createStarted();
try (Repository allUsersRepo =
repoManager.openMetadataRepository(allUsersName)) {
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
deleteRefs(RefNames.REFS_DRAFT_COMMENTS, allUsersRepo);
for (Project.NameKey project : changesByProject.keySet()) {
try {

6
gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
View File

@@ -311,7 +311,7 @@ public class PatchLineCommentsUtil {
public void deleteAllDraftsFromAllUsers(Change.Id changeId)
throws IOException {
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
for (Ref ref : getDraftRefs(repo, changeId).values()) {
@@ -373,7 +373,7 @@ public class PatchLineCommentsUtil {
}
private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {
@@ -383,7 +383,7 @@ public class PatchLineCommentsUtil {
public Map<String, Ref> getDraftRefs(Change.Id changeId)
throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
return getDraftRefs(repo, changeId);
} catch (IOException e) {
throw new OrmException(e);

8
gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
View File

@@ -87,7 +87,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
RefUpdate u = repo.updateRef(
RefNames.refsStarredChanges(accountId, changeId));
@@ -136,7 +136,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
RefUpdate u = repo.updateRef(
RefNames.refsStarredChanges(accountId, changeId));
@@ -174,7 +174,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
batchUpdate.setAllowNonFastForwards(true);
@@ -251,7 +251,7 @@ public class StarredChangesUtil {
}
private Set<String> getRefNames(String prefix) throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {

20
gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java
View File

@@ -46,9 +46,6 @@ public interface GitRepositoryManager {
/**
* Create (and open) a repository by name.
* <p>
* If the implementation supports separate metadata repositories, this method
* must also create the metadata repository, but does not open it.
*
* @param name the repository name, relative to the base directory.
* @return the cached Repository instance. Caller must call {@code close()}
@@ -62,23 +59,6 @@ public interface GitRepositoryManager {
throws RepositoryCaseMismatchException, RepositoryNotFoundException,
IOException;
/**
* Open the repository storing metadata for the given project.
* <p>
* This includes any project-specific metadata <em>except</em> what is stored
* in {@code refs/meta/config}. Implementations may choose to store all
* metadata in the original project.
*
* @param name the base project name name.
* @return the cached metadata Repository instance. Caller must call
* {@code close()} when done to decrement the resource handle.
* @throws RepositoryNotFoundException the name does not denote an existing
* repository.
* @throws IOException the name cannot be read as a repository.
*/
Repository openMetadataRepository(Project.NameKey name)
throws RepositoryNotFoundException, IOException;
/** @return set of all known projects, sorted by natural NameKey order. */
SortedSet<Project.NameKey> list();

33
gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
View File

@@ -14,16 +14,12 @@
package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkState;
import com.google.common.base.MoreObjects;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -124,23 +120,17 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
}
private final Path basePath;
private final NotesMigration notesMigration;
private final Path noteDbPath;
private final Lock namesUpdateLock;
private volatile SortedSet<Project.NameKey> names = new TreeSet<>();
@Inject
LocalDiskRepositoryManager(SitePaths site,
@GerritServerConfig Config cfg,
NotesMigration notesMigration) {
this.notesMigration = notesMigration;
@GerritServerConfig Config cfg) {
basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
noteDbPath = site.resolve(MoreObjects.firstNonNull(
cfg.getString("gerrit", null, "noteDbPath"), "notedb"));
namesUpdateLock = new ReentrantLock(true /* fair */);
}
@@ -213,15 +203,7 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
@Override
public Repository createRepository(Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException {
Repository repo = createRepository(getBasePath(name), name);
if (notesMigration.writeChanges() && !noteDbPath.equals(basePath)) {
createRepository(noteDbPath, name);
}
return repo;
}
private Repository createRepository(Path path, Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException {
Path path = getBasePath(name);
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
@@ -276,17 +258,6 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
}
}
@Override
public Repository openMetadataRepository(Project.NameKey name)
throws RepositoryNotFoundException, IOException {
checkState(notesMigration.readChanges(), "NoteDb disabled");
try {
return openRepository(noteDbPath, name);
} catch (RepositoryNotFoundException e) {
return createRepository(noteDbPath, name);
}
}
private void onCreateProject(final Project.NameKey newProjectName) {
namesUpdateLock.lock();
try {

4
gerrit-server/src/main/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.Config;
@@ -48,9 +47,8 @@ public class MultiBaseLocalDiskRepositoryManager extends
@Inject
MultiBaseLocalDiskRepositoryManager(SitePaths site,
@GerritServerConfig Config cfg,
NotesMigration notesMigration,
RepositoryConfig config) {
super(site, cfg, notesMigration);
super(site, cfg);
this.config = config;
for (Path alternateBasePath : config.getAllBasePaths()) {

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
View File

@@ -125,8 +125,7 @@ public abstract class AbstractChangeNotes<T> {
return self();
}
try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
Repository repo =
args.repoManager.openMetadataRepository(getProjectName());
Repository repo = args.repoManager.openRepository(getProjectName());
LoadHandle handle = openHandle(repo)) {
revision = handle.id();
onLoad(handle);
@@ -155,8 +154,7 @@ public abstract class AbstractChangeNotes<T> {
} else if (!args.migration.enabled()) {
return null;
}
try (Repository repo =
args.repoManager.openMetadataRepository(getProjectName())) {
try (Repository repo = args.repoManager.openRepository(getProjectName())) {
Ref ref = repo.getRefDatabase().exactRef(getRefName());
return ref != null ? ref.getObjectId() : null;
} catch (IOException e) {

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -134,7 +134,7 @@ class ChangeNotesParser implements AutoCloseable {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.repo = repoManager.openMetadataRepository(project);
this.repo = repoManager.openRepository(project);
this.noteUtil = noteUtil;
this.metrics = metrics;
approvals = Maps.newHashMap();

16
gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
View File

@@ -55,8 +55,8 @@ import java.util.Set;
/**
* Object to manage a single sequence of updates to NoteDb.
* <p>
* Instances are one-time-use. Handles updating both the change meta repo and
* the All-Users meta repo for any affected changes, with proper ordering.
* Instances are one-time-use. Handles updating both the change repo and the
* All-Users repo for any affected changes, with proper ordering.
* <p>
* To see the state that would be applied prior to executing the full sequence
* of updates, use {@link #stage()}.
@@ -161,26 +161,24 @@ public class NoteDbUpdateManager {
private void initCodeRepo() throws IOException {
if (codeRepo == null) {
codeRepo = openRepo(projectName, false);
codeRepo = openRepo(projectName);
}
}
private void initChangeRepo() throws IOException {
if (changeRepo == null) {
changeRepo = openRepo(projectName, true);
changeRepo = openRepo(projectName);
}
}
private void initAllUsersRepo() throws IOException {
if (allUsersRepo == null) {
allUsersRepo = openRepo(allUsersName, true);
allUsersRepo = openRepo(allUsersName);
}
}
private OpenRepo openRepo(Project.NameKey p, boolean meta) throws IOException {
Repository repo = meta
? repoManager.openMetadataRepository(p)
: repoManager.openRepository(p);
private OpenRepo openRepo(Project.NameKey p) throws IOException {
Repository repo = repoManager.openRepository(p);
ObjectInserter ins = repo.newObjectInserter();
return new OpenRepo(repo, new RevWalk(ins.newReader()), ins,
new ChainedReceiveCommands(), true);

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java
View File

@@ -129,7 +129,7 @@ public class RepoSequence {
}
private void acquire() throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(projectName);
try (Repository repo = repoManager.openRepository(projectName);
RevWalk rw = new RevWalk(repo)) {
TryAcquire attempt = new TryAcquire(repo, rw);
RefUpdate.Result result = retryer.call(attempt);

8
gerrit-server/src/test/java/com/google/gerrit/server/git/LocalDiskRepositoryManagerTest.java
View File

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.testutil.TempFileUtil;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.StandardKeyEncoder;
@@ -53,16 +52,13 @@ public class LocalDiskRepositoryManagerTest extends EasyMockSupport {
site.resolve("git").toFile().mkdir();
cfg = new Config();
cfg.setString("gerrit", null, "basePath", "git");
repoManager =
new LocalDiskRepositoryManager(site, cfg,
createNiceMock(NotesMigration.class));
repoManager = new LocalDiskRepositoryManager(site, cfg);
repoManager.start();
}
@Test(expected = IllegalStateException.class)
public void testThatNullBasePathThrowsAnException() {
new LocalDiskRepositoryManager(site, new Config(),
createNiceMock(NotesMigration.class));
new LocalDiskRepositoryManager(site, new Config());
}
@Test

9
gerrit-server/src/test/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java
View File

@@ -23,7 +23,6 @@ import static org.easymock.EasyMock.reset;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.testutil.TempFileUtil;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.StandardKeyEncoder;
@@ -66,11 +65,8 @@ public class MultiBaseLocalDiskRepositoryManagerTest {
configMock = createNiceMock(RepositoryConfig.class);
expect(configMock.getAllBasePaths()).andReturn(new ArrayList<Path>()).anyTimes();
replay(configMock);
NotesMigration notesMigrationMock = createNiceMock(NotesMigration.class);
replay(notesMigrationMock);
repoManager =
new MultiBaseLocalDiskRepositoryManager(site, cfg,
notesMigrationMock, configMock);
new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock);
}
@After
@@ -188,7 +184,6 @@ public class MultiBaseLocalDiskRepositoryManagerTest {
.andReturn(Arrays.asList(Paths.get("repos"))).anyTimes();
replay(configMock);
repoManager =
new MultiBaseLocalDiskRepositoryManager(site, cfg,
createNiceMock(NotesMigration.class), configMock);
new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock);
}
}

6
gerrit-server/src/test/java/com/google/gerrit/server/notedb/RepoSequenceTest.java
View File

@@ -160,7 +160,7 @@ public class RepoSequenceTest {
@Test
public void failOnWrongType() throws Exception {
try (Repository repo = repoManager.openMetadataRepository(project)) {
try (Repository repo = repoManager.openRepository(project)) {
TestRepository<Repository> tr = new TestRepository<>(repo);
tr.branch(RefNames.REFS_SEQUENCES + "id").commit().create();
try {
@@ -206,7 +206,7 @@ public class RepoSequenceTest {
private ObjectId writeBlob(String sequenceName, String value) {
String refName = RefNames.REFS_SEQUENCES + sequenceName;
try (Repository repo = repoManager.openMetadataRepository(project);
try (Repository repo = repoManager.openRepository(project);
ObjectInserter ins = repo.newObjectInserter()) {
ObjectId newId = ins.insert(OBJ_BLOB, value.getBytes(UTF_8));
ins.flush();
@@ -222,7 +222,7 @@ public class RepoSequenceTest {
private String readBlob(String sequenceName) throws Exception {
String refName = RefNames.REFS_SEQUENCES + sequenceName;
try (Repository repo = repoManager.openMetadataRepository(project);
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
ObjectId id = repo.exactRef(refName).getObjectId();
return new String(rw.getObjectReader().open(id).getCachedBytes(), UTF_8);

6
gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
View File

@@ -88,12 +88,6 @@ public class InMemoryRepositoryManager implements GitRepositoryManager {
return repo;
}
@Override
public synchronized Repo openMetadataRepository(
Project.NameKey name) throws RepositoryNotFoundException {
return openRepository(name);
}
@Override
public synchronized SortedSet<Project.NameKey> list() {
SortedSet<Project.NameKey> names = Sets.newTreeSet();

2
gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java
View File

@@ -117,7 +117,7 @@ public class NoteDbChecker {
public void assertNoChangeRef(Project.NameKey project, Change.Id changeId)
throws Exception {
try (Repository repo = repoManager.openMetadataRepository(project)) {
try (Repository repo = repoManager.openRepository(project)) {
assertThat(repo.exactRef(ChangeNoteUtil.changeRefName(changeId)))
.isNull();
}