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() {