Add some tests for ResourceServlet

These are small tests using a custom subclass of ResourceServlet
pointing at in in-memory filesystem using Jimfs. Larger tests that
actually check the mappings in StaticModule are somewhat trickier.

These tests are not yet comprehensive, but were enough to identify one
bug in 404 behavior, where an uncaught NoSuchFileException turned what
should have been a 404 into a 500.

Change-Id: Iee8059444a66f9be608c6605ec16cc1b46c443e2
This commit is contained in:
Dave Borowitz 2015-12-01 13:29:03 -05:00
parent 054b8f385f
commit 9d60d90661
7 changed files with 387 additions and 16 deletions

View File

@ -59,6 +59,7 @@ java_test(
'//gerrit-server:server',
'//gerrit-util-http:http',
'//gerrit-util-http:testutil',
'//lib:jimfs',
'//lib:junit',
'//lib:gson',
'//lib:gwtorm',
@ -70,6 +71,7 @@ java_test(
'//lib/guice:guice-servlet',
'//lib/jgit:jgit',
'//lib/jgit:junit',
'//lib/joda:joda-time',
],
source_under_test = [':httpd'],
# TODO(sop) Remove after Buck supports Eclipse

View File

@ -26,6 +26,7 @@ import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableMap;
@ -96,10 +97,18 @@ public abstract class ResourceServlet extends HttpServlet {
private final Cache<Path, Resource> cache;
private final boolean refresh;
private final int cacheFileSizeLimitBytes;
protected ResourceServlet(Cache<Path, Resource> cache, boolean refresh) {
this(cache, refresh, CACHE_FILE_SIZE_LIMIT_BYTES);
}
@VisibleForTesting
ResourceServlet(Cache<Path, Resource> cache, boolean refresh,
int cacheFileSizeLimitBytes) {
this.cache = checkNotNull(cache, "cache");
this.refresh = refresh;
this.cacheFileSizeLimitBytes = cacheFileSizeLimitBytes;
}
/**
@ -214,7 +223,7 @@ public abstract class ResourceServlet extends HttpServlet {
private boolean maybeStream(Path p, HttpServletRequest req,
HttpServletResponse rsp) throws IOException {
try {
if (Files.size(p) < CACHE_FILE_SIZE_LIMIT_BYTES) {
if (Files.size(p) < cacheFileSizeLimitBytes) {
return false;
}
} catch (NoSuchFileException e) {
@ -294,7 +303,12 @@ public abstract class ResourceServlet extends HttpServlet {
}
boolean isStale(Path p, ResourceServlet rs) throws IOException {
FileTime t = rs.getLastModifiedTime(p);
FileTime t;
try {
t = rs.getLastModifiedTime(p);
} catch (NoSuchFileException e) {
return this != NOT_FOUND;
}
return t.toMillis() == 0
|| lastModified.toMillis() == 0
|| !lastModified.equals(t);

View File

@ -0,0 +1,337 @@
// Copyright (C) 2015 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.httpd.raw;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.Lists;
import com.google.common.io.ByteStreams;
import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import com.google.gerrit.httpd.raw.ResourceServlet.Resource;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
import com.google.gerrit.util.http.testutil.FakeHttpServletResponse;
import org.joda.time.format.ISODateTimeFormat;
import org.junit.Before;
import org.junit.Test;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.util.concurrent.atomic.AtomicLong;
import java.util.zip.GZIPInputStream;
public class ResourceServletTest {
private static Cache<Path, Resource> newCache(int size) {
return CacheBuilder.newBuilder()
.maximumSize(size)
.recordStats()
.build();
}
private static class Servlet extends ResourceServlet {
private static final long serialVersionUID = 1L;
private final FileSystem fs;
private Servlet(FileSystem fs, Cache<Path, Resource> cache,
boolean refresh) {
super(cache, refresh);
this.fs = fs;
}
private Servlet(FileSystem fs, Cache<Path, Resource> cache,
boolean refresh, int cacheFileSizeLimitBytes) {
super(cache, refresh, cacheFileSizeLimitBytes);
this.fs = fs;
}
@Override
protected Path getResourcePath(String pathInfo) {
return fs.getPath("/" + CharMatcher.is('/').trimLeadingFrom(pathInfo));
}
}
private FileSystem fs;
private AtomicLong ts;
@Before
public void setUp() {
fs = Jimfs.newFileSystem(Configuration.unix());
ts = new AtomicLong(ISODateTimeFormat.dateTime().parseMillis(
"2010-01-30T12:00:00.000-08:00"));
}
@Test
public void notFoundWithoutRefresh() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, false);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(request("/notfound"), res);
assertThat(res.getStatus()).isEqualTo(SC_NOT_FOUND);
assertNotCacheable(res);
assertCacheHits(cache, 0, 1);
res = new FakeHttpServletResponse();
servlet.doGet(request("/notfound"), res);
assertThat(res.getStatus()).isEqualTo(SC_NOT_FOUND);
assertNotCacheable(res);
assertCacheHits(cache, 1, 1);
}
@Test
public void notFoundWithRefresh() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, true);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(request("/notfound"), res);
assertThat(res.getStatus()).isEqualTo(SC_NOT_FOUND);
assertNotCacheable(res);
assertCacheHits(cache, 0, 1);
res = new FakeHttpServletResponse();
servlet.doGet(request("/notfound"), res);
assertThat(res.getStatus()).isEqualTo(SC_NOT_FOUND);
assertNotCacheable(res);
assertCacheHits(cache, 1, 1);
}
@Test
public void smallFileWithRefresh() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, true);
writeFile("/foo", "foo1");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertCacheable(res, true);
assertHasETag(res);
// Miss on getIfPresent, miss on get.
assertCacheHits(cache, 0, 2);
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertCacheable(res, true);
assertHasETag(res);
assertCacheHits(cache, 1, 2);
writeFile("/foo", "foo2");
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo2");
assertCacheable(res, true);
assertHasETag(res);
// Hit, invalidate, miss.
assertCacheHits(cache, 2, 3);
}
@Test
public void smallFileWithoutRefresh() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, false);
writeFile("/foo", "foo1");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertCacheable(res, false);
assertHasETag(res);
// Miss on getIfPresent, miss on get.
assertCacheHits(cache, 0, 2);
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertCacheable(res, false);
assertHasETag(res);
assertCacheHits(cache, 1, 2);
writeFile("/foo", "foo2");
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertCacheable(res, false);
assertHasETag(res);
assertCacheHits(cache, 2, 2);
}
@Test
public void verySmallFileDoesntBotherWithGzip() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, true);
writeFile("/foo", "foo1");
FakeHttpServletRequest req = request("/foo")
.addHeader("Accept-Encoding", "gzip");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(req, res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getHeader("Content-Encoding")).isNull();
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertHasETag(res);
assertCacheable(res, true);
}
@Test
public void smallFileWithGzip() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, true);
String content = Strings.repeat("a", 100);
writeFile("/foo", content);
FakeHttpServletRequest req = request("/foo")
.addHeader("Accept-Encoding", "gzip");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(req, res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getHeader("Content-Encoding")).isEqualTo("gzip");
assertThat(gunzip(res.getActualBody())).isEqualTo(content);
assertHasETag(res);
assertCacheable(res, true);
}
@Test
public void largeFileBypassesCacheRegardlessOfRefreshParamter()
throws Exception {
for (boolean refresh : Lists.newArrayList(true, false)) {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, refresh, 3);
writeFile("/foo", "foo1");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertThat(res.getHeader("Last-Modified")).isNotNull();
assertCacheable(res, refresh);
assertHasLastModified(res);
assertCacheHits(cache, 0, 1);
writeFile("/foo", "foo1");
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo1");
assertThat(res.getHeader("Last-Modified")).isNotNull();
assertCacheable(res, refresh);
assertHasLastModified(res);
assertCacheHits(cache, 0, 2);
writeFile("/foo", "foo2");
res = new FakeHttpServletResponse();
servlet.doGet(request("/foo"), res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getActualBodyString()).isEqualTo("foo2");
assertThat(res.getHeader("Last-Modified")).isNotNull();
assertCacheable(res, refresh);
assertHasLastModified(res);
assertCacheHits(cache, 0, 3);
}
}
@Test
public void largeFileWithGzip() throws Exception {
Cache<Path, Resource> cache = newCache(1);
Servlet servlet = new Servlet(fs, cache, true, 3);
String content = Strings.repeat("a", 100);
writeFile("/foo", content);
FakeHttpServletRequest req = request("/foo")
.addHeader("Accept-Encoding", "gzip");
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.doGet(req, res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
assertThat(res.getHeader("Content-Encoding")).isEqualTo("gzip");
assertThat(gunzip(res.getActualBody())).isEqualTo(content);
assertHasLastModified(res);
assertCacheable(res, true);
}
// TODO(dborowitz): Check MIME type.
// TODO(dborowitz): Test that JS is not gzipped.
// TODO(dborowitz): Test ?e parameter.
// TODO(dborowitz): Test If-None-Match behavior.
// TODO(dborowitz): Test If-Modified-Since behavior.
private void writeFile(String path, String content) throws Exception {
Files.write(fs.getPath(path), content.getBytes(UTF_8));
Files.setLastModifiedTime(
fs.getPath(path), FileTime.fromMillis(ts.getAndIncrement()));
}
private static void assertCacheHits(Cache<?, ?> cache, int hits, int misses) {
assertThat(cache.stats().hitCount()).named("hits").isEqualTo(hits);
assertThat(cache.stats().missCount()).named("misses").isEqualTo(misses);
}
private static void assertCacheable(FakeHttpServletResponse res,
boolean revalidate) {
String header = res.getHeader("Cache-Control").toLowerCase();
assertThat(header).contains("public");
if (revalidate) {
assertThat(header).contains("must-revalidate");
} else {
assertThat(header).doesNotContain("must-revalidate");
}
}
private static void assertHasLastModified(FakeHttpServletResponse res) {
assertThat(res.getHeader("Last-Modified")).isNotNull();
assertThat(res.getHeader("ETag")).isNull();
}
private static void assertHasETag(FakeHttpServletResponse res) {
assertThat(res.getHeader("ETag")).isNotNull();
assertThat(res.getHeader("Last-Modified")).isNull();
}
private static void assertNotCacheable(FakeHttpServletResponse res) {
assertThat(res.getHeader("Cache-Control")).contains("no-cache");
assertThat(res.getHeader("ETag")).isNull();
assertThat(res.getHeader("Last-Modified")).isNull();
}
private static FakeHttpServletRequest request(String path) {
return new FakeHttpServletRequest().setPathInfo(path);
}
private static String gunzip(byte[] data) throws Exception {
try (InputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) {
return new String(ByteStreams.toByteArray(in), UTF_8);
}
}
}

View File

@ -14,6 +14,7 @@ java_library(
'//gerrit-extension-api:api',
'//lib:guava',
'//lib:servlet-api-3_1',
'//lib/httpcomponents:httpclient',
'//lib/jgit:jgit',
],
visibility = ['PUBLIC'],

View File

@ -26,6 +26,8 @@ import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
import com.google.gerrit.extensions.restapi.Url;
import org.apache.http.client.utils.DateUtils;
import java.io.BufferedReader;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
@ -267,7 +269,8 @@ public class FakeHttpServletRequest implements HttpServletRequest {
@Override
public long getDateHeader(String name) {
throw new UnsupportedOperationException();
String v = getHeader(name);
return v != null ? DateUtils.parseDate(v).getTime() : 0;
}
@Override
@ -301,7 +304,7 @@ public class FakeHttpServletRequest implements HttpServletRequest {
}
public FakeHttpServletRequest setPathInfo(String path) {
this.path = checkNotNull(path);
this.path = path;
return this;
}
@ -477,4 +480,9 @@ public class FakeHttpServletRequest implements HttpServletRequest {
Class<T> httpUpgradeHandlerClass) {
throw new UnsupportedOperationException();
}
public FakeHttpServletRequest addHeader(String name, String value) {
headers.put(name, value);
return this;
}
}

View File

@ -85,12 +85,10 @@ public class FakeHttpServletResponse implements HttpServletResponse {
public synchronized ServletOutputStream getOutputStream() {
checkState(writer == null, "getWriter() already called");
if (outputStream == null) {
final PrintWriter osWriter = new PrintWriter(actualBody);
outputStream = new ServletOutputStream() {
@Override
public void write(int c) throws IOException {
osWriter.write(c);
osWriter.flush();
actualBody.write(c);
}
@Override
@ -150,13 +148,13 @@ public class FakeHttpServletResponse implements HttpServletResponse {
@Override
public void setContentLengthLong(long length) {
headers.removeAll(HttpHeaders.CONTENT_LENGTH);
headers.put(HttpHeaders.CONTENT_LENGTH, Long.toString(length));
addHeader(HttpHeaders.CONTENT_LENGTH, Long.toString(length));
}
@Override
public void setContentType(String type) {
headers.removeAll(HttpHeaders.CONTENT_TYPE);
headers.put(HttpHeaders.CONTENT_TYPE, type);
addHeader(HttpHeaders.CONTENT_TYPE, type);
}
@Override
@ -176,17 +174,17 @@ public class FakeHttpServletResponse implements HttpServletResponse {
@Override
public void addHeader(String name, String value) {
headers.put(name, value);
headers.put(name.toLowerCase(), value);
}
@Override
public void addIntHeader(String name, int value) {
headers.put(name, Integer.toString(value));
addHeader(name, Integer.toString(value));
}
@Override
public boolean containsHeader(String name) {
return headers.containsKey(name);
return headers.containsKey(name.toLowerCase());
}
@Override
@ -237,13 +235,13 @@ public class FakeHttpServletResponse implements HttpServletResponse {
@Override
public void setHeader(String name, String value) {
headers.removeAll(name);
headers.removeAll(name.toLowerCase());
addHeader(name, value);
}
@Override
public void setIntHeader(String name, int value) {
headers.removeAll(name);
headers.removeAll(name.toLowerCase());
addIntHeader(name, value);
}
@ -267,7 +265,8 @@ public class FakeHttpServletResponse implements HttpServletResponse {
@Override
public String getHeader(String name) {
return Iterables.getFirst(headers.get(checkNotNull(name)), null);
return Iterables.getFirst(
headers.get(checkNotNull(name.toLowerCase())), null);
}
@Override
@ -277,7 +276,7 @@ public class FakeHttpServletResponse implements HttpServletResponse {
@Override
public Collection<String> getHeaders(String name) {
return headers.get(checkNotNull(name));
return headers.get(checkNotNull(name.toLowerCase()));
}
public byte[] getActualBody() {

View File

@ -184,6 +184,16 @@ maven_jar(
license = 'protobuf',
)
# Test-only dependencies below.
maven_jar(
name = 'jimfs',
id = 'com.google.jimfs:jimfs:1.0',
sha1 = 'edd65a2b792755f58f11134e76485a928aab4c97',
license = 'DO_NOT_DISTRIBUTE',
deps = [':guava'],
)
maven_jar(
name = 'junit',
id = 'junit:junit:4.11',