diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/CacheControl.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/CacheControl.java index a8ffe94694..6c3e4fd8e2 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/CacheControl.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/CacheControl.java @@ -35,6 +35,7 @@ public class CacheControl { private final Type type; private final long age; private final TimeUnit unit; + private boolean mustRevalidate; private CacheControl(Type type, long age, TimeUnit unit) { this.type = type; @@ -53,4 +54,13 @@ public class CacheControl { public TimeUnit getUnit() { return unit; } + + public boolean isMustRevalidate() { + return mustRevalidate; + } + + public CacheControl setMustRevalidate() { + mustRevalidate = true; + return this; + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestResource.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestResource.java index 063a570be4..f245fac67c 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestResource.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestResource.java @@ -14,6 +14,8 @@ package com.google.gerrit.extensions.restapi; +import java.sql.Timestamp; + /** * Generic resource handle defining arguments to views. *

@@ -21,4 +23,9 @@ package com.google.gerrit.extensions.restapi; * {@link RestView} such as {@link RestReadView} or {@link RestModifyView}. */ public interface RestResource { + + /** A resource with a last modification date. */ + public interface HasLastModified { + public Timestamp getLastModified(); + } } diff --git a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/server/CacheHeaders.java b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/server/CacheHeaders.java index 11409e8ff3..d44405fd6c 100644 --- a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/server/CacheHeaders.java +++ b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/server/CacheHeaders.java @@ -59,10 +59,34 @@ public class CacheHeaders { public static void setCacheable( HttpServletRequest req, HttpServletResponse res, long age, TimeUnit unit) { + setCacheable(req, res, age, unit, false); + } + + /** + * Permit caching the response for up to the age specified. + *

+ * If the request is on a secure connection (e.g. SSL) private caching is + * used. This allows the user-agent to cache the response, but requests + * intermediate proxies to not cache. This may offer better protection for + * Set-Cookie headers. + *

+ * If the request is on plaintext (insecure), public caching is used. This may + * allow an intermediate proxy to cache the response, including any Set-Cookie + * header that may have also been included. + * + * @param req current request. + * @param res response being returned. + * @param age how long the response can be cached. + * @param unit time unit for age, usually {@link TimeUnit#SECONDS}. + * @param mustRevalidate true if the client must validate the cached entity. + */ + public static void setCacheable( + HttpServletRequest req, HttpServletResponse res, + long age, TimeUnit unit, boolean mustRevalidate) { if (req.isSecure()) { - setCacheablePrivate(res, age, unit); + setCacheablePrivate(res, age, unit, mustRevalidate); } else { - setCacheablePublic(res, age, unit); + setCacheablePublic(res, age, unit, mustRevalidate); } } @@ -76,15 +100,16 @@ public class CacheHeaders { * @param res response being returned. * @param age how long the response can be cached. * @param unit time unit for age, usually {@link TimeUnit#SECONDS}. + * @param mustRevalidate true if the client must validate the cached entity. */ public static void setCacheablePublic(HttpServletResponse res, - long age, TimeUnit unit) { + long age, TimeUnit unit, boolean mustRevalidate) { long now = System.currentTimeMillis(); long sec = maxAgeSeconds(age, unit); res.setDateHeader("Expires", now + SECONDS.toMillis(sec)); res.setDateHeader("Date", now); - cache(res, "public", age, unit); + cache(res, "public", age, unit, mustRevalidate); } /** @@ -93,20 +118,22 @@ public class CacheHeaders { * @param res response being returned. * @param age how long the response can be cached. * @param unit time unit for age, usually {@link TimeUnit#SECONDS}. + * @param mustRevalidate true if the client must validate the cached entity. */ public static void setCacheablePrivate(HttpServletResponse res, - long age, TimeUnit unit) { + long age, TimeUnit unit, boolean mustRevalidate) { long now = System.currentTimeMillis(); res.setDateHeader("Expires", now); res.setDateHeader("Date", now); - cache(res, "private", age, unit); + cache(res, "private", age, unit, mustRevalidate); } private static void cache(HttpServletResponse res, - String type, long age, TimeUnit unit) { + String type, long age, TimeUnit unit, boolean revalidate) { res.setHeader("Cache-Control", String.format( - "%s, max-age=%d", - type, maxAgeSeconds(age, unit))); + "%s, max-age=%d%s", + type, maxAgeSeconds(age, unit), + revalidate ? ", must-revalidate" : "")); } private static long maxAgeSeconds(long age, TimeUnit unit) { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index c671ec784f..58ee02c479 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -23,6 +23,7 @@ import static javax.servlet.http.HttpServletResponse.SC_CREATED; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; +import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_OK; import static javax.servlet.http.HttpServletResponse.SC_PRECONDITION_FAILED; @@ -109,6 +110,7 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.sql.Timestamp; import java.util.Collections; import java.util.List; import java.util.Map; @@ -279,6 +281,11 @@ public class RestApiServlet extends HttpServlet { checkAccessAnnotations(view.getClass()); } + if (notModified(req, rsrc)) { + res.sendError(SC_NOT_MODIFIED); + return; + } + Multimap config = LinkedHashMultimap.create(); ParameterParser.splitQueryString(req.getQueryString(), config, params); if (!globals.paramParser.get().parse(view, params, req, res)) { @@ -302,7 +309,7 @@ public class RestApiServlet extends HttpServlet { @SuppressWarnings("rawtypes") Response r = (Response) result; status = r.statusCode(); - configureCaching(req, res, r.caching()); + configureCaching(req, res, rsrc, r.caching()); } else if (result instanceof Response.Redirect) { CacheHeaders.setNotCacheable(res); res.sendRedirect(((Response.Redirect) result).location()); @@ -353,8 +360,20 @@ public class RestApiServlet extends HttpServlet { } } + private boolean notModified(HttpServletRequest req, RestResource rsrc) { + if (rsrc instanceof RestResource.HasLastModified + && "GET".equals(req.getMethod())) { + Timestamp m = ((RestResource.HasLastModified) rsrc).getLastModified(); + long d = req.getDateHeader(HttpHeaders.IF_MODIFIED_SINCE); + + // HTTP times are in seconds, database may have millisecond precision. + return d / 1000L == m.getTime() / 1000L; + } + return false; + } + private static void configureCaching(HttpServletRequest req, - HttpServletResponse res, CacheControl c) { + HttpServletResponse res, RestResource rsrc, CacheControl c) { if ("GET".equals(req.getMethod())) { switch (c.getType()) { case NONE: @@ -362,10 +381,16 @@ public class RestApiServlet extends HttpServlet { CacheHeaders.setNotCacheable(res); break; case PRIVATE: - CacheHeaders.setCacheablePrivate(res, c.getAge(), c.getUnit()); + addResourceStateHeaders(res, rsrc); + CacheHeaders.setCacheablePrivate(res, + c.getAge(), c.getUnit(), + c.isMustRevalidate()); break; case PUBLIC: - CacheHeaders.setCacheable(req, res, c.getAge(), c.getUnit()); + addResourceStateHeaders(res, rsrc); + CacheHeaders.setCacheable(req, res, + c.getAge(), c.getUnit(), + c.isMustRevalidate()); break; } } else { @@ -373,6 +398,15 @@ public class RestApiServlet extends HttpServlet { } } + private static void addResourceStateHeaders( + HttpServletResponse res, RestResource rsrc) { + if (rsrc instanceof RestResource.HasLastModified) { + res.setDateHeader( + HttpHeaders.LAST_MODIFIED, + ((RestResource.HasLastModified) rsrc).getLastModified().getTime()); + } + } + private void checkPreconditions(HttpServletRequest req, RestResource rsrc) throws PreconditionFailedException { if ("*".equals(req.getHeader("If-None-Match"))) { @@ -850,7 +884,7 @@ public class RestApiServlet extends HttpServlet { HttpServletResponse res, int statusCode, String msg, CacheControl c) throws IOException { res.setStatus(statusCode); - configureCaching(req, res, c); + configureCaching(req, res, null, c); replyText(null, res, msg); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java index be3081ab92..e2d3a1563a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java @@ -15,12 +15,15 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.restapi.RestResource; +import com.google.gerrit.extensions.restapi.RestResource.HasLastModified; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.project.ChangeControl; import com.google.inject.TypeLiteral; -public class ChangeResource implements RestResource { +import java.sql.Timestamp; + +public class ChangeResource implements RestResource, HasLastModified { public static final TypeLiteral> CHANGE_KIND = new TypeLiteral>() {}; @@ -41,4 +44,9 @@ public class ChangeResource implements RestResource { public Change getChange() { return getControl().getChange(); } + + @Override + public Timestamp getLastModified() { + return getChange().getLastUpdatedOn(); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java index 3d9c0fe139..2375fb0752 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java @@ -15,12 +15,16 @@ package com.google.gerrit.server.change; import com.google.gerrit.common.changes.ListChangesOption; +import com.google.gerrit.extensions.restapi.CacheControl; +import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import org.kohsuke.args4j.Option; +import java.util.concurrent.TimeUnit; + public class GetDetail implements RestReadView { private final ChangeJson json; @@ -45,6 +49,7 @@ public class GetDetail implements RestReadView { @Override public Object apply(ChangeResource rsrc) throws OrmException { - return json.format(rsrc); + return Response.ok(json.format(rsrc)) + .caching(CacheControl.PRIVATE(0, TimeUnit.SECONDS).setMustRevalidate()); } }