opendev/gerrit
Code Issues Proposed changes

Refactor GitRepositoryManager to be an interface

Browse Source 
This permits us to easily mock it with EasyMock.  The implementation
that relies on the local filesystem API within JGit has been renamed
to LocalDiskRepositoryManager.

Change-Id: Ib47a2a22f4f6564e1b592a8476c3bc699f4e3b2e
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-12-19 12:06:42 -08:00
parent a06dc11ffe
commit fbcea4cf7a
6 changed files with 226 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitWebServlet.java
View File

@@ -34,7 +34,7 @@ import com.google.gerrit.httpd.GitWebConfig;
import com.google.gerrit.reviewdb.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.inject.Inject;
@@ -77,12 +77,12 @@ class GitWebServlet extends HttpServlet {
private final Set<String> deniedActions;
private final int bufferSize = 8192;
private final File gitwebCgi;
private final GitRepositoryManager repoManager;
private final LocalDiskRepositoryManager repoManager;
private final ProjectControl.Factory projectControl;
private final EnvList _env;
@Inject
GitWebServlet(final GitRepositoryManager repoManager,
GitWebServlet(final LocalDiskRepositoryManager repoManager,
final ProjectControl.Factory projectControl,
final SitePaths site, final GerritConfig gerritConfig,
final GitWebConfig gitWebConfig) throws IOException {

11
gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java
View File

@@ -30,6 +30,7 @@ import com.google.gerrit.pgm.util.IoUtil;
import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.gerrit.server.git.GitProjectImporter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.schema.SchemaUpdater;
@@ -176,13 +177,17 @@ public class Init extends SiteProgram {
void importGit() throws OrmException, IOException {
if (flags.importProjects) {
System.err.println("Scanning " + repositoryManager.getBasePath());
gitProjectImporter.run(new GitProjectImporter.Messages() {
@Override
public void warning(String msg) {
public void info(String msg) {
System.err.println(msg);
System.err.flush();
}
@Override
public void warning(String msg) {
info(msg);
}
});
}
}
@@ -247,7 +252,7 @@ public class Init extends SiteProgram {
bind(ConsoleUI.class).toInstance(init.ui);
bind(InitFlags.class).toInstance(init.flags);
bind(GitRepositoryManager.class);
bind(GitRepositoryManager.class).to(LocalDiskRepositoryManager.class);
bind(GitProjectImporter.class);
}
});

5
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -37,6 +37,7 @@ import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.auth.ldap.LdapModule;
import com.google.gerrit.server.cache.CachePool;
import com.google.gerrit.server.git.ChangeMergeQueue;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue;
@@ -111,7 +112,7 @@ public class GerritGlobalModule extends FactoryModule {
factory(AccountInfoCacheFactory.Factory.class);
factory(ProjectState.Factory.class);
bind(GitRepositoryManager.class);
bind(GitRepositoryManager.class).to(LocalDiskRepositoryManager.class);
bind(FileTypeRegistry.class).to(MimeUtilFileTypeRegistry.class);
bind(WorkQueue.class);
@@ -141,7 +142,7 @@ public class GerritGlobalModule extends FactoryModule {
install(new LifecycleModule() {
@Override
protected void configure() {
listener().to(GitRepositoryManager.Lifecycle.class);
listener().to(LocalDiskRepositoryManager.Lifecycle.class);
listener().to(CachePool.Lifecycle.class);
listener().to(WorkQueue.Lifecycle.class);
}

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

@@ -32,15 +32,16 @@ import java.util.Set;
/** Imports all projects found within the repository manager. */
public class GitProjectImporter {
public interface Messages {
void info(String msg);
void warning(String msg);
}
private final GitRepositoryManager repositoryManager;
private final LocalDiskRepositoryManager repositoryManager;
private final SchemaFactory<ReviewDb> schema;
private Messages messages;
@Inject
GitProjectImporter(final GitRepositoryManager repositoryManager,
GitProjectImporter(final LocalDiskRepositoryManager repositoryManager,
final SchemaFactory<ReviewDb> schema) {
this.repositoryManager = repositoryManager;
this.schema = schema;
@@ -48,6 +49,7 @@ public class GitProjectImporter {
public void run(final Messages msg) throws OrmException, IOException {
messages = msg;
messages.info("Scanning " + repositoryManager.getBasePath());
final ReviewDb db = schema.open();
try {
final HashSet<String> have = new HashSet<String>();

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

@@ -1,4 +1,4 @@
// Copyright (C) 2008 The Android Open Source Project
// Copyright (C) 2009 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -14,75 +14,22 @@
package com.google.gerrit.server.git;
import com.google.gerrit.lifecycle.LifecycleListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.LockFile;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.RepositoryCache;
import org.eclipse.jgit.lib.WindowCache;
import org.eclipse.jgit.lib.WindowCacheConfig;
import org.eclipse.jgit.lib.RepositoryCache.FileKey;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
/** Class managing Git repositories. */
@Singleton
public class GitRepositoryManager {
private static final Logger log =
LoggerFactory.getLogger(GitRepositoryManager.class);
private static final String UNNAMED =
"Unnamed repository; edit this file to name it for gitweb.";
public static class Lifecycle implements LifecycleListener {
private final Config cfg;
@Inject
Lifecycle(@GerritServerConfig final Config cfg) {
this.cfg = cfg;
}
@Override
public void start() {
final WindowCacheConfig c = new WindowCacheConfig();
c.fromConfig(cfg);
WindowCache.reconfigure(c);
}
@Override
public void stop() {
}
}
private final File basePath;
@Inject
GitRepositoryManager(final SitePaths site,
@GerritServerConfig final Config cfg) {
basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
}
/** @return base directory under which all projects are stored. */
public File getBasePath() {
return basePath;
}
/**
* Manages Git repositories for the Gerrit server process.
* <p>
* Implementations of this interface should be a {@link Singleton} and
* registered in Guice so they are globally available within the server
* environment.
*/
public interface GitRepositoryManager {
/**
* Get (or open) a repository by name.
*
@@ -92,22 +39,8 @@ public class GitRepositoryManager {
* @throws RepositoryNotFoundException the name does not denote an existing
* repository, or the name cannot be read as a repository.
*/
public Repository openRepository(String name)
throws RepositoryNotFoundException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
try {
final FileKey loc = FileKey.lenient(new File(basePath, name));
return RepositoryCache.open(loc);
} catch (IOException e1) {
final RepositoryNotFoundException e2;
e2 = new RepositoryNotFoundException("Cannot open repository " + name);
e2.initCause(e1);
throw e2;
}
}
public abstract Repository openRepository(String name)
throws RepositoryNotFoundException;
/**
* Create (and open) a repository by name.
@@ -118,25 +51,8 @@ public class GitRepositoryManager {
* @throws RepositoryNotFoundException the name does not denote an existing
* repository, or the name cannot be read as a repository.
*/
public Repository createRepository(String name)
throws RepositoryNotFoundException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
try {
if (!name.endsWith(".git")) {
name = name + ".git";
}
final FileKey loc = FileKey.exact(new File(basePath, name));
return RepositoryCache.open(loc, false);
} catch (IOException e1) {
final RepositoryNotFoundException e2;
e2 = new RepositoryNotFoundException("Cannot open repository " + name);
e2.initCause(e1);
throw e2;
}
}
public abstract Repository createRepository(String name)
throws RepositoryNotFoundException;
/**
* Read the {@code GIT_DIR/description} file for gitweb.
@@ -150,29 +66,8 @@ public class GitRepositoryManager {
* @throws IOException the description file exists, but is not readable by
* this process.
*/
public String getProjectDescription(final String name)
throws RepositoryNotFoundException, IOException {
final Repository e = openRepository(name);
final File d = new File(e.getDirectory(), "description");
String description;
try {
description = RawParseUtils.decode(IO.readFully(d));
} catch (FileNotFoundException err) {
return null;
}
if (description != null) {
description = description.trim();
if (description.isEmpty()) {
description = null;
}
if (UNNAMED.equals(description)) {
description = null;
}
}
return description;
}
public abstract String getProjectDescription(final String name)
throws RepositoryNotFoundException, IOException;
/**
* Set the {@code GIT_DIR/description} file for gitweb.
@@ -183,48 +78,6 @@ public class GitRepositoryManager {
* @param name the repository name, relative to the base directory.
* @param description new description text for the repository.
*/
public void setProjectDescription(final String name, final String description) {
// Update git's description file, in case gitweb is being used
//
try {
final Repository e;
final LockFile f;
e = openRepository(name);
f = new LockFile(new File(e.getDirectory(), "description"));
if (f.lock()) {
String d = description;
if (d != null) {
d = d.trim();
if (d.length() > 0) {
d += "\n";
}
} else {
d = "";
}
f.write(Constants.encode(d));
f.commit();
}
e.close();
} catch (RepositoryNotFoundException e) {
log.error("Cannot update description for " + name, e);
} catch (IOException e) {
log.error("Cannot update description for " + name, e);
}
}
private boolean isUnreasonableName(final String name) {
if (name.length() == 0) return true; // no empty paths
if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
if (name.charAt(0) == '/') return true; // no absolute paths
if (new File(name).isAbsolute()) return true; // no absolute paths
if (name.startsWith("../")) return true; // no "l../etc/passwd"
if (name.contains("/../")) return true; // no "foo/../etc/passwd"
if (name.contains("/./")) return true; // "foo/./foo" is insane to ask
if (name.contains("//")) return true; // windows UNC path can be "//..."
return false; // is a reasonable name
}
public abstract void setProjectDescription(final String name,
final String description);
}

191
gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java Normal file
View File

@@ -0,0 +1,191 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
import com.google.gerrit.lifecycle.LifecycleListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.LockFile;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.RepositoryCache;
import org.eclipse.jgit.lib.WindowCache;
import org.eclipse.jgit.lib.WindowCacheConfig;
import org.eclipse.jgit.lib.RepositoryCache.FileKey;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
/** Manages Git repositories stored on the local filesystem. */
@Singleton
public class LocalDiskRepositoryManager implements GitRepositoryManager {
private static final Logger log =
LoggerFactory.getLogger(LocalDiskRepositoryManager.class);
private static final String UNNAMED =
"Unnamed repository; edit this file to name it for gitweb.";
public static class Lifecycle implements LifecycleListener {
private final Config cfg;
@Inject
Lifecycle(@GerritServerConfig final Config cfg) {
this.cfg = cfg;
}
@Override
public void start() {
final WindowCacheConfig c = new WindowCacheConfig();
c.fromConfig(cfg);
WindowCache.reconfigure(c);
}
@Override
public void stop() {
}
}
private final File basePath;
@Inject
LocalDiskRepositoryManager(final SitePaths site,
@GerritServerConfig final Config cfg) {
basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
}
/** @return base directory under which all projects are stored. */
public File getBasePath() {
return basePath;
}
public Repository openRepository(String name)
throws RepositoryNotFoundException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
try {
final FileKey loc = FileKey.lenient(new File(basePath, name));
return RepositoryCache.open(loc);
} catch (IOException e1) {
final RepositoryNotFoundException e2;
e2 = new RepositoryNotFoundException("Cannot open repository " + name);
e2.initCause(e1);
throw e2;
}
}
public Repository createRepository(String name)
throws RepositoryNotFoundException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
try {
if (!name.endsWith(".git")) {
name = name + ".git";
}
final FileKey loc = FileKey.exact(new File(basePath, name));
return RepositoryCache.open(loc, false);
} catch (IOException e1) {
final RepositoryNotFoundException e2;
e2 = new RepositoryNotFoundException("Cannot open repository " + name);
e2.initCause(e1);
throw e2;
}
}
public String getProjectDescription(final String name)
throws RepositoryNotFoundException, IOException {
final Repository e = openRepository(name);
final File d = new File(e.getDirectory(), "description");
String description;
try {
description = RawParseUtils.decode(IO.readFully(d));
} catch (FileNotFoundException err) {
return null;
}
if (description != null) {
description = description.trim();
if (description.isEmpty()) {
description = null;
}
if (UNNAMED.equals(description)) {
description = null;
}
}
return description;
}
public void setProjectDescription(final String name, final String description) {
// Update git's description file, in case gitweb is being used
//
try {
final Repository e;
final LockFile f;
e = openRepository(name);
f = new LockFile(new File(e.getDirectory(), "description"));
if (f.lock()) {
String d = description;
if (d != null) {
d = d.trim();
if (d.length() > 0) {
d += "\n";
}
} else {
d = "";
}
f.write(Constants.encode(d));
f.commit();
}
e.close();
} catch (RepositoryNotFoundException e) {
log.error("Cannot update description for " + name, e);
} catch (IOException e) {
log.error("Cannot update description for " + name, e);
}
}
private boolean isUnreasonableName(final String name) {
if (name.length() == 0) return true; // no empty paths
if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
if (name.charAt(0) == '/') return true; // no absolute paths
if (new File(name).isAbsolute()) return true; // no absolute paths
if (name.startsWith("../")) return true; // no "l../etc/passwd"
if (name.contains("/../")) return true; // no "foo/../etc/passwd"
if (name.contains("/./")) return true; // "foo/./foo" is insane to ask
if (name.contains("//")) return true; // windows UNC path can be "//..."
return false; // is a reasonable name
}
}