Don’t allow client to cache PolyGerrit index.html

The top-level index should never be cached on the client since
it’s lightweight and points to the latest versions of the gr-app
resources. This was causing issues where users were seeing stale
versions of PolyGerrit because this page was being cached.

It should be noted that the _server_ should cache the index.html
file indefinitely, and that this change introduces a potentially
confusing parameter since “cache” is used both in the context of
the in-memory cache of the binary and also the user’s browser
cache.

Bug: Issue 4403
Change-Id: I70543aceedd5eea00ba9ee5123f16aaffb1f935c

gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/ResourceServlet.java

@@ -98,17 +98,24 @@ public abstract class ResourceServlet extends HttpServlet {
 
   private final Cache<Path, Resource> cache;
   private final boolean refresh;
+  private final boolean cacheOnClient;
   private final int cacheFileSizeLimitBytes;
 
   protected ResourceServlet(Cache<Path, Resource> cache, boolean refresh) {
-    this(cache, refresh, CACHE_FILE_SIZE_LIMIT_BYTES);
+    this(cache, refresh, true, CACHE_FILE_SIZE_LIMIT_BYTES);
+  }
+
+  protected ResourceServlet(Cache<Path, Resource> cache, boolean refresh,
+      boolean cacheOnClient) {
+    this(cache, refresh, cacheOnClient, CACHE_FILE_SIZE_LIMIT_BYTES);
   }
 
   @VisibleForTesting
   ResourceServlet(Cache<Path, Resource> cache, boolean refresh,
-      int cacheFileSizeLimitBytes) {
+      boolean cacheOnClient, int cacheFileSizeLimitBytes) {
     this.cache = checkNotNull(cache, "cache");
     this.refresh = refresh;
+    this.cacheOnClient = cacheOnClient;
     this.cacheFileSizeLimitBytes = cacheFileSizeLimitBytes;
   }
 
@@ -173,7 +180,7 @@ public abstract class ResourceServlet extends HttpServlet {
       CacheHeaders.setNotCacheable(rsp);
       rsp.setStatus(SC_NOT_FOUND);
       return;
-    } else if (r.etag.equals(req.getHeader(IF_NONE_MATCH))) {
+    } else if (cacheOnClient && r.etag.equals(req.getHeader(IF_NONE_MATCH))) {
       rsp.setStatus(SC_NOT_MODIFIED);
       return;
     }
@@ -186,6 +193,12 @@ public abstract class ResourceServlet extends HttpServlet {
         tosend = gz;
       }
     }
+
+    if (cacheOnClient) {
+      rsp.setHeader(ETAG, r.etag);
+    } else {
+      CacheHeaders.setNotCacheable(rsp);
+    }
     if (!CacheHeaders.hasCacheHeader(rsp)) {
       if (e != null && r.etag.equals(e)) {
         CacheHeaders.setCacheable(req, rsp, 360, DAYS, false);
@@ -193,7 +206,6 @@ public abstract class ResourceServlet extends HttpServlet {
         CacheHeaders.setCacheable(req, rsp, 15, MINUTES, refresh);
       }
     }
-    rsp.setHeader(ETAG, r.etag);
     rsp.setContentType(r.contentType);
     rsp.setContentLength(tosend.length);
     try (OutputStream out = rsp.getOutputStream()) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/SingleFileServlet.java

@@ -29,6 +29,12 @@ class SingleFileServlet extends ResourceServlet {
     this.path = path;
   }
 
+  SingleFileServlet(Cache<Path, Resource> cache, Path path, boolean refresh,
+      boolean cacheOnClient) {
+    super(cache, refresh, cacheOnClient);
+    this.path = path;
+  }
+
   @Override
   protected Path getResourcePath(String pathInfo) {
     return path;

gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java

@@ -237,9 +237,10 @@ public class StaticModule extends ServletModule {
     @Named(POLYGERRIT_INDEX_SERVLET)
     HttpServlet getPolyGerritUiIndexServlet(
         @Named(CACHE) Cache<Path, Resource> cache) {
-      return new SingleFileServlet(
-          cache, polyGerritBasePath().resolve("index.html"),
-          getPaths().isDev());
+      return new SingleFileServlet(cache,
+          polyGerritBasePath().resolve("index.html"),
+          getPaths().isDev(),
+          false);
     }
 
     @Provides

gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/ResourceServletTest.java

@@ -63,9 +63,21 @@ public class ResourceServletTest {
       this.fs = fs;
     }
 
+    private Servlet(FileSystem fs, Cache<Path, Resource> cache,
+        boolean refresh, boolean cacheOnClient) {
+      super(cache, refresh, cacheOnClient);
+      this.fs = fs;
+    }
+
     private Servlet(FileSystem fs, Cache<Path, Resource> cache,
         boolean refresh, int cacheFileSizeLimitBytes) {
-      super(cache, refresh, cacheFileSizeLimitBytes);
+      super(cache, refresh, true, cacheFileSizeLimitBytes);
+      this.fs = fs;
+    }
+
+    private Servlet(FileSystem fs, Cache<Path, Resource> cache,
+        boolean refresh, boolean cacheOnClient, int cacheFileSizeLimitBytes) {
+      super(cache, refresh, cacheOnClient, cacheFileSizeLimitBytes);
       this.fs = fs;
     }
 
@@ -155,6 +167,37 @@ public class ResourceServletTest {
     assertCacheHits(cache, 2, 3);
   }
 
+  @Test
+  public void smallFileWithoutClientCache() throws Exception {
+    Cache<Path, Resource> cache = newCache(1);
+    Servlet servlet = new Servlet(fs, cache, false, false);
+
+    writeFile("/foo", "foo1");
+    FakeHttpServletResponse res = new FakeHttpServletResponse();
+    servlet.doGet(request("/foo"), res);
+    assertThat(res.getStatus()).isEqualTo(SC_OK);
+    assertThat(res.getActualBodyString()).isEqualTo("foo1");
+    assertNotCacheable(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");
+    assertNotCacheable(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");
+    assertNotCacheable(res);
+    assertCacheHits(cache, 2, 2);
+  }
+
   @Test
   public void smallFileWithoutRefresh() throws Exception {
     Cache<Path, Resource> cache = newCache(1);