opendev/gerrit
Code Issues Proposed changes

Convert @SitePath from File to Path

Browse Source 
This saves code in some cases, as it allows us to use some of the
nicer Java 7 Files static methods. Mostly, though, it opens the door
to using an in-memory filesystem in tests. Eventually there are even
more fun possibilities, like teaching parts of SitePaths to be read
out of somewhere else entirely (like a git repository).

Change-Id: Ifa13772a79ded03049bd9f62ade6e25d19e5bb05
This commit is contained in:
Dave Borowitz
2015-02-23 10:32:23 -08:00
parent 0f092fe518
commit 8e481f3c33
20 changed files with 181 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
gerrit-server/src/main/java/com/google/gerrit/server/config/GetSummary.java
View File

@@ -24,7 +24,6 @@ import com.google.inject.Inject;
import org.eclipse.jgit.internal.storage.file.WindowCacheStatAccessor;
import org.kohsuke.args4j.Option;
import java.io.File;
import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.lang.management.OperatingSystemMXBean;
@@ -33,6 +32,8 @@ import java.lang.management.ThreadInfo;
import java.lang.management.ThreadMXBean;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
@@ -43,7 +44,7 @@ import java.util.Map;
public class GetSummary implements RestReadView<ConfigResource> {
private final WorkQueue workQueue;
private final File sitePath;
private final Path sitePath;
@Option(name = "--gc", usage = "perform Java GC before retrieving memory stats")
private boolean gc;
@@ -62,7 +63,7 @@ public class GetSummary implements RestReadView<ConfigResource> {
}
@Inject
public GetSummary(WorkQueue workQueue, @SitePath File sitePath) {
public GetSummary(WorkQueue workQueue, @SitePath Path sitePath) {
this.workQueue = workQueue;
this.sitePath = sitePath;
}
@@ -186,7 +187,8 @@ public class GetSummary implements RestReadView<ConfigResource> {
} catch (UnknownHostException e) {
}
jvmSummary.currentWorkingDirectory = path(new File(".").getAbsoluteFile().getParentFile());
jvmSummary.currentWorkingDirectory =
path(Paths.get(".").toAbsolutePath().getParent());
jvmSummary.site = path(sitePath);
return jvmSummary;
}
@@ -210,11 +212,11 @@ public class GetSummary implements RestReadView<ConfigResource> {
return String.format("%1$6.2f%2$s", value, suffix).trim();
}
private static String path(File file) {
private static String path(Path path) {
try {
return file.getCanonicalPath();
return path.toRealPath().normalize().toString();
} catch (IOException err) {
return file.getAbsolutePath();
return path.toAbsolutePath().normalize().toString();
}
}

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

@@ -20,6 +20,7 @@ import com.google.inject.Singleton;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Path;
/** Important paths within a {@link SitePath}. */
@Singleton
@@ -64,8 +65,9 @@ public final class SitePaths {
public final boolean isNew;
@Inject
public SitePaths(final @SitePath File sitePath) throws FileNotFoundException {
site_path = sitePath;
public SitePaths(final @SitePath Path sitePath) throws FileNotFoundException {
// TODO(dborowitz): Convert all of these to Paths.
site_path = sitePath.toFile();
bin_dir = new File(site_path, "bin");
etc_dir = new File(site_path, "etc");

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

@@ -33,7 +33,7 @@ import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.File;
import java.nio.file.Path;
/**
* Copies critical objects from the {@code dbInjector} into a plugin.
@@ -47,11 +47,11 @@ import java.io.File;
class CopyConfigModule extends AbstractModule {
@Inject
@SitePath
private File sitePath;
private Path sitePath;
@Provides
@SitePath
File getSitePath() {
Path getSitePath() {
return sitePath;
}

12
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java
View File

@@ -32,14 +32,14 @@ import com.google.inject.Inject;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collections;
/** Creates the current database schema and populates initial code rows. */
public class SchemaCreator {
private final @SitePath
File site_path;
Path site_path;
private final AllProjectsCreator allProjectsCreator;
private final AllUsersCreator allUsersCreator;
@@ -55,10 +55,10 @@ public class SchemaCreator {
AllUsersCreator auc,
@GerritPersonIdent PersonIdent au,
DataSourceType dst) {
this(site.site_path, ap, auc, au, dst);
this(site.site_path.toPath(), ap, auc, au, dst);
}
public SchemaCreator(@SitePath File site,
public SchemaCreator(@SitePath Path site,
AllProjectsCreator ap,
AllUsersCreator auc,
@GerritPersonIdent PersonIdent au,
@@ -117,9 +117,9 @@ public class SchemaCreator {
final SystemConfig s = SystemConfig.create();
try {
s.sitePath = site_path.getCanonicalPath();
s.sitePath = site_path.toRealPath().normalize().toString();
} catch (IOException e) {
s.sitePath = site_path.getAbsolutePath();
s.sitePath = site_path.toAbsolutePath().normalize().toString();
}
c.systemConfig().insert(Collections.singleton(s));
return s;

51
gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java
View File

@@ -28,85 +28,86 @@ import org.junit.Test;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
public class SitePathsTest {
@Test
public void testCreate_NotExisting() throws IOException {
final File root = random();
final Path root = random();
final SitePaths site = new SitePaths(root);
assertTrue(site.isNew);
assertEquals(root, site.site_path);
assertEquals(new File(root, "etc"), site.etc_dir);
assertEquals(root.toFile(), site.site_path);
assertEquals(root.resolve("etc").toFile(), site.etc_dir);
}
@Test
public void testCreate_Empty() throws IOException {
final File root = random();
final Path root = random();
try {
assertTrue(root.mkdir());
Files.createDirectory(root);
final SitePaths site = new SitePaths(root);
assertTrue(site.isNew);
assertEquals(root, site.site_path);
assertEquals(root.toFile(), site.site_path);
} finally {
root.delete();
Files.delete(root);
}
}
@Test
public void testCreate_NonEmpty() throws IOException {
final File root = random();
final File txt = new File(root, "test.txt");
final Path root = random();
final Path txt = root.resolve("test.txt");
try {
assertTrue(root.mkdir());
assertTrue(txt.createNewFile());
Files.createDirectory(root);
Files.createFile(txt);
final SitePaths site = new SitePaths(root);
assertFalse(site.isNew);
assertEquals(root, site.site_path);
assertEquals(root.toFile(), site.site_path);
} finally {
txt.delete();
root.delete();
Files.delete(txt);
Files.delete(root);
}
}
@Test
public void testCreate_NotDirectory() throws IOException {
final File root = random();
final Path root = random();
try {
assertTrue(root.createNewFile());
Files.createFile(root);
try {
new SitePaths(root);
fail("Did not throw exception");
} catch (FileNotFoundException e) {
assertEquals("Not a directory: " + root.getPath(), e.getMessage());
assertEquals("Not a directory: " + root, e.getMessage());
}
} finally {
root.delete();
Files.delete(root);
}
}
@Test
public void testResolve() throws IOException {
final File root = random();
final Path root = random();
final SitePaths site = new SitePaths(root);
assertNull(site.resolve(null));
assertNull(site.resolve(""));
assertNotNull(site.resolve("a"));
assertEquals(new File(root, "a").getCanonicalFile(), site.resolve("a"));
assertEquals(root.resolve("a").toAbsolutePath().normalize().toFile(),
site.resolve("a"));
final String pfx = HostPlatform.isWin32() ? "C:/" : "/";
assertNotNull(site.resolve(pfx + "a"));
assertEquals(new File(pfx + "a").getCanonicalFile(), site.resolve(pfx + "a"));
}
private static File random() throws IOException {
File tmp = File.createTempFile("gerrit_test_", "_site");
if (!tmp.delete()) {
throw new IOException("Cannot create " + tmp.getPath());
}
private static Path random() throws IOException {
Path tmp = Files.createTempFile("gerrit_test_", "_site");
Files.deleteIfExists(tmp);
return tmp;
}
}

5
gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java
View File

@@ -43,9 +43,10 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.UUID;
@@ -67,7 +68,7 @@ public class SchemaUpdaterTest {
IOException {
db.create();
final File site = new File(UUID.randomUUID().toString());
final Path site = Paths.get(UUID.randomUUID().toString());
final SitePaths paths = new SitePaths(site);
SchemaUpdater u = Guice.createInjector(new FactoryModule() {
@Override

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

@@ -69,11 +69,12 @@ import com.google.inject.servlet.RequestScoped;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.File;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.nio.file.Path;
import java.nio.file.Paths;
public class InMemoryModule extends FactoryModule {
public static Config newDefaultConfig() {
@@ -125,7 +126,8 @@ public class InMemoryModule extends FactoryModule {
bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);
bind(File.class).annotatedWith(SitePath.class).toInstance(new File("."));
// TODO(dborowitz): Use jimfs.
bind(Path.class).annotatedWith(SitePath.class).toInstance(Paths.get("."));
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(cfg);
bind(SocketAddress.class).annotatedWith(RemotePeer.class).toInstance(
new InetSocketAddress(InetAddresses.forString("127.0.0.1"), 1234));