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

7 files changed, 387 insertions, 16 deletions

diff --git a/gerrit-httpd/BUCK b/gerrit-httpd/BUCK
index 3f81d86174..3085054087 100644
--- a/gerrit-httpd/BUCK
+++ b/gerrit-httpd/BUCK
@@ -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
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/ResourceServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/ResourceServlet.java
index 507bc59812..6f7ad519a9 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/ResourceServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/ResourceServlet.java
@@ -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);
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/ResourceServletTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/ResourceServletTest.java
new file mode 100644
index 0000000000..2ddd4eef7a
--- /dev/null
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/ResourceServletTest.java
@@ -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);
+    }
+  }
+}
diff --git a/gerrit-util-http/BUCK b/gerrit-util-http/BUCK
index 4c3816b46c..e9b2a3da7e 100644
--- a/gerrit-util-http/BUCK
+++ b/gerrit-util-http/BUCK
@@ -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'],
diff --git a/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java b/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
index 04c7b48637..3991b95051 100644
--- a/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
+++ b/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
@@ -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;
+  }
 }
diff --git a/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java b/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
index 6201bb0e48..442c4744b2 100644
--- a/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
+++ b/gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
@@ -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() {
diff --git a/lib/BUCK b/lib/BUCK
index a92f910117..c635bbb762 100644
--- a/lib/BUCK
+++ b/lib/BUCK
@@ -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',