Merge "Refactor XDocLoader to make the code better readable and extensible"
diff --git a/BUCK b/BUCK
index 99a8eca..cccde03 100644
--- a/BUCK
+++ b/BUCK
@@ -2,7 +2,7 @@
MODULE = 'com.googlesource.gerrit.plugins.xdocs.XDocs'
-ASCIIDOCTOR = '//lib/asciidoctor:asciidoc_lib' if __standalone_mode__ \
+ASCIIDOCTOR = '//lib/asciidoctor:asciidoc_lib' if STANDALONE_MODE \
else '//plugins/x-docs/lib/asciidoctor:asciidoc_lib'
gerrit_plugin(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java
index f557bee..78520f7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java
@@ -23,6 +23,7 @@
import com.google.common.net.HttpHeaders;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.httpd.resources.Resource;
import com.google.gerrit.httpd.resources.SmallResource;
@@ -106,104 +107,52 @@
@Override
public void service(HttpServletRequest req, HttpServletResponse res)
throws IOException {
- if (!"GET".equals(req.getMethod()) && !"HEAD".equals(req.getMethod())) {
- CacheHeaders.setNotCacheable(res);
- res.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
- return;
- }
+ try {
+ validateRequestMethod(req);
- ResourceKey key = ResourceKey.fromPath(req.getPathInfo());
- ProjectState state = projectCache.get(key.project);
- if (state == null) {
- Resource.NOT_FOUND.send(req, res);
- return;
- }
- XDocProjectConfig cfg = cfgFactory.create(state);
- if (key.file == null) {
- res.sendRedirect(getRedirectUrl(req, key, cfg));
- return;
- }
+ ResourceKey key = ResourceKey.fromPath(req.getPathInfo());
+ ProjectState state = getProject(key);
+ XDocProjectConfig cfg = cfgFactory.create(state);
- MimeType mimeType = fileTypeRegistry.getMimeType(key.file, null);
- FormatterProvider formatter;
- if (req.getParameter("raw") != null) {
- formatter = formatters.getRawFormatter();
- } else {
- formatter = formatters.get(state, key.file);
- if (formatter == null
- && !("image".equals(mimeType.getMediaType())
- && fileTypeRegistry.isSafeInline(mimeType))) {
- Resource.NOT_FOUND.send(req, res);
+ if (key.file == null) {
+ res.sendRedirect(getRedirectUrl(req, key, cfg));
return;
}
- }
- try {
+ MimeType mimeType = fileTypeRegistry.getMimeType(key.file, null);
+ FormatterProvider formatter = getFormatter(req, key);
+ if (formatter == null && !isSafeImage(mimeType)) {
+ throw new ResourceNotFoundException();
+ }
+
ProjectControl projectControl = projectControlFactory.validateFor(key.project);
- String rev = key.revision;
- if (rev == null) {
- rev = cfg.getIndexRef();
- }
- if (Constants.HEAD.equals(rev)) {
- rev = getHead.get().apply(new ProjectResource(projectControl));
- } else {
- if (!ObjectId.isId(rev)) {
- if (!rev.startsWith(Constants.R_REFS)) {
- rev = Constants.R_HEADS + rev;
- }
- if (!projectControl.controlForRef(rev).isVisible()) {
- Resource.NOT_FOUND.send(req, res);
- return;
- }
- }
- }
+ String rev = getRevision(cfg, key.revision, projectControl);
Repository repo = repoManager.openRepository(key.project);
try {
- ObjectId revId =
- repo.resolve(MoreObjects.firstNonNull(rev, Constants.HEAD));
- if (revId == null) {
- Resource.NOT_FOUND.send(req, res);
- return;
- }
+ ObjectId revId = resolveRevision(repo, rev);
if (ObjectId.isId(rev)) {
- RevWalk rw = new RevWalk(repo);
- try {
- RevCommit commit = rw.parseCommit(repo.resolve(rev));
- if (!projectControl.canReadCommit(db.get(), rw, commit)) {
- Resource.NOT_FOUND.send(req, res);
- return;
- }
- } finally {
- rw.release();
- }
+ validateCanReadCommit(repo, projectControl, revId);
}
- String eTag = null;
- String receivedETag = req.getHeader(HttpHeaders.IF_NONE_MATCH);
- if (receivedETag != null) {
- eTag = computeETag(key.project, revId, key.file);
- if (eTag.equals(receivedETag)) {
- res.sendError(SC_NOT_MODIFIED);
- return;
- }
+ if (isResourceNotModified(req, key, revId)) {
+ res.sendError(SC_NOT_MODIFIED);
+ return;
}
Resource rsc;
if (formatter != null) {
rsc = docCache.get(formatter, key.project, key.file, revId);
- } else if ("image".equals(mimeType.getMediaType())) {
+ } else if (isImage(mimeType)) {
rsc = getImageResource(repo, revId, key.file);
} else {
rsc = Resource.NOT_FOUND;
}
if (rsc != Resource.NOT_FOUND) {
- res.setHeader(
- HttpHeaders.ETAG,
- MoreObjects.firstNonNull(eTag,
- computeETag(key.project, revId, key.file)));
+ res.setHeader(HttpHeaders.ETAG,
+ computeETag(key.project, revId, key.file));
}
CacheHeaders.setCacheablePrivate(res, 7, TimeUnit.DAYS, false);
rsc.send(req, res);
@@ -214,7 +163,9 @@
} catch (RepositoryNotFoundException | NoSuchProjectException
| ResourceNotFoundException | AuthException | RevisionSyntaxException e) {
Resource.NOT_FOUND.send(req, res);
- return;
+ } catch (MethodNotAllowedException e) {
+ CacheHeaders.setNotCacheable(res);
+ res.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
}
}
@@ -236,8 +187,7 @@
byte[] content = loader.getBytes(Integer.MAX_VALUE);
MimeType mimeType = fileTypeRegistry.getMimeType(file, content);
- if (!"image".equals(mimeType.getMediaType())
- || !fileTypeRegistry.isSafeInline(mimeType)) {
+ if (!isSafeImage(mimeType)) {
return Resource.NOT_FOUND;
}
return new SmallResource(content)
@@ -254,6 +204,94 @@
}
}
+ private static void validateRequestMethod(HttpServletRequest req)
+ throws MethodNotAllowedException {
+ if (!("GET".equals(req.getMethod()) || "HEAD".equals(req.getMethod()))) {
+ throw new MethodNotAllowedException();
+ }
+ }
+
+ private ProjectState getProject(ResourceKey key)
+ throws ResourceNotFoundException {
+ ProjectState state = projectCache.get(key.project);
+ if (state == null) {
+ throw new ResourceNotFoundException();
+ }
+ return state;
+ }
+
+ private FormatterProvider getFormatter(HttpServletRequest req, ResourceKey key)
+ throws ResourceNotFoundException {
+ if (req.getParameter("raw") != null) {
+ return formatters.getRawFormatter();
+ } else {
+ return formatters.get(getProject(key), key.file);
+ }
+ }
+
+ private boolean isSafeImage(MimeType mimeType) {
+ return isImage(mimeType) && fileTypeRegistry.isSafeInline(mimeType);
+ }
+
+ private static boolean isImage(MimeType mimeType) {
+ return "image".equals(mimeType.getMediaType());
+ }
+
+ private String getRevision(XDocProjectConfig cfg, String revision,
+ ProjectControl projectControl) throws ResourceNotFoundException,
+ AuthException, IOException {
+ String rev = revision;
+ if (rev == null) {
+ rev = cfg.getIndexRef();
+ }
+ if (Constants.HEAD.equals(rev)) {
+ rev = getHead.get().apply(new ProjectResource(projectControl));
+ } else {
+ if (!ObjectId.isId(rev)) {
+ if (!rev.startsWith(Constants.R_REFS)) {
+ rev = Constants.R_HEADS + rev;
+ }
+ if (!projectControl.controlForRef(rev).isVisible()) {
+ throw new ResourceNotFoundException();
+ }
+ }
+ }
+ return rev;
+ }
+
+ private static ObjectId resolveRevision(Repository repo, String revision)
+ throws ResourceNotFoundException, IOException {
+ ObjectId revId =
+ repo.resolve(MoreObjects.firstNonNull(revision, Constants.HEAD));
+ if (revId == null) {
+ throw new ResourceNotFoundException();
+ }
+ return revId;
+ }
+
+ private void validateCanReadCommit(Repository repo,
+ ProjectControl projectControl, ObjectId revId)
+ throws ResourceNotFoundException, IOException {
+ RevWalk rw = new RevWalk(repo);
+ try {
+ RevCommit commit = rw.parseCommit(revId);
+ if (!projectControl.canReadCommit(db.get(), rw, commit)) {
+ throw new ResourceNotFoundException();
+ }
+ } finally {
+ rw.release();
+ }
+ }
+
+ private static boolean isResourceNotModified(HttpServletRequest req,
+ ResourceKey key, ObjectId revId) {
+ String receivedETag = req.getHeader(HttpHeaders.IF_NONE_MATCH);
+ if (receivedETag != null) {
+ return receivedETag.equals(computeETag(key.project, revId, key.file));
+ }
+ return false;
+ }
+
private static String computeETag(Project.NameKey project, ObjectId revId,
String file) {
return Hashing.md5().newHasher()
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocScreen.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocScreen.java
index 92fa3ee..0bfa9c0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocScreen.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocScreen.java
@@ -53,8 +53,11 @@
setStyleName("xdocs-panel");
HorizontalPanel p = new HorizontalPanel();
+ p.setStyleName("xdocs-header");
p.add(new InlineHyperlink(projectName, "/admin/projects/" + projectName));
- p.add(new Label(" / " + fileName + " (" + revision + ")"));
+ p.add(new Label("/"));
+ p.add(new Label(fileName));
+ p.add(new Label("(" + revision + ")"));
add(p);
final String url = getUrl(projectName, revision, fileName);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/public/xdocs.css b/src/main/java/com/googlesource/gerrit/plugins/xdocs/public/xdocs.css
index cf984d4..42471c7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/public/xdocs.css
+++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/public/xdocs.css
@@ -3,8 +3,14 @@
width: 100%;
}
+.xdocs-header td {
+ padding-right: 5px;
+}
+
.xdocs-panel iframe {
width: 100%;
+ border-width: 1px;
+ border-style: solid;
}
.xdocs-error {
