Fix outstanding issues with conversion to NIO

NIO methods throw NoSuchFileException, not FileNotFoundException. How
nice.

Throwing from Files#getLastModifiedTime, apart from cluttering up
throws clauses, means callers have to check for FileNotFoundException,
which is verbose and error prone. Instead, implement a helper with the
old File#lastModified() semantics; this preserves old behavior, which
might not handle some error cases well, but worked well enough. (In
theory we could use FileSnapshot or something, but I don't want to
shave that particular yak until/unless I get a chance to add a
Path-compatible interface to FileSnapshot in JGit as well.)

Organize some imports.

Tested:
-Tests pass.
-Initialized a site, including downloading.
-Launched a site.
-Added and removed site headers and CSS in a running site.
-Added and removed plugins, both live and while shutdown.

Change-Id: I60dfe5280512f00dce4ec99fe3689b2f2b3c6685
This commit is contained in:
Dave Borowitz
2015-02-24 10:57:11 -08:00
parent 07b40724b5
commit 05b1ae22d5
15 changed files with 50 additions and 46 deletions

View File

@@ -68,8 +68,17 @@ public class FileUtil {
}
}
/**
* Get the last modified time of a path.
* <p>
* Equivalent to {@code File#lastModified()}, returning 0 on errors, including
* file not found. Callers that prefer exceptions can use {@link
* Files#getLastModifiedTime(Path, java.nio.file.LinkOption...)}.
*
* @param p path.
* @return last modified time, in milliseconds since epoch.
*/
public static long lastModified(Path p) {
// Replicate File#lastModified() behavior of returning 0 on errors.
try {
return Files.getLastModifiedTime(p).toMillis();
} catch (IOException e) {

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.common;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.Ordering;
@@ -38,14 +40,6 @@ public final class SiteLibraryLoaderUtil {
}
}
private static long lastModified(Path p) {
try {
return Files.getLastModifiedTime(p).toMillis();
} catch (IOException e) {
return 0;
}
}
public static List<Path> listJars(Path dir) throws IOException {
DirectoryStream.Filter<Path> filter = new DirectoryStream.Filter<Path>() {
@Override

View File

@@ -23,13 +23,13 @@ import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;
import java.io.ByteArrayOutputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringWriter;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.zip.GZIPOutputStream;
@@ -192,7 +192,7 @@ public class HtmlDomUtil {
Document doc = newBuilder().parse(in);
compact(doc);
return doc;
} catch (FileNotFoundException e) {
} catch (NoSuchFileException e) {
return null;
} catch (SAXException | ParserConfigurationException | IOException e) {
throw new IOException("Error reading " + path, e);
@@ -208,7 +208,7 @@ public class HtmlDomUtil {
Path path = parentDir.resolve(name);
try (InputStream in = Files.newInputStream(path)) {
return new String(ByteStreams.toByteArray(in), ENC);
} catch (FileNotFoundException e) {
} catch (NoSuchFileException e) {
return null;
} catch (IOException e) {
throw new IOException("Error reading " + path, e);

View File

@@ -14,16 +14,18 @@
package com.google.gerrit.httpd.gitweb;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.io.ByteStreams;
import com.google.gerrit.httpd.GitWebConfig;
import com.google.gwtexpui.server.CacheHeaders;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
@@ -45,10 +47,10 @@ class GitLogoServlet extends HttpServlet {
if (src != null) {
try (InputStream in = Files.newInputStream(src)) {
png = ByteStreams.toByteArray(in);
} catch (FileNotFoundException e) {
} catch (NoSuchFileException e) {
png = null;
}
modified = Files.getLastModifiedTime(src).toMillis();
modified = lastModified(src);
} else {
modified = -1;
png = null;

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.httpd.gitweb;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.gerrit.httpd.GitWebConfig;
import com.google.gerrit.httpd.HtmlDomUtil;
import com.google.gerrit.server.config.SitePaths;
@@ -23,7 +25,6 @@ import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
@@ -63,7 +64,7 @@ abstract class GitWebCssServlet extends HttpServlet {
final String name = src.getFileName().toString();
final String raw = HtmlDomUtil.readFile(dir, name);
if (raw != null) {
modified = Files.getLastModifiedTime(src).toMillis();
modified = lastModified(src);
raw_css = raw.getBytes(ENC);
gz_css = HtmlDomUtil.compress(raw_css);
} else {

View File

@@ -14,16 +14,18 @@
package com.google.gerrit.httpd.gitweb;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.io.ByteStreams;
import com.google.gerrit.httpd.GitWebConfig;
import com.google.gwtexpui.server.CacheHeaders;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
@@ -45,10 +47,10 @@ class GitWebJavaScriptServlet extends HttpServlet {
if (src != null) {
try (InputStream in = Files.newInputStream(src)) {
png = ByteStreams.toByteArray(in);
} catch (FileNotFoundException e) {
} catch (NoSuchFileException e) {
png = null;
}
modified = Files.getLastModifiedTime(src).toMillis();
modified = lastModified(src);
} else {
modified = -1;
png = null;

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.httpd.plugins;
import static com.google.gerrit.common.FileUtil.lastModified;
import static com.google.gerrit.server.plugins.PluginEntry.ATTR_CHARACTER_ENCODING;
import static com.google.gerrit.server.plugins.PluginEntry.ATTR_CONTENT_TYPE;
@@ -304,8 +305,7 @@ class HttpPluginServlet extends HttpServlet
}
if (!entry.isPresent() && file.endsWith("/index.html")) {
String pfx = file.substring(0, file.length() - "index.html".length());
long pluginLastModified =
Files.getLastModifiedTime(holder.plugin.getSrcFile()).toMillis();
long pluginLastModified = lastModified(holder.plugin.getSrcFile());
if (hasUpToDateCachedResource(rsc, pluginLastModified)) {
rsc.send(req, res);
} else {

View File

@@ -14,7 +14,7 @@
package com.google.gerrit.httpd.raw;
import static java.nio.file.Files.getLastModifiedTime;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
@@ -55,7 +55,6 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.io.StringWriter;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.util.ArrayList;
import java.util.List;
@@ -289,15 +288,15 @@ public class HostPageServlet extends HttpServlet {
private static class FileInfo {
private final Path path;
private final FileTime time;
private final long time;
FileInfo(Path p) throws IOException {
FileInfo(Path p) {
path = p;
time = getLastModifiedTime(path);
time = lastModified(path);
}
boolean isStale() throws IOException {
return !time.equals(getLastModifiedTime(path));
boolean isStale() {
return time != lastModified(path);
}
}
@@ -340,7 +339,7 @@ public class HostPageServlet extends HttpServlet {
debug = new Content(hostDoc);
}
boolean isStale() throws IOException {
boolean isStale() {
return css.isStale() || header.isStale() || footer.isStale();
}

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.httpd.template;
import static com.google.gerrit.common.FileUtil.lastModified;
import com.google.common.base.Strings;
import com.google.gerrit.httpd.HtmlDomUtil;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -28,9 +30,7 @@ import org.w3c.dom.Document;
import org.w3c.dom.Element;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
@Singleton
public class SiteHeaderFooter {
@@ -112,7 +112,7 @@ public class SiteHeaderFooter {
Element header;
Element footer;
Template(SitePaths site) throws IOException {
Template(SitePaths site) {
cssFile = new FileInfo(site.site_css);
headerFile = new FileInfo(site.site_header);
footerFile = new FileInfo(site.site_footer);
@@ -126,7 +126,7 @@ public class SiteHeaderFooter {
footer = readXml(footerFile);
}
boolean isStale() throws IOException {
boolean isStale() {
return cssFile.isStale() || headerFile.isStale() || footerFile.isStale();
}
@@ -138,15 +138,15 @@ public class SiteHeaderFooter {
private static class FileInfo {
final Path path;
final FileTime time;
final long time;
FileInfo(Path p) throws IOException {
FileInfo(Path p) {
path = p;
time = Files.getLastModifiedTime(path);
time = lastModified(p);
}
boolean isStale() throws IOException {
return !time.equals(Files.getLastModifiedTime(path));
boolean isStale() {
return time != lastModified(path);
}
}
}

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.pgm;
import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.annotations.VisibleForTesting;

View File

@@ -17,7 +17,6 @@ package com.google.gerrit.pgm.init;
import static com.google.gerrit.common.FileUtil.chmod;
import static com.google.gerrit.pgm.init.api.InitUtil.die;
import static com.google.gerrit.pgm.init.api.InitUtil.hostname;
import static java.nio.file.Files.exists;
import com.google.gerrit.pgm.init.api.ConsoleUI;

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.pgm.init;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.easymock.EasyMock.createStrictMock;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;

View File

@@ -26,8 +26,8 @@ import org.eclipse.jgit.util.FS;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
@Singleton

View File

@@ -36,9 +36,9 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.security.KeyPair;
import java.security.PublicKey;
@@ -203,7 +203,7 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
}
}
return Collections.unmodifiableSet(keys);
} catch (FileNotFoundException noFile) {
} catch (NoSuchFileException noFile) {
return Collections.emptySet();
} catch (IOException err) {
log.error("Cannot read " + path, err);