Merge "Allow user agents to cache /diff responses"

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/Response.java

@@ -19,6 +19,10 @@ public abstract class Response<T> {
   @SuppressWarnings({"rawtypes"})
   private static final Response NONE = new None();
 
+  public enum CacheControl {
+    NONE, PUBLIC, PRIVATE;
+  }
+
   /** HTTP 200 OK: pointless wrapper for type safety. */
   public static <T> Response<T> ok(T value) {
     return new Impl<T>(200, value);
@@ -50,11 +54,14 @@ public abstract class Response<T> {
 
   public abstract int statusCode();
   public abstract T value();
+  public abstract CacheControl caching();
+  public abstract Response<T> caching(CacheControl c);
   public abstract String toString();
 
   private static final class Impl<T> extends Response<T> {
     private final int statusCode;
     private final T value;
+    private CacheControl caching = CacheControl.NONE;
 
     private Impl(int sc, T val) {
       statusCode = sc;
@@ -71,6 +78,17 @@ public abstract class Response<T> {
       return value;
     }
 
+    @Override
+    public CacheControl caching() {
+      return caching;
+    }
+
+    @Override
+    public Response<T> caching(CacheControl c) {
+      caching = c;
+      return this;
+    }
+
     @Override
     public String toString() {
       return "[" + statusCode() + "] " + value();
@@ -90,6 +108,16 @@ public abstract class Response<T> {
       throw new UnsupportedOperationException();
     }
 
+    @Override
+    public CacheControl caching() {
+      return CacheControl.NONE;
+    }
+
+    @Override
+    public Response<Object> caching(CacheControl c) {
+      throw new UnsupportedOperationException();
+    }
+
     @Override
     public String toString() {
       return "[204 No Content] None";

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/ParameterParser.java

@@ -33,6 +33,7 @@ import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonPrimitive;
+import com.google.gwtexpui.server.CacheHeaders;
 import com.google.inject.Inject;
 
 import org.kohsuke.args4j.CmdLineException;
@@ -79,6 +80,7 @@ class ParameterParser {
       msg.write('\n');
       clp.printUsage(msg, null);
       msg.write('\n');
+      CacheHeaders.setNotCacheable(res);
       replyBinaryResult(req, res,
           BinaryResult.create(msg.toString()).setContentType("text/plain"));
       return false;

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -112,6 +112,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 import java.util.zip.GZIPOutputStream;
 
@@ -184,7 +185,6 @@ public class RestApiServlet extends HttpServlet {
   protected final void service(HttpServletRequest req, HttpServletResponse res)
       throws ServletException, IOException {
     long auditStartTs = System.currentTimeMillis();
-    CacheHeaders.setNotCacheable(res);
     res.setHeader("Content-Disposition", "attachment");
     res.setHeader("X-Content-Type-Options", "nosniff");
     int status = SC_OK;
@@ -302,9 +302,13 @@ public class RestApiServlet extends HttpServlet {
         @SuppressWarnings("rawtypes")
         Response r = (Response) result;
         status = r.statusCode();
+        configureCaching(req, res, r);
       } else if (result instanceof Response.Redirect) {
+        CacheHeaders.setNotCacheable(res);
         res.sendRedirect(((Response.Redirect) result).location());
         return;
+      } else {
+        CacheHeaders.setNotCacheable(res);
       }
       res.setStatus(status);
 
@@ -349,6 +353,26 @@ public class RestApiServlet extends HttpServlet {
     }
   }
 
+  private static <T> void configureCaching(HttpServletRequest req,
+      HttpServletResponse res, Response<T> r) {
+    if ("GET".equals(req.getMethod())) {
+      switch (r.caching()) {
+        case NONE:
+        default:
+          CacheHeaders.setNotCacheable(res);
+          break;
+        case PRIVATE:
+          CacheHeaders.setCacheablePrivate(res, 7, TimeUnit.DAYS);
+          break;
+        case PUBLIC:
+          CacheHeaders.setCacheable(req, res, 7, TimeUnit.DAYS);
+          break;
+      }
+    } else {
+      CacheHeaders.setNotCacheable(res);
+    }
+  }
+
   private void checkPreconditions(HttpServletRequest req, RestResource rsrc)
       throws PreconditionFailedException {
     if ("*".equals(req.getHeader("If-None-Match"))) {
@@ -818,6 +842,7 @@ public class RestApiServlet extends HttpServlet {
   static void replyError(HttpServletResponse res, int statusCode, String msg)
       throws IOException {
     res.setStatus(statusCode);
+    CacheHeaders.setNotCacheable(res);
     replyText(null, res, msg);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java

@@ -24,6 +24,7 @@ import com.google.gerrit.common.data.PatchScript.DisplayMethod;
 import com.google.gerrit.common.data.PatchScript.FileMode;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.prettify.common.SparseFileContent;
 import com.google.gerrit.reviewdb.client.Account;
@@ -149,7 +150,7 @@ public class GetDiff implements RestReadView<FileResource> {
       result.diffHeader = ps.getPatchHeader();
     }
     result.content = content.lines;
-    return result;
+    return Response.ok(result).caching(Response.CacheControl.PRIVATE);
   }
 
   static class Result {