Use ETag and If-None-Match for /detail caching
Last-Modified is only second level precision and may allow a client to fail to see subsequent updates within the same second window. Improve the caching check by using an ETag string that is computed from the full timestamp and the row version fields of Change. Change-Id: I58fc95b5396baa1d8afb676ff672c9f19b835f1c
This commit is contained in:
@@ -26,6 +26,15 @@ public interface RestResource {
|
|||||||
|
|
||||||
/** A resource with a last modification date. */
|
/** A resource with a last modification date. */
|
||||||
public interface HasLastModified {
|
public interface HasLastModified {
|
||||||
|
/**
|
||||||
|
* @return time for the Last-Modified header. HTTP truncates the header
|
||||||
|
* value to seconds.
|
||||||
|
*/
|
||||||
public Timestamp getLastModified();
|
public Timestamp getLastModified();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** A resource with an ETag. */
|
||||||
|
public interface HasETag {
|
||||||
|
public String getETag();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -360,9 +360,19 @@ public class RestApiServlet extends HttpServlet {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean notModified(HttpServletRequest req, RestResource rsrc) {
|
private static boolean notModified(HttpServletRequest req, RestResource rsrc) {
|
||||||
if (rsrc instanceof RestResource.HasLastModified
|
if (!"GET".equals(req.getMethod())) {
|
||||||
&& "GET".equals(req.getMethod())) {
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (rsrc instanceof RestResource.HasETag) {
|
||||||
|
String have = req.getHeader(HttpHeaders.IF_NONE_MATCH);
|
||||||
|
if (have != null) {
|
||||||
|
return have.equals(((RestResource.HasETag) rsrc).getETag());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (rsrc instanceof RestResource.HasLastModified) {
|
||||||
Timestamp m = ((RestResource.HasLastModified) rsrc).getLastModified();
|
Timestamp m = ((RestResource.HasLastModified) rsrc).getLastModified();
|
||||||
long d = req.getDateHeader(HttpHeaders.IF_MODIFIED_SINCE);
|
long d = req.getDateHeader(HttpHeaders.IF_MODIFIED_SINCE);
|
||||||
|
|
||||||
@@ -400,6 +410,11 @@ public class RestApiServlet extends HttpServlet {
|
|||||||
|
|
||||||
private static void addResourceStateHeaders(
|
private static void addResourceStateHeaders(
|
||||||
HttpServletResponse res, RestResource rsrc) {
|
HttpServletResponse res, RestResource rsrc) {
|
||||||
|
if (rsrc instanceof RestResource.HasETag) {
|
||||||
|
res.setHeader(
|
||||||
|
HttpHeaders.ETAG,
|
||||||
|
((RestResource.HasETag) rsrc).getETag());
|
||||||
|
}
|
||||||
if (rsrc instanceof RestResource.HasLastModified) {
|
if (rsrc instanceof RestResource.HasLastModified) {
|
||||||
res.setDateHeader(
|
res.setDateHeader(
|
||||||
HttpHeaders.LAST_MODIFIED,
|
HttpHeaders.LAST_MODIFIED,
|
||||||
|
|||||||
@@ -431,6 +431,10 @@ public final class Change {
|
|||||||
lastUpdatedOn = now;
|
lastUpdatedOn = now;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public int getRowVersion() {
|
||||||
|
return rowVersion;
|
||||||
|
}
|
||||||
|
|
||||||
public void resetLastUpdatedOn() {
|
public void resetLastUpdatedOn() {
|
||||||
lastUpdatedOn = new Timestamp(System.currentTimeMillis());
|
lastUpdatedOn = new Timestamp(System.currentTimeMillis());
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -15,6 +15,7 @@
|
|||||||
package com.google.gerrit.server.change;
|
package com.google.gerrit.server.change;
|
||||||
|
|
||||||
import com.google.gerrit.extensions.restapi.RestResource;
|
import com.google.gerrit.extensions.restapi.RestResource;
|
||||||
|
import com.google.gerrit.extensions.restapi.RestResource.HasETag;
|
||||||
import com.google.gerrit.extensions.restapi.RestResource.HasLastModified;
|
import com.google.gerrit.extensions.restapi.RestResource.HasLastModified;
|
||||||
import com.google.gerrit.extensions.restapi.RestView;
|
import com.google.gerrit.extensions.restapi.RestView;
|
||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
@@ -23,7 +24,8 @@ import com.google.inject.TypeLiteral;
|
|||||||
|
|
||||||
import java.sql.Timestamp;
|
import java.sql.Timestamp;
|
||||||
|
|
||||||
public class ChangeResource implements RestResource, HasLastModified {
|
public class ChangeResource implements RestResource,
|
||||||
|
HasLastModified, HasETag {
|
||||||
public static final TypeLiteral<RestView<ChangeResource>> CHANGE_KIND =
|
public static final TypeLiteral<RestView<ChangeResource>> CHANGE_KIND =
|
||||||
new TypeLiteral<RestView<ChangeResource>>() {};
|
new TypeLiteral<RestView<ChangeResource>>() {};
|
||||||
|
|
||||||
@@ -49,4 +51,11 @@ public class ChangeResource implements RestResource, HasLastModified {
|
|||||||
public Timestamp getLastModified() {
|
public Timestamp getLastModified() {
|
||||||
return getChange().getLastUpdatedOn();
|
return getChange().getLastUpdatedOn();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public String getETag() {
|
||||||
|
return String.format("%x-%x",
|
||||||
|
getChange().getLastUpdatedOn().getTime(),
|
||||||
|
getChange().getRowVersion());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user