Implement a redirector for gitweb-style URLs. Add a filter to the host index chain to redirect in the presence of gitweb-style query parameters. Only handles gitweb URLs using the query parameter flavor; see gitweb(1) for details. The intent of this filter is to provide compatibility for preexisting links to old gitweb URLs to be migrated to Gitiles; of course Gitiles itself does not create such URLs. For this reason, use HTTP 301 (Moved Permanently) for the redirect rather than 302 Found. Include simple but not exhaustive tests; the code block in question is far more concise than exhaustive tests would be. Change-Id: Ief7022b7cd419fc0380992071ad6e4a4428a6169
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java index 04d9ef9..7b70400 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletRequest.java
@@ -318,7 +318,7 @@ @Override public String getQueryString() { - return null; + return GitilesView.paramsToString(parameters); } @Override
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 5249e41..a6c90e3 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/FakeHttpServletResponse.java
@@ -16,9 +16,11 @@ import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Charsets; +import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.net.HttpHeaders; @@ -252,4 +254,8 @@ public String getActualBodyString() { return RawParseUtils.decode(getActualBody()); } + + public String getHeader(String name) { + return Iterables.getFirst(headers.get(checkNotNull(name)), null); + } }
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/GitwebRedirectFilterTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/GitwebRedirectFilterTest.java new file mode 100644 index 0000000..3f2645a --- /dev/null +++ b/gitiles-servlet/src/test/java/com/google/gitiles/GitwebRedirectFilterTest.java
@@ -0,0 +1,136 @@ +// Copyright 2012 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. +// 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.gitiles.FakeHttpServletRequest.SERVLET_PATH; +import static com.google.gitiles.TestGitilesUrls.HOST_NAME; + +import static javax.servlet.http.HttpServletResponse.SC_GONE; +import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY; + +import com.google.common.net.HttpHeaders; + +import junit.framework.TestCase; + +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.dfs.DfsRepository; +import org.eclipse.jgit.storage.dfs.DfsRepositoryDescription; +import org.eclipse.jgit.storage.dfs.InMemoryRepository; + +import javax.servlet.http.HttpServletRequest; + +/** Tests for gitweb redirector. */ +public class GitwebRedirectFilterTest extends TestCase { + private TestRepository<DfsRepository> repo; + private GitilesServlet servlet; + + @Override + protected void setUp() throws Exception { + repo = new TestRepository<DfsRepository>( + new InMemoryRepository(new DfsRepositoryDescription("test"))); + servlet = TestGitilesServlet.create(repo); + } + + private void assertRedirectsTo(String expectedLocation, HttpServletRequest req) throws Exception { + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + assertEquals(SC_MOVED_PERMANENTLY, res.getStatus()); + assertEquals(expectedLocation, res.getHeader(HttpHeaders.LOCATION)); + } + + private void assertGone(HttpServletRequest req) throws Exception { + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + assertEquals(SC_GONE, res.getStatus()); + } + + private static FakeHttpServletRequest newRequest(String qs) { + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo("/"); + req.setQueryString(qs); + return req; + } + + public void testHostIndex() throws Exception { + assertRedirectsTo( + GitilesView.hostIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .toUrl(), + newRequest("a=project_index")); + } + + public void testRepositoryIndex() throws Exception { + assertGone(newRequest("a=summary")); + assertRedirectsTo( + GitilesView.repositoryIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test") + .toUrl(), + newRequest("a=summary;p=test")); + } + + public void testShow() throws Exception { + assertGone(newRequest("a=commit")); + assertGone(newRequest("a=commit;p=test")); + RevCommit commit = repo.branch("refs/heads/master").commit().create(); + assertRedirectsTo( + GitilesView.revision() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test") + .setRevision(commit) + .toUrl(), + newRequest("a=commit;p=test&h=" + ObjectId.toString(commit))); + } + + public void testNoStripDotGit() throws Exception { + assertRedirectsTo( + GitilesView.repositoryIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test.git") + .toUrl(), + newRequest("a=summary;p=test.git")); + assertRedirectsTo( + GitilesView.repositoryIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test") + .toUrl(), + newRequest("a=summary;p=test")); + } + + public void testStripDotGit() throws Exception { + servlet = TestGitilesServlet.create(repo, new GitwebRedirectFilter(true)); + assertRedirectsTo( + GitilesView.repositoryIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test") + .toUrl(), + newRequest("a=summary;p=test.git")); + assertRedirectsTo( + GitilesView.repositoryIndex() + .setHostName(HOST_NAME) + .setServletPath(SERVLET_PATH) + .setRepositoryName("test") + .toUrl(), + newRequest("a=summary;p=test")); + } +}
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesAccess.java b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesAccess.java index 0b31e24..b9607ae 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesAccess.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesAccess.java
@@ -16,6 +16,8 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.collect.ImmutableMap; + import org.eclipse.jgit.storage.dfs.DfsRepository; import java.util.Map; @@ -36,9 +38,13 @@ return new GitilesAccess() { @Override public Map<String, RepositoryDescription> listRepositories(Set<String> branches) { - // TODO(dborowitz): Implement this, using the DfsRepositoryDescriptions to - // get the repository names. - throw new UnsupportedOperationException(); + if (branches != null && !branches.isEmpty()) { + throw new UnsupportedOperationException("branches set not yet supported"); + } + RepositoryDescription desc = new RepositoryDescription(); + desc.name = repo.getDescription().getRepositoryName(); + desc.cloneUrl = TestGitilesUrls.URLS.getBaseGitUrl(req) + "/" + desc.name; + return ImmutableMap.of(desc.name, desc); } @Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesServlet.java b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesServlet.java index b3cb72b..ee7b6ac 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesServlet.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesServlet.java
@@ -33,6 +33,12 @@ /** Static utility methods for creating {@link GitilesServlet}s for testing. */ public class TestGitilesServlet { + /** @see #create(TestRepository) */ + public static GitilesServlet create(final TestRepository<DfsRepository> repo) + throws ServletException { + return create(repo, new GitwebRedirectFilter()); + } + /** * Create a servlet backed by a single test repository. * <p> @@ -43,10 +49,11 @@ * to the servlet's {@code service} method to test. * * @param repo the test repo backing the servlet. + * @param gitwebRedirect optional redirect filter for gitweb URLs. * @return a servlet. */ - public static GitilesServlet create(final TestRepository<DfsRepository> repo) - throws ServletException { + public static GitilesServlet create(final TestRepository<DfsRepository> repo, + GitwebRedirectFilter gitwebRedirect) throws ServletException { final String repoName = repo.getRepository().getDescription().getRepositoryName(); GitilesServlet servlet = @@ -62,7 +69,7 @@ } return repo.getRepository(); } - }, null, null); + }, null, null, gitwebRedirect); servlet.init(new ServletConfig() { @Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesUrls.java b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesUrls.java index f8a0883..14ccef7 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesUrls.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/TestGitilesUrls.java
@@ -19,20 +19,21 @@ /** {@link GitilesUrls} for testing. */ public class TestGitilesUrls implements GitilesUrls { public static final GitilesUrls URLS = new TestGitilesUrls(); + public static final String HOST_NAME = "test-host"; @Override public String getHostName(HttpServletRequest req) { - return "test-host"; + return HOST_NAME; } @Override public String getBaseGitUrl(HttpServletRequest req) { - return "git://test-host/foo"; + return "git://" + HOST_NAME + "/foo"; } @Override public String getBaseGerritUrl(HttpServletRequest req) { - return "http://test-host-review/foo/"; + return "http://" + HOST_NAME + "-review/foo/"; } private TestGitilesUrls() {