Hoist FormatType switching into BaseServlet We want to support rendering to different formats (particularly JSON) for all views, not just the host index view. At the moment we are still maintaining a 1:1 mapping between view types and servlets, so supporting JSON output for a particular already-existing view means putting JSON code and HTML code in the same servlet class. Return 400s for unsupported values for ?format= by default. Change-Id: I5e97b6ba72e310bc66cbfb4a540d94759cbd4273
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 c92579d..a2bc65c 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/BaseServlet.java
@@ -14,18 +14,30 @@ package com.google.gitiles; +import static com.google.gitiles.FormatType.DEFAULT; +import static com.google.gitiles.FormatType.HTML; +import static com.google.gitiles.FormatType.JSON; +import static com.google.gitiles.FormatType.TEXT; + +import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; +import static javax.servlet.http.HttpServletResponse.SC_OK; import com.google.common.base.Charsets; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.net.HttpHeaders; +import com.google.gson.FieldNamingPolicy; +import com.google.gson.GsonBuilder; import org.joda.time.Instant; import java.io.IOException; +import java.io.PrintWriter; +import java.lang.reflect.Type; import java.util.Map; +import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -60,6 +72,77 @@ } } + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + FormatType format; + try { + format = FormatType.getFormatType(req); + } catch (IllegalArgumentException err) { + res.sendError(SC_BAD_REQUEST); + return; + } + if (format == DEFAULT) { + format = getDefaultFormat(req); + } + switch (format) { + case HTML: + doGetHtml(req, res); + break; + case TEXT: + doGetText(req, res); + break; + case JSON: + doGetJson(req, res); + break; + default: + res.sendError(SC_BAD_REQUEST); + break; + } + } + + /** + * @param req in-progress request. + * @return the default {@link FormatType} used when {@code ?format=} is not + * specified. + */ + protected FormatType getDefaultFormat(HttpServletRequest req) { + return HTML; + } + + /** + * Handle a GET request when the requested format type was HTML. + * + * @param req in-progress request. + * @param res in-progress response. + */ + protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + res.sendError(SC_BAD_REQUEST); + } + + /** + * Handle a GET request when the requested format type was plain text. + * + * @param req in-progress request. + * @param res in-progress response. + */ + protected void doGetText(HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + res.sendError(SC_BAD_REQUEST); + } + + /** + * Handle a GET request when the requested format type was JSON. + * + * @param req in-progress request. + * @param res in-progress response. + */ + protected void doGetJson(HttpServletRequest req, HttpServletResponse res) + throws IOException, ServletException { + res.sendError(SC_BAD_REQUEST); + } + protected static Map<String, Object> getData(HttpServletRequest req) { @SuppressWarnings("unchecked") Map<String, Object> data = (Map<String, Object>) req.getAttribute(DATA_ATTRIBUTE); @@ -91,7 +174,16 @@ getData(req).put(key, value); } - protected void render(HttpServletRequest req, HttpServletResponse res, String templateName, + /** + * Render data to HTML using Soy. + * + * @param req in-progress request. + * @param res in-progress response. + * @param key key. + * @param templateName Soy template name; must be in one of the template files + * defined in {@link Renderer#SOY_FILENAMES}. + */ + protected void renderHtml(HttpServletRequest req, HttpServletResponse res, String templateName, Map<String, ?> soyData) throws IOException { try { res.setContentType(FormatType.HTML.getMimeType()); @@ -115,6 +207,57 @@ } } + /** + * Render data to JSON using GSON. + * + * @param req in-progress request. + * @param res in-progress response. + * @param src @see Gson#toJson(Object, Type, Appendable) + * @param typeOfSrc @see Gson#toJson(Object, Type, Appendable) + */ + protected void renderJson(HttpServletRequest req, HttpServletResponse res, Object src, + Type typeOfSrc) throws IOException { + res.setContentType(JSON.getMimeType()); + res.setCharacterEncoding("UTF-8"); + res.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment"); + setCacheHeaders(req, res); + res.setStatus(SC_OK); + + PrintWriter writer = res.getWriter(); + new GsonBuilder() + .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) + .setPrettyPrinting() + .generateNonExecutableJson() + .create() + .toJson(src, typeOfSrc, writer); + writer.print('\n'); + writer.close(); + } + + /** + * Prepare the response to render plain text. + * <p> + * Unlike + * {@link #renderHtml(HttpServletRequest, HttpServletResponse, String, Map)} + * and + * {@link #renderJson(HttpServletRequest, HttpServletResponse, Object, Type)}, + * which assume the data to render is already completely prepared, this method + * does not write any data, only headers, and returns the response's + * ready-to-use writer. + * + * @param req in-progress request. + * @param res in-progress response. + * @return the response's writer. + */ + protected PrintWriter startRenderText(HttpServletRequest req, HttpServletResponse res) + throws IOException { + res.setContentType(TEXT.getMimeType()); + res.setCharacterEncoding("UTF-8"); + res.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment"); + setCacheHeaders(req, res); + return res.getWriter(); + } + protected void setCacheHeaders(HttpServletRequest req, HttpServletResponse res) { setNotCacheable(res); }
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 598527c..a046ff1 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/HostIndexServlet.java
@@ -15,9 +15,7 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.gitiles.FormatType.JSON; -import static com.google.gitiles.FormatType.TEXT; -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; @@ -26,9 +24,6 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; -import com.google.common.net.HttpHeaders; -import com.google.gson.FieldNamingPolicy; -import com.google.gson.GsonBuilder; import com.google.gson.reflect.TypeToken; import com.google.template.soy.data.SoyListData; import com.google.template.soy.data.SoyMapData; @@ -64,55 +59,36 @@ this.accessFactory = checkNotNull(accessFactory, "accessFactory"); } - @Override - protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException { - FormatType format; - try { - format = FormatType.getFormatType(req); - } catch (IllegalArgumentException err) { - res.sendError(SC_BAD_REQUEST); - return; - } + private Map<String, RepositoryDescription> getDescriptions(HttpServletRequest req, + HttpServletResponse res) throws IOException { + return getDescriptions(req, res, parseShowBranch(req)); + } - Set<String> branches = parseShowBranch(req); + private Map<String, RepositoryDescription> getDescriptions(HttpServletRequest req, + HttpServletResponse res, Set<String> branches) throws IOException { Map<String, RepositoryDescription> descs; try { descs = accessFactory.forRequest(req).listRepositories(branches); } catch (RepositoryNotFoundException e) { res.sendError(SC_NOT_FOUND); - return; + return null; } catch (ServiceNotEnabledException e) { res.sendError(SC_FORBIDDEN); - return; + return null; } catch (ServiceNotAuthorizedException e) { res.sendError(SC_UNAUTHORIZED); - return; + return null; } catch (ServiceMayNotContinueException e) { // TODO(dborowitz): Show the error message to the user. res.sendError(SC_FORBIDDEN); - return; + return null; } catch (IOException err) { String name = urls.getHostName(req); log.warn("Cannot scan repositories" + (name != null ? "for " + name : ""), err); res.sendError(SC_SERVICE_UNAVAILABLE); - return; + return null; } - - switch (format) { - case HTML: - case DEFAULT: - default: - displayHtml(req, res, descs); - break; - - case TEXT: - displayText(req, res, branches, descs); - break; - - case JSON: - displayJson(req, res, descs); - break; - } + return descs; } private SoyMapData toSoyMapData(RepositoryDescription desc, GitilesView view) { @@ -125,27 +101,32 @@ .toUrl()); } - private void displayHtml(HttpServletRequest req, HttpServletResponse res, - Map<String, RepositoryDescription> descs) throws IOException { + @Override + protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { + Map<String, RepositoryDescription> descs = getDescriptions(req, res); + if (descs == null) { + return; + } SoyListData repos = new SoyListData(); for (RepositoryDescription desc : descs.values()) { repos.add(toSoyMapData(desc, ViewFilter.getView(req))); } - render(req, res, "gitiles.hostIndex", ImmutableMap.of( + renderHtml(req, res, "gitiles.hostIndex", ImmutableMap.of( "hostName", urls.getHostName(req), "baseUrl", urls.getBaseGitUrl(req), "repositories", repos)); } - private void displayText(HttpServletRequest req, HttpServletResponse res, - Set<String> branches, Map<String, RepositoryDescription> descs) throws IOException { - res.setContentType(TEXT.getMimeType()); - res.setCharacterEncoding("UTF-8"); - res.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment"); - setNotCacheable(res); + @Override + protected void doGetText(HttpServletRequest req, HttpServletResponse res) throws IOException { + Set<String> branches = parseShowBranch(req); + Map<String, RepositoryDescription> descs = getDescriptions(req, res, branches); + if (descs == null) { + return; + } - PrintWriter writer = res.getWriter(); + PrintWriter writer = startRenderText(req, res); for (RepositoryDescription repo : descs.values()) { for (String name : branches) { String ref = repo.branches.get(name); @@ -163,24 +144,13 @@ writer.close(); } - private void displayJson(HttpServletRequest req, HttpServletResponse res, - Map<String, RepositoryDescription> descs) throws IOException { - res.setContentType(JSON.getMimeType()); - res.setCharacterEncoding("UTF-8"); - res.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment"); - setNotCacheable(res); - - PrintWriter writer = res.getWriter(); - new GsonBuilder() - .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) - .setPrettyPrinting() - .generateNonExecutableJson() - .create() - .toJson(descs, - new TypeToken<Map<String, RepositoryDescription>>() {}.getType(), - writer); - writer.print('\n'); - writer.close(); + @Override + protected void doGetJson(HttpServletRequest req, HttpServletResponse res) throws IOException { + Map<String, RepositoryDescription> descs = getDescriptions(req, res); + if (descs == null) { + return; + } + renderJson(req, res, descs, new TypeToken<Map<String, RepositoryDescription>>() {}.getType()); } private static Set<String> parseShowBranch(HttpServletRequest req) {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java index 748c5c9..46b6c44 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/LogServlet.java
@@ -119,7 +119,7 @@ data.put("title", title); - render(req, res, "gitiles.logDetail", data); + renderHtml(req, res, "gitiles.logDetail", data); } catch (RevWalkException e) { log.warn("Error in rev walk", e); res.setStatus(SC_INTERNAL_SERVER_ERROR);
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/PathServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/PathServlet.java index 387e342..fff423a 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/PathServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/PathServlet.java
@@ -284,7 +284,7 @@ } } // TODO(sop): Allow caching trees by SHA-1 when no S cookie is sent. - render(req, res, "gitiles.pathDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.pathDetail", ImmutableMap.of( "title", !view.getTreePath().isEmpty() ? view.getTreePath() : "/", "breadcrumbs", view.getBreadcrumbs(hasSingleTree), "type", FileType.TREE.toString(), @@ -305,7 +305,7 @@ List<Boolean> hasSingleTree) throws IOException { GitilesView view = ViewFilter.getView(req); // TODO(sop): Allow caching files by SHA-1 when no S cookie is sent. - render(req, res, "gitiles.pathDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.pathDetail", ImmutableMap.of( "title", ViewFilter.getView(req).getTreePath(), "breadcrumbs", view.getBreadcrumbs(hasSingleTree), "type", FileType.forEntry(tw).toString(), @@ -328,7 +328,7 @@ data.put("sha", ObjectId.toString(id)); data.put("data", null); data.put("size", Long.toString(loader.getSize())); - render(req, res, "gitiles.pathDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.pathDetail", ImmutableMap.of( "title", ViewFilter.getView(req).getTreePath(), "breadcrumbs", view.getBreadcrumbs(hasSingleTree), "type", FileType.REGULAR_FILE.toString(), @@ -349,7 +349,7 @@ } // TODO(sop): Allow caching files by SHA-1 when no S cookie is sent. - render(req, res, "gitiles.pathDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.pathDetail", ImmutableMap.of( "title", ViewFilter.getView(req).getTreePath(), "breadcrumbs", view.getBreadcrumbs(hasSingleTree), "type", FileType.SYMLINK.toString(), @@ -396,7 +396,7 @@ } // TODO(sop): Allow caching links by SHA-1 when no S cookie is sent. - render(req, res, "gitiles.pathDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.pathDetail", ImmutableMap.of( "title", view.getTreePath(), "type", FileType.GITLINK.toString(), "data", data));
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RefServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RefServlet.java index 959a449..bb51f04 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/RefServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/RefServlet.java
@@ -60,7 +60,7 @@ } finally { walk.release(); } - render(req, res, "gitiles.refsDetail", + renderHtml(req, res, "gitiles.refsDetail", ImmutableMap.of("branches", getBranches(req, 0), "tags", tags)); }
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 eb61a25..9672815 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -55,7 +55,7 @@ @Override protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException { - render(req, res, "gitiles.repositoryIndex", buildData(req)); + renderHtml(req, res, "gitiles.repositoryIndex", buildData(req)); } @VisibleForTesting
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java index c2eacdf..e4258c0 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
@@ -110,7 +110,7 @@ } } - render(req, res, "gitiles.revisionDetail", ImmutableMap.of( + renderHtml(req, res, "gitiles.revisionDetail", ImmutableMap.of( "title", view.getRevision().getName(), "objects", soyObjects, "hasBlob", hasBlob));