Support branch redirect in Gitiles
- it's for helping users keep their old URLs working when migrating
from branch names like "studio-master-dev" to "studio-main-dev"
- to avoid confusing automation, it only affects interactive browsing,
not `?format=JSON` API calls
- it's customizable and we anticipate that Gerrit's gitiles plugin will
override the BranchRedirectFilter class as part of the same effort as
https://crbug.com/gerrit/13027
PiperOrigin-RevId: 348239020
Change-Id: I80729c8f315ca80f874e07d6919734cb07120625
diff --git a/java/com/google/gitiles/BranchRedirectFilter.java b/java/com/google/gitiles/BranchRedirectFilter.java
new file mode 100644
index 0000000..48ff35b
--- /dev/null
+++ b/java/com/google/gitiles/BranchRedirectFilter.java
@@ -0,0 +1,112 @@
+// Copyright 2021 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gitiles;
+
+import static com.google.common.net.HttpHeaders.LOCATION;
+import static com.google.gitiles.FormatType.HTML;
+import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY;
+import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
+
+import java.io.IOException;
+import java.util.Optional;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Filter to replace the URL string that contains a branch name to a new branch name. The updated
+ * branch mapping is provided by {@code BranchRedirectFilter#getRedirectBranch} method If it updates
+ * the branch it then returns the new URL with updated branch name as redirect.
+ *
+ * <p>This implementation does not provide a branch redirect mapping. Hence including this filter
+ * as-is would be a no-op. To make this effective {@code BranchRedirectFilter#getRedirectBranch}
+ * needs to be overridden that provides a mapping to the requested repo/branch.
+ */
+public class BranchRedirectFilter extends AbstractHttpFilter {
+ /**
+ * Provides an extendable interface that can be used to provide implementation for determining
+ * redirect branch
+ *
+ * @param repo Repository
+ * @param sourceBranch full branch name eg. refs/heads/master
+ * @return Returns the branch that should be redirected to on a given repo. {@code
+ * Optional.empty()} means no redirect.
+ */
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ return Optional.empty();
+ }
+
+ private Optional<String> rewriteRevision(Repository repo, Revision rev) {
+ if (Revision.isNull(rev)) {
+ return Optional.empty();
+ }
+ return getRedirectBranch(repo, rev.getName());
+ }
+
+ private static Revision rewriteRevision(Revision revision, Optional<String> targetBranch) {
+ if (!targetBranch.isPresent()) {
+ return revision;
+ }
+
+ return new Revision(
+ targetBranch.get(),
+ revision.getId(),
+ revision.getType(),
+ revision.getPeeledId(),
+ revision.getPeeledType());
+ }
+
+ @Override
+ public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
+ throws IOException, ServletException {
+ if (!hasRepository(req) || isForAutomation(req)) {
+ chain.doFilter(req, res);
+ return;
+ }
+
+ GitilesView view = ViewFilter.getView(req);
+ Repository repo = ServletUtils.getRepository(req);
+
+ Optional<String> rewrittenRevision = rewriteRevision(repo, view.getRevision());
+ Optional<String> rewrittenOldRevision = rewriteRevision(repo, view.getOldRevision());
+
+ if (!rewrittenRevision.isPresent() && !rewrittenOldRevision.isPresent()) {
+ chain.doFilter(req, res);
+ return;
+ }
+
+ Revision rev = rewriteRevision(view.getRevision(), rewrittenRevision);
+ Revision oldRev = rewriteRevision(view.getOldRevision(), rewrittenOldRevision);
+ if (rev.equals(view.getRevision()) && oldRev.equals(view.getOldRevision())) {
+ chain.doFilter(req, res);
+ return;
+ }
+
+ String url = view.toBuilder().setRevision(rev).setOldRevision(oldRev).toUrl();
+ res.setStatus(SC_MOVED_PERMANENTLY);
+ res.setHeader(LOCATION, url);
+ }
+
+ private static boolean hasRepository(HttpServletRequest req) {
+ return req.getAttribute(ATTRIBUTE_REPOSITORY) != null;
+ }
+
+ private static boolean isForAutomation(HttpServletRequest req) {
+ return !FormatType.getFormatType(req).orElse(HTML).equals(HTML);
+ }
+}
diff --git a/java/com/google/gitiles/GitilesFilter.java b/java/com/google/gitiles/GitilesFilter.java
index 3013096..8876007 100644
--- a/java/com/google/gitiles/GitilesFilter.java
+++ b/java/com/google/gitiles/GitilesFilter.java
@@ -1,4 +1,4 @@
-// Copyright 2012 Google Inc. All Rights Reserved.
+// Copyright 2021 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -176,6 +176,7 @@
private BlameCache blameCache;
private GitwebRedirectFilter gitwebRedirect;
private Filter errorHandler;
+ private BranchRedirectFilter branchRedirect;
private boolean initialized;
GitilesFilter() {}
@@ -190,6 +191,7 @@
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
@Nullable GitwebRedirectFilter gitwebRedirect,
+ @Nullable BranchRedirectFilter branchRedirect,
@Nullable Filter errorHandler) {
this.config = checkNotNull(config, "config");
this.renderer = renderer;
@@ -203,6 +205,7 @@
this.resolver = resolver;
}
this.errorHandler = errorHandler;
+ this.branchRedirect = branchRedirect;
}
@Override
@@ -224,13 +227,21 @@
if (gitwebRedirect != null) {
root.through(gitwebRedirect);
}
+ if (branchRedirect != null) {
+ root.through(branchRedirect);
+ }
root.through(dispatchFilter);
- serveRegex(REPO_REGEX).through(repositoryFilter).through(viewFilter).through(dispatchFilter);
+ serveRegex(REPO_REGEX)
+ .through(repositoryFilter)
+ .through(viewFilter)
+ .through(branchRedirect)
+ .through(dispatchFilter);
serveRegex(REPO_PATH_REGEX)
.through(repositoryFilter)
.through(viewFilter)
+ .through(branchRedirect)
.through(dispatchFilter);
initialized = true;
diff --git a/java/com/google/gitiles/GitilesServlet.java b/java/com/google/gitiles/GitilesServlet.java
index 6d10c0c..df2c3da 100644
--- a/java/com/google/gitiles/GitilesServlet.java
+++ b/java/com/google/gitiles/GitilesServlet.java
@@ -51,7 +51,8 @@
@Nullable VisibilityCache visibilityCache,
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
- @Nullable GitwebRedirectFilter gitwebRedirect) {
+ @Nullable GitwebRedirectFilter gitwebRedirect,
+ @Nullable BranchRedirectFilter branchRedirect) {
this(
config,
renderer,
@@ -62,6 +63,7 @@
timeCache,
blameCache,
gitwebRedirect,
+ branchRedirect,
null);
}
@@ -75,6 +77,7 @@
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
@Nullable GitwebRedirectFilter gitwebRedirect,
+ @Nullable BranchRedirectFilter branchRedirect,
@Nullable Filter errorHandler) {
super(
new GitilesFilter(
@@ -87,6 +90,7 @@
timeCache,
blameCache,
gitwebRedirect,
+ branchRedirect,
errorHandler));
}
diff --git a/javatests/com/google/gitiles/BranchRedirectFilterTest.java b/javatests/com/google/gitiles/BranchRedirectFilterTest.java
new file mode 100644
index 0000000..c994038
--- /dev/null
+++ b/javatests/com/google/gitiles/BranchRedirectFilterTest.java
@@ -0,0 +1,245 @@
+// Copyright 2021 Google LLC. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gitiles;
+
+import static com.google.common.truth.Truth.assertThat;
+import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY;
+import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+
+import com.google.common.net.HttpHeaders;
+import java.util.Optional;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepository;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for BranchRedirect. */
+@RunWith(JUnit4.class)
+public class BranchRedirectFilterTest {
+ private static final String MASTER = "refs/heads/master";
+ private static final String MAIN = "refs/heads/main";
+ private static final String DEVELOP = "refs/heads/develop";
+ private static final String FOO = "refs/heads/foo";
+ private static final String BAR = "refs/heads/bar";
+ private static final String ORIGIN = "http://localhost";
+ private static final String QUERY_STRING_HTML = "format=html";
+ private static final String QUERY_STRING_JSON = "format=json";
+
+ private TestRepository<DfsRepository> repo;
+ private GitilesServlet servlet;
+
+ @Before
+ public void setUp() throws Exception {
+ repo = new TestRepository<>(new InMemoryRepository(new DfsRepositoryDescription("repo")));
+ BranchRedirectFilter branchRedirectFilter =
+ new BranchRedirectFilter() {
+ @Override
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ if (MASTER.equals(sourceBranch)) {
+ return Optional.of(MAIN);
+ }
+ if (FOO.equals(sourceBranch)) {
+ return Optional.of(BAR);
+ }
+ return Optional.empty();
+ }
+ };
+ servlet = TestGitilesServlet.create(repo, new GitwebRedirectFilter(), branchRedirectFilter);
+ }
+
+ @Test
+ public void show_withoutRedirect() throws Exception {
+ repo.branch("develop").commit().add("foo", "contents").create();
+
+ String path = "/repo/+/refs/heads/develop/foo";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ }
+
+ @Test
+ public void show_withRedirect() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+
+ String path = "/repo/+/refs/heads/master/foo";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main/foo?format=html");
+ }
+
+ @Test
+ public void show_onAutomationRequest() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+
+ String path = "/repo/+/refs/heads/master/foo";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_JSON);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ }
+
+ @Test
+ public void showParent_withRedirect() throws Exception {
+ RevCommit parent = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent).create();
+
+ String path = "/repo/+/refs/heads/master^";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ // It is resolved to the object id by ViewFilter.
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_TEMPORARILY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/" + parent.toObjectId().name() + "?format=html");
+ }
+
+ @Test
+ public void diff_withRedirect_onSingleBranch() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(DEVELOP).commit().add("foo", "contents").create();
+
+ String path = "/repo/+/refs/heads/master..refs/heads/develop";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main..refs/heads/develop/?format=html");
+ }
+
+ @Test
+ // @Ignore
+ public void diff_withRedirect_onBothBranch() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(FOO).commit().add("foo", "contents").create();
+
+ String path = "/repo/+/refs/heads/foo..refs/heads/master";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/bar..refs/heads/main/?format=html");
+ }
+
+ @Test
+ public void diff_withRedirect() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+
+ String path = "/repo/+diff/refs/heads/master^!";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main%5E%21/?format=html");
+ }
+
+ @Test
+ public void log_withRedirect() throws Exception {
+ repo.branch(MASTER).commit().add("foo", "contents").create();
+
+ String path = "/repo/+log/refs/heads/master";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+log/refs/heads/main/?format=html");
+ }
+
+ @Test
+ @Ignore
+ public void diff_withGrandParent_redirect() throws Exception {
+ RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit parent2 =
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+
+ String path = "/repo/+diff/refs/heads/master^^..refs/heads/master";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main%5E%5E..refs/heads/main/?format=html");
+ }
+
+ @Test
+ @Ignore
+ public void diff_withRelativeParent_redirect() throws Exception {
+ RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit parent2 =
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+
+ String path = "/repo/+diff/refs/heads/master~1..refs/heads/master";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main%5E%21/?format=html");
+ }
+
+ @Test
+ @Ignore
+ public void diff_withRelativeGrandParent_redirect() throws Exception {
+ RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit parent2 =
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
+ repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+
+ String path = "/repo/+diff/refs/heads/master~2..refs/heads/master";
+ FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
+ FakeHttpServletResponse res = new FakeHttpServletResponse();
+
+ servlet.service(req, res);
+ assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
+ assertThat(res.getHeader(HttpHeaders.LOCATION))
+ .isEqualTo("/b/repo/+/refs/heads/main%7E2..refs/heads/main/?format=html");
+ }
+
+ private static FakeHttpServletRequest newHttpRequest(
+ String path, String origin, String queryString) {
+ FakeHttpServletRequest req = FakeHttpServletRequest.newRequest();
+ req.setHeader(HttpHeaders.ORIGIN, origin);
+ req.setPathInfo(path);
+ req.setQueryString(queryString);
+ return req;
+ }
+}
diff --git a/javatests/com/google/gitiles/TestGitilesServlet.java b/javatests/com/google/gitiles/TestGitilesServlet.java
index 0a7beb1..a35e76f 100644
--- a/javatests/com/google/gitiles/TestGitilesServlet.java
+++ b/javatests/com/google/gitiles/TestGitilesServlet.java
@@ -31,10 +31,17 @@
/** Static utility methods for creating {@link GitilesServlet}s for testing. */
public class TestGitilesServlet {
- /** @see #create(TestRepository) */
+ /** @see #create(TestRepository,GitwebRedirectFilter,BranchRedirectFilter) */
public static GitilesServlet create(final TestRepository<DfsRepository> repo)
throws ServletException {
- return create(repo, new GitwebRedirectFilter());
+ return create(repo, new GitwebRedirectFilter(), new BranchRedirectFilter());
+ }
+
+ /** @see #create(TestRepository,GitwebRedirectFilter,BranchRedirectFilter) */
+ public static GitilesServlet create(
+ final TestRepository<DfsRepository> repo, GitwebRedirectFilter gitwebRedirect)
+ throws ServletException {
+ return create(repo, gitwebRedirect, new BranchRedirectFilter());
}
/**
@@ -48,10 +55,13 @@
*
* @param repo the test repo backing the servlet.
* @param gitwebRedirect optional redirect filter for gitweb URLs.
+ * @param branchRedirect branch redirect filter
* @return a servlet.
*/
public static GitilesServlet create(
- final TestRepository<DfsRepository> repo, GitwebRedirectFilter gitwebRedirect)
+ final TestRepository<DfsRepository> repo,
+ GitwebRedirectFilter gitwebRedirect,
+ BranchRedirectFilter branchRedirect)
throws ServletException {
final String repoName = repo.getRepository().getDescription().getRepositoryName();
GitilesServlet servlet =
@@ -74,7 +84,8 @@
null,
null,
null,
- gitwebRedirect);
+ gitwebRedirect,
+ branchRedirect);
servlet.init(
new ServletConfig() {
