Make HEAD cheaper for RepositoryIndex
On HEAD requests avoid doing any log processing or content generation
and just issue back 200 OK. This allows clients to use HEAD to probe
for existance of a repository and get back 200 OK from the servlet or
404 Not Found from the higher level resolving filter.
Provide the same in HostIndexServlet to look for subdirectories of
repositories. If there are no repositories with that prefix return
404 Not Found.
Change-Id: Ie69a7da327f1c01a214ca8608580b51577ad9ce2
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java
index dbe3099..476e8cc 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java
@@ -25,6 +25,7 @@
import static javax.servlet.http.HttpServletResponse.SC_OK;
import static org.eclipse.jgit.util.HttpSupport.ENCODING_GZIP;
+import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -102,17 +103,12 @@
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse res)
throws IOException, ServletException {
- FormatType format;
- try {
- format = FormatType.getFormatType(req);
- } catch (IllegalArgumentException err) {
+ Optional<FormatType> format = getFormat(req);
+ if (!format.isPresent()) {
res.sendError(SC_BAD_REQUEST);
return;
}
- if (format == DEFAULT) {
- format = getDefaultFormat(req);
- }
- switch (format) {
+ switch (format.get()) {
case HTML:
doGetHtml(req, res);
break;
@@ -128,6 +124,14 @@
}
}
+ protected Optional<FormatType> getFormat(HttpServletRequest req) {
+ Optional<FormatType> format = FormatType.getFormatType(req);
+ if (format.isPresent() && format.get() == DEFAULT) {
+ return Optional.of(getDefaultFormat(req));
+ }
+ return format;
+ }
+
/**
* @param req in-progress request.
* @return the default {@link FormatType} used when {@code ?format=} is not
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/FormatType.java b/gitiles-servlet/src/main/java/com/google/gitiles/FormatType.java
index 9417a4c..1d2f61c 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/FormatType.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/FormatType.java
@@ -14,6 +14,8 @@
package com.google.gitiles;
+import com.google.common.base.Enums;
+import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.net.HttpHeaders;
@@ -28,38 +30,36 @@
private static final String FORMAT_TYPE_ATTRIBUTE = FormatType.class.getName();
- public static FormatType getFormatType(HttpServletRequest req) {
- FormatType result = (FormatType) req.getAttribute(FORMAT_TYPE_ATTRIBUTE);
+ public static Optional<FormatType> getFormatType(HttpServletRequest req) {
+ @SuppressWarnings("unchecked")
+ Optional<FormatType> result =
+ (Optional<FormatType>) req.getAttribute(FORMAT_TYPE_ATTRIBUTE);
if (result != null) {
return result;
}
- String format = req.getParameter("format");
- if (format != null) {
- for (FormatType type : FormatType.values()) {
- if (format.equalsIgnoreCase(type.name())) {
- return set(req, type);
- }
- }
- throw new IllegalArgumentException("Invalid format " + format);
+ String fmt = req.getParameter("format");
+ if (!Strings.isNullOrEmpty(fmt)) {
+ return set(req, Enums.getIfPresent(FormatType.class, fmt.toUpperCase()));
}
String accept = req.getHeader(HttpHeaders.ACCEPT);
if (Strings.isNullOrEmpty(accept)) {
- return set(req, DEFAULT);
+ return set(req, Optional.of(DEFAULT));
}
for (String p : accept.split("[ ,;][ ,;]*")) {
for (FormatType type : FormatType.values()) {
if (p.equals(type.mimeType)) {
- return set(req, type != HTML ? type : DEFAULT);
+ return set(req, Optional.of(type != HTML ? type : DEFAULT));
}
}
}
- return set(req, DEFAULT);
+ return set(req, Optional.of(DEFAULT));
}
- private static FormatType set(HttpServletRequest req, FormatType format) {
+ private static Optional<FormatType> set(HttpServletRequest req,
+ Optional<FormatType> format) {
req.setAttribute(FORMAT_TYPE_ATTRIBUTE, format);
return format;
}
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java
index 79d8927..2cc1351 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java
@@ -15,11 +15,13 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
+import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
@@ -104,6 +106,37 @@
}
@Override
+ protected void doHead(HttpServletRequest req, HttpServletResponse res)
+ throws IOException {
+ Optional<FormatType> format = getFormat(req);
+ if (!format.isPresent()) {
+ res.sendError(SC_BAD_REQUEST);
+ return;
+ }
+
+ GitilesView view = ViewFilter.getView(req);
+ String prefix = view.getRepositoryPrefix();
+ if (prefix != null) {
+ Map<String, RepositoryDescription> descs =
+ list(req, res, prefix, Collections.<String> emptySet());
+ if (descs == null) {
+ return;
+ }
+ }
+ switch (format.get()) {
+ case HTML:
+ case JSON:
+ case TEXT:
+ res.setStatus(HttpServletResponse.SC_OK);
+ res.setContentType(format.get().getMimeType());
+ break;
+ default:
+ res.sendError(SC_BAD_REQUEST);
+ break;
+ }
+ }
+
+ @Override
protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException {
GitilesView view = ViewFilter.getView(req);
String prefix = view.getRepositoryPrefix();
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
index 13185fa..d1dfbd8 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -15,7 +15,9 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -60,6 +62,28 @@
}
@Override
+ protected void doHead(HttpServletRequest req, HttpServletResponse res)
+ throws IOException {
+ // If the repository didn't exist a prior filter would have 404 replied.
+ Optional<FormatType> format = getFormat(req);
+ if (!format.isPresent()) {
+ res.sendError(SC_BAD_REQUEST);
+ return;
+ }
+ switch (format.get()) {
+ case HTML:
+ case JSON:
+ res.setStatus(HttpServletResponse.SC_OK);
+ res.setContentType(format.get().getMimeType());
+ break;
+ case TEXT:
+ default:
+ res.sendError(SC_BAD_REQUEST);
+ break;
+ }
+ }
+
+ @Override
protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException {
GitilesView view = ViewFilter.getView(req);
Repository repo = ServletUtils.getRepository(req);
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java
index 5890e78..343b270 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java
@@ -70,6 +70,7 @@
private ListMultimap<String, String> parameters;
private String hostName;
private int port;
+ private String method;
private String contextPath;
private String servletPath;
private String path;
@@ -79,6 +80,7 @@
this.hostName = checkNotNull(hostName, "hostName");
checkArgument(port > 0);
this.port = port;
+ this.method = "GET";
this.contextPath = checkNotNull(contextPath, "contextPath");
this.servletPath = checkNotNull(servletPath, "servletPath");
attributes = Maps.newConcurrentMap();
@@ -297,7 +299,11 @@
@Override
public String getMethod() {
- return "GET";
+ return method;
+ }
+
+ public void setMethod(String m) {
+ method = m;
}
@Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/HostIndexServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/HostIndexServletTest.java
index f96a37c..f98e7bf 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/HostIndexServletTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/HostIndexServletTest.java
@@ -34,6 +34,8 @@
import java.util.Map;
+import javax.servlet.http.HttpServletResponse;
+
/** Tests for {@link HostIndexServlet}. */
@RunWith(JUnit4.class)
public class HostIndexServletTest extends ServletTest {
@@ -152,4 +154,34 @@
public void emptySubdirectoryList() throws Exception {
assertNotFound("/no.repos/", null);
}
+
+ @Test
+ public void headOnRoot() throws Exception {
+ FakeHttpServletRequest req = FakeHttpServletRequest.newRequest();
+ req.setMethod("HEAD");
+ req.setPathInfo("/");
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
+ public void headOnMissingSubdir() throws Exception {
+ FakeHttpServletRequest req = FakeHttpServletRequest.newRequest();
+ req.setMethod("HEAD");
+ req.setPathInfo("/no.repos/");
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_NOT_FOUND);
+ }
+
+ @Test
+ public void headOnPopulatedSubdir() throws Exception {
+ FakeHttpServletRequest req = FakeHttpServletRequest.newRequest();
+ req.setMethod("HEAD");
+ req.setPathInfo("/foo/");
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
}
