Allow user agents to cache /diff responses
The /diff response is expensive to compute at the server side, and is rather large to transfer to the client. Allow user agents to cache the resource locally for up to 7 days. This should eventually allow navigation within a patch set to be quicker as the browser will be able to load previously viewed files from cache, saving round-trip time to the server. Change-Id: I1c2f3e237a0c4c802564e5d5d87cd13d0d1b358f
This commit is contained in:
@@ -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";
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user