opendev/gerrit
Code Issues Proposed changes

Use FileBasedConfig Config rather than RepositoryConfig

Browse Source 
When using the JGit config file code to read gerrit.config and
replication.config we are processing our own configuration file
and not a repository-specific configuration file.  Thus using
the type RepositoryConfig is an abuse, and we should instead
use FileBasedConfig when we must read it from disk, but in most
places the generic base class Config is much more suitable.

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-07-29 09:09:06 -07:00
parent fdff36116e
commit 92cf27fe3a
6 changed files with 83 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/main/java/com/google/gerrit/server/GerritConfigProvider.java
View File

@@ -27,7 +27,7 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import org.spearce.jgit.lib.RepositoryConfig;
import org.spearce.jgit.lib.Config;
import java.net.Inet6Address;
import java.net.InetAddress;
@@ -55,7 +55,7 @@ class GerritConfigProvider implements Provider<GerritConfig> {
}
private GerritConfig create() throws OrmException {
final RepositoryConfig cfg = server.getGerritConfig();
final Config cfg = server.getGerritConfig();
final GerritConfig config = new GerritConfig();
config.setCanonicalUrl(server.getCanonicalURL());
config.setUseContributorAgreements(cfg.getBoolean("auth",

18
src/main/java/com/google/gerrit/server/GerritServer.java
View File

@@ -48,10 +48,12 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.errors.ConfigInvalidException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.Config;
import org.spearce.jgit.lib.FileBasedConfig;
import org.spearce.jgit.lib.PersonIdent;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.RepositoryCache;
import org.spearce.jgit.lib.RepositoryConfig;
import org.spearce.jgit.lib.UserConfig;
import org.spearce.jgit.lib.WindowCache;
import org.spearce.jgit.lib.WindowCacheConfig;
import org.spearce.jgit.lib.RepositoryCache.FileKey;
@@ -101,7 +103,7 @@ public class GerritServer {
private final Database<ReviewDb> db;
private final File sitePath;
private final RepositoryConfig gerritConfigFile;
private final FileBasedConfig gerritConfigFile;
private final int sessionAge;
private final SignedToken xsrf;
private final SignedToken account;
@@ -117,7 +119,7 @@ public class GerritServer {
sitePath = path;
final File cfgLoc = new File(sitePath, "gerrit.config");
gerritConfigFile = new RepositoryConfig(null, cfgLoc);
gerritConfigFile = new FileBasedConfig(cfgLoc);
try {
gerritConfigFile.load();
} catch (FileNotFoundException e) {
@@ -206,7 +208,7 @@ public class GerritServer {
}
private void configureDefaultCache(final Configuration mgrCfg) {
final RepositoryConfig i = gerritConfigFile;
final Config i = gerritConfigFile;
final CacheConfiguration c = new CacheConfiguration();
c.setMaxElementsInMemory(i.getInt("cache", "memorylimit", 1024));
@@ -233,7 +235,7 @@ public class GerritServer {
private CacheConfiguration configureNamedCache(final Configuration mgrCfg,
final String name, final boolean disk, final int defaultAge) {
final RepositoryConfig i = gerritConfigFile;
final Config i = gerritConfigFile;
final CacheConfiguration def = mgrCfg.getDefaultCacheConfiguration();
final CacheConfiguration cfg;
try {
@@ -294,7 +296,7 @@ public class GerritServer {
throw new EmailException("Sending email is disabled");
}
final RepositoryConfig cfg = getGerritConfig();
final Config cfg = getGerritConfig();
String smtpHost = cfg.getString("sendemail", null, "smtpserver");
if (smtpHost == null) {
smtpHost = "127.0.0.1";
@@ -404,7 +406,7 @@ public class GerritServer {
}
/** Get the parsed <code>$site_path/gerrit.config</code> file. */
public RepositoryConfig getGerritConfig() {
public Config getGerritConfig() {
return gerritConfigFile;
}
@@ -485,7 +487,7 @@ public class GerritServer {
if (name == null) {
name = "Gerrit Code Review";
}
String email = getGerritConfig().getCommitterEmail();
String email = getGerritConfig().get(UserConfig.KEY).getCommitterEmail();
if (email == null || email.length() == 0) {
email = "gerrit@localhost";
}

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

@@ -23,7 +23,7 @@ import eu.medsea.mimeutil.MimeUtil2;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.lib.RepositoryConfig;
import org.spearce.jgit.lib.Config;
import java.security.AccessController;
import java.security.PrivilegedAction;
@@ -109,13 +109,13 @@ class MimeUtilFileTypeRegistry implements FileTypeRegistry {
return false;
}
final RepositoryConfig cfg = server.getGerritConfig();
final Config cfg = server.getGerritConfig();
final boolean any = isSafe(cfg, "*/*", false);
final boolean genericMedia = isSafe(cfg, type.getMediaType() + "/*", any);
return isSafe(cfg, type.toString(), genericMedia);
}
private static boolean isSafe(RepositoryConfig cfg, String type, boolean def) {
private static boolean isSafe(Config cfg, String type, boolean def) {
return cfg.getBoolean(SECTION_MIMETYPE, type, KEY_SAFE, def);
}

12
src/main/java/com/google/gerrit/server/ssh/GerritSshDaemon.java
View File

@@ -61,7 +61,7 @@ import org.apache.sshd.server.kex.DHG14;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.lib.RepositoryConfig;
import org.spearce.jgit.lib.Config;
import java.io.File;
import java.io.IOException;
@@ -132,7 +132,7 @@ public class GerritSshDaemon extends SshServer {
final PublickeyAuthenticator userAuth, @SitePath final File sitePath) {
setPort(22/* never used */);
final RepositoryConfig cfg = srv.getGerritConfig();
final Config cfg = srv.getGerritConfig();
listen = parseListen(cfg);
reuseAddress = cfg.getBoolean("sshd", "reuseaddress", true);
keepAlive = cfg.getBoolean("sshd", "tcpkeepalive", true);
@@ -257,7 +257,7 @@ public class GerritSshDaemon extends SshServer {
return r.toString();
}
private List<SocketAddress> parseListen(final RepositoryConfig cfg) {
private List<SocketAddress> parseListen(final Config cfg) {
final ArrayList<SocketAddress> bind = new ArrayList<SocketAddress>(2);
final String[] want = cfg.getStringList("sshd", null, "listenaddress");
if (want == null || want.length == 0) {
@@ -343,7 +343,7 @@ public class GerritSshDaemon extends SshServer {
}
@SuppressWarnings("unchecked")
private void initCiphers(final RepositoryConfig cfg) {
private void initCiphers(final Config cfg) {
final List<NamedFactory<Cipher>> a = new LinkedList<NamedFactory<Cipher>>();
a.add(new AES128CBC.Factory());
a.add(new TripleDESCBC.Factory());
@@ -375,13 +375,13 @@ public class GerritSshDaemon extends SshServer {
}
@SuppressWarnings("unchecked")
private void initMacs(final RepositoryConfig cfg) {
private void initMacs(final Config cfg) {
setMacFactories(filter(cfg, "mac", new HMACMD5.Factory(),
new HMACSHA1.Factory(), new HMACMD596.Factory(),
new HMACSHA196.Factory()));
}
private static <T> List<NamedFactory<T>> filter(final RepositoryConfig cfg,
private static <T> List<NamedFactory<T>> filter(final Config cfg,
final String key, final NamedFactory<T>... avail) {
final ArrayList<NamedFactory<T>> def = new ArrayList<NamedFactory<T>>();
for (final NamedFactory<T> n : avail) {