Merge "Redirect to normalized versions of parent revision expressions"
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
index fc553c7..d3bf7cf 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -196,7 +196,8 @@
case LOG:
break;
default:
- checkState(revision == null, "cannot set old revision on %s view", type);
+ revision = Objects.firstNonNull(revision, Revision.NULL);
+ checkState(revision == Revision.NULL, "cannot set old revision on %s view", type);
break;
}
this.oldRevision = revision;
@@ -212,7 +213,7 @@
}
public Revision getOldRevision() {
- return revision;
+ return oldRevision;
}
public Builder setPathPart(String path) {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java b/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
index 3fdf777..a93d4b5 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/Revision.java
@@ -43,6 +43,14 @@
/** Common default branch given to clients. */
public static final Revision HEAD = named("HEAD");
+ public static Revision normalizeParentExpressions(Revision rev) {
+ if (rev == null
+ || (rev.name.indexOf("~") < 0 && rev.name.indexOf("^") < 0)) {
+ return rev;
+ }
+ return new Revision(rev.id.name(), rev.id, rev.type, rev.peeledId, rev.peeledType);
+ }
+
private final String name;
private final ObjectId id;
private final int type;
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
index 13537dc..e8b1055 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
@@ -106,11 +106,16 @@
res.setStatus(SC_NOT_FOUND);
return;
}
+
@SuppressWarnings("unchecked")
Map<String, String[]> params = req.getParameterMap();
view.setHostName(urls.getHostName(req))
.setServletPath(req.getContextPath() + req.getServletPath())
.putAllParams(params);
+ if (normalize(view, res)) {
+ return;
+ }
+
setView(req, view.build());
try {
chain.doFilter(req, res);
@@ -119,6 +124,20 @@
}
}
+ private boolean normalize(GitilesView.Builder view, HttpServletResponse res)
+ throws IOException {
+ if (view.getOldRevision() != Revision.NULL) {
+ return false;
+ }
+ Revision r = view.getRevision();
+ Revision nr = Revision.normalizeParentExpressions(r);
+ if (r != nr) {
+ res.sendRedirect(view.setRevision(nr).toUrl());
+ return true;
+ }
+ return false;
+ }
+
private GitilesView.Builder parse(HttpServletRequest req) throws IOException {
String repoName = trimLeadingSlash(getRegexGroup(req, 1));
if (repoName.isEmpty()) {
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java
index 85da54f..558eb4c 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java
@@ -208,8 +208,9 @@
}
@Override
- public synchronized void sendRedirect(String msg) {
+ public synchronized void sendRedirect(String loc) {
status = SC_FOUND;
+ setHeader(HttpHeaders.LOCATION, loc);
committed = true;
}
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
index 4e80c39..2660541 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
@@ -20,6 +20,7 @@
import static com.google.gitiles.GitilesFilter.ROOT_REGEX;
import com.google.common.collect.ImmutableList;
+import com.google.common.net.HttpHeaders;
import com.google.common.util.concurrent.Atomics;
import com.google.gitiles.GitilesView.Type;
@@ -440,8 +441,43 @@
assertEquals("foo/bar", view.getPathPart());
}
+ public void testNormalizeParents() throws Exception {
+ RevCommit parent = repo.commit().create();
+ RevCommit master = repo.branch("refs/heads/master").commit().parent(parent).create();
+ GitilesView view;
+
+ assertEquals("/b/repo/+/master", getView("/repo/+/master").toUrl());
+ assertEquals("/b/repo/+/" + master.name(), getView("/repo/+/" + master.name()).toUrl());
+ assertEquals("/b/repo/+/" + parent.name(), getRedirectUrl("/repo/+/master~"));
+ assertEquals("/b/repo/+/" + parent.name(), getRedirectUrl("/repo/+/master^"));
+
+ view = getView("/repo/+log/master~..master/");
+ assertEquals("master", view.getRevision().getName());
+ assertEquals("master~", view.getOldRevision().getName());
+
+ view = getView("/repo/+log/master^!/");
+ assertEquals("master", view.getRevision().getName());
+ assertEquals("master^", view.getOldRevision().getName());
+ }
+
+ private String getRedirectUrl(String pathAndQuery) throws ServletException, IOException {
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+ service(pathAndQuery, Atomics.<GitilesView> newReference(), res);
+ assertEquals(302, res.getStatus());
+ return res.getHeader(HttpHeaders.LOCATION);
+ }
+
private GitilesView getView(String pathAndQuery) throws ServletException, IOException {
- final AtomicReference<GitilesView> view = Atomics.newReference();
+ AtomicReference<GitilesView> view = Atomics.newReference();
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+ service(pathAndQuery, view, res);
+ assertTrue("expected non-redirect status, got " + res.getStatus(),
+ res.getStatus() < 300 || res.getStatus() >= 400);
+ return view.get();
+ }
+
+ private void service(String pathAndQuery, final AtomicReference<GitilesView> view,
+ FakeHttpServletResponse res) throws ServletException, IOException {
HttpServlet testServlet = new HttpServlet() {
private static final long serialVersionUID = 1L;
@Override
@@ -470,9 +506,7 @@
} else {
req.setPathInfo(pathAndQuery);
}
- dummyServlet(mf).service(req, new FakeHttpServletResponse());
-
- return view.get();
+ dummyServlet(mf).service(req, res);
}
private MetaServlet dummyServlet(MetaFilter mf) {
