Merge changes Icf4d0259,I488046e9 * changes: Fix relative hyperlinks in Markdown Fix checkView error for missing servletPath
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 0a76b7d..aae388c 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -387,7 +387,7 @@ private void checkHostIndex() { checkView(hostName != null, "missing hostName on %s view", type); - checkView(servletPath != null, "missing hostName on %s view", type); + checkView(servletPath != null, "missing servletPath on %s view", type); } private void checkRepositoryIndex() {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java index 7d0d4e1..1b5df78 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
@@ -17,6 +17,8 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.gitiles.doc.MarkdownUtil.getInnerText; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.CharMatcher; import com.google.common.base.Strings; import com.google.gitiles.GitilesView; import com.google.gitiles.ThreadSafePrettifyParser; @@ -361,49 +363,52 @@ } } - private String href(String url) { - if (HtmlBuilder.isValidHttpUri(url)) { - return url; - } else if (MarkdownUtil.isAbsolutePathToMarkdown(url)) { - return GitilesView.doc().copyFrom(view).setPathPart(url).build().toUrl(); - } else if (url.startsWith("/")) { - return GitilesView.show().copyFrom(view).setPathPart(url).build().toUrl(); - } else if (!url.startsWith("../") && !url.startsWith("./")) { - return url; + @VisibleForTesting + String href(String target) { + if (HtmlBuilder.isValidHttpUri(target)) { + return target; } - String dir = Strings.nullToEmpty(view.getPathPart()); - if (!readme) { - int slash = dir.lastIndexOf('/'); - dir = slash < 0 ? "" : dir.substring(0, slash); + if (target.startsWith("/")) { + return toPath(target); } - while (!url.isEmpty()) { - if (url.startsWith("./")) { - url = url.substring(2); - } else if (url.startsWith("../")) { + + String viewPath = Strings.nullToEmpty(view.getPathPart()); + String dir; + if (readme) { + dir = CharMatcher.is('/').trimTrailingFrom(viewPath); + } else { + // When readme is false this instance is rendering a file, whose name + // appears as the last component of viewPath. Other links are relative + // to the file's container directory, so trim the file. + dir = trimLastComponent(viewPath); + } + + while (!target.isEmpty()) { + if (target.startsWith("../") || target.equals("..")) { if (dir.isEmpty()) { return FilterNormalizeUri.INSTANCE.getInnocuousOutput(); } - url = url.substring(3); - - int slash = dir.lastIndexOf('/'); - if (slash >= 0) { - dir = dir.substring(0, slash); - } else { - dir = ""; - break; - } + dir = trimLastComponent(dir); + target = target.equals("..") ? "" : target.substring(3); + } else if (target.startsWith("./")) { + target = target.substring(2); + } else if (target.equals(".")) { + target = ""; } else { break; } } + return toPath(dir + '/' + target); + } - if (url.isEmpty()) { - return FilterNormalizeUri.INSTANCE.getInnocuousOutput(); - } + private static String trimLastComponent(String path) { + int slash = path.lastIndexOf('/'); + return slash < 0 ? "" : path.substring(0, slash); + } - GitilesView.Builder dest = url.endsWith(".md") ? GitilesView.doc() : GitilesView.show(); - return dest.copyFrom(view).setPathPart(dir + url).build().toUrl(); + private String toPath(String path) { + return GitilesView.path().copyFrom(view).setPathPart(path).build().toUrl(); } @Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java index ad0f4e2..f73c331 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java
@@ -52,8 +52,8 @@ assertThat(html).contains("<title>Site Title - page</title>"); assertThat(html).contains("<span class=\"Header-anchorTitle\">Site Title</span>"); - assertThat(html).contains("<li><a href=\"index.md\">Home</a></li>"); - assertThat(html).contains("<li><a href=\"README.md\">README</a></li>"); + assertThat(html).contains("<li><a href=\"/b/repo/+/master/index.md\">Home</a></li>"); + assertThat(html).contains("<li><a href=\"/b/repo/+/master/README.md\">README</a></li>"); assertThat(html) .contains( "<h1>" + "<a class=\"h\" name=\"page\" href=\"#page\"><span></span></a>" + "page</h1>"); @@ -108,7 +108,7 @@ repo.branch("master").commit().add("A/B/README.md", "[c](../../C)").create(); String html = buildHtml("/repo/+doc/master/A/B/README.md"); - assertThat(html).contains("<a href=\"/b/repo/+show/master/C\">c</a>"); + assertThat(html).contains("<a href=\"/b/repo/+/master/C\">c</a>"); } @Test @@ -116,6 +116,6 @@ repo.branch("master").commit().add("README.md", "[c](/x)").create(); String html = buildHtml("/repo/+doc/master/README.md"); - assertThat(html).contains("<a href=\"/b/repo/+show/master/x\">c</a>"); + assertThat(html).contains("<a href=\"/b/repo/+/master/x\">c</a>"); } }
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java new file mode 100644 index 0000000..7b3eced --- /dev/null +++ b/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java
@@ -0,0 +1,146 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// 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.doc; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gitiles.GitilesView; + +import org.eclipse.jgit.lib.Config; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class LinkTest { + private GitilesView view; + private Config config; + + @Before + public void setup() { + view = GitilesView.revision() + .setHostName("127.0.0.1") + .setServletPath("/g") + .setRepositoryName("repo") + .setRevision("HEAD") + .build(); + config = new Config(); + } + + @Test + public void httpLink() { + MarkdownToHtml md = new MarkdownToHtml(view, config); + String url; + + url = "http://example.com/foo.html"; + assertThat(md.href(url)).isEqualTo(url); + + url = "https://example.com/foo.html"; + assertThat(md.href(url)).isEqualTo(url); + + url = "//example.com/foo.html"; + assertThat(md.href(url)).isEqualTo(url); + } + + @Test + public void absolutePath() { + MarkdownToHtml md = new MarkdownToHtml(view, config); + + assertThat(md.href("/")).isEqualTo("/g/repo/+/HEAD/"); + assertThat(md.href("/index.md")).isEqualTo("/g/repo/+/HEAD/index.md"); + assertThat(md.href("/doc/index.md")).isEqualTo("/g/repo/+/HEAD/doc/index.md"); + + // GitilesView trims trailing '/' from path expressions. + assertThat(md.href("/doc/")).isEqualTo("/g/repo/+/HEAD/doc"); + } + + @Test + public void relativePathInRootFile() { + testMarkdownInRoot(file("/index.md")); + } + + @Test + public void relativePathInTreeFile() { + testMarkdownInTree(file("/doc/index.md")); + } + + @Test + public void relativePathInRepositoryIndexReadme() { + testMarkdownInRoot(repoIndexReadme()); + } + + @Test + public void relativePathInCommitReadme() { + testMarkdownInRoot(revisionReadme()); + } + + @Test + public void relativePathInTreeReadme() { + testMarkdownInTree(treeReadme("/doc")); + testMarkdownInTree(treeReadme("/doc/")); + } + + private static void testMarkdownInRoot(MarkdownToHtml md) { + assertThat(md.href("setup.md")).isEqualTo("/g/repo/+/HEAD/setup.md"); + assertThat(md.href("./setup.md")).isEqualTo("/g/repo/+/HEAD/setup.md"); + assertThat(md.href("./")).isEqualTo("/g/repo/+/HEAD/"); + assertThat(md.href(".")).isEqualTo("/g/repo/+/HEAD/"); + + assertThat(md.href("../")).isEqualTo("#zSoyz"); + assertThat(md.href("../../")).isEqualTo("#zSoyz"); + assertThat(md.href("../..")).isEqualTo("#zSoyz"); + assertThat(md.href("..")).isEqualTo("#zSoyz"); + } + + private static void testMarkdownInTree(MarkdownToHtml md) { + assertThat(md.href("setup.md")).isEqualTo("/g/repo/+/HEAD/doc/setup.md"); + assertThat(md.href("./setup.md")).isEqualTo("/g/repo/+/HEAD/doc/setup.md"); + assertThat(md.href("../setup.md")).isEqualTo("/g/repo/+/HEAD/setup.md"); + assertThat(md.href("../tech/setup.md")).isEqualTo("/g/repo/+/HEAD/tech/setup.md"); + + assertThat(md.href("./")).isEqualTo("/g/repo/+/HEAD/doc"); + assertThat(md.href(".")).isEqualTo("/g/repo/+/HEAD/doc"); + assertThat(md.href("../")).isEqualTo("/g/repo/+/HEAD/"); + assertThat(md.href("..")).isEqualTo("/g/repo/+/HEAD/"); + + assertThat(md.href("../../")).isEqualTo("#zSoyz"); + assertThat(md.href("../..")).isEqualTo("#zSoyz"); + assertThat(md.href("../../../")).isEqualTo("#zSoyz"); + assertThat(md.href("../../..")).isEqualTo("#zSoyz"); + } + + private MarkdownToHtml file(String path) { + return new MarkdownToHtml( + GitilesView.doc().copyFrom(view).setPathPart(path).build(), + config); + } + + private MarkdownToHtml repoIndexReadme() { + return readme(view); + } + + private MarkdownToHtml revisionReadme() { + return readme(GitilesView.revision().copyFrom(view).build()); + } + + private MarkdownToHtml treeReadme(String path) { + return readme(GitilesView.path().copyFrom(view).setPathPart(path).build()); + } + + private MarkdownToHtml readme(GitilesView v) { + return new MarkdownToHtml(v, config).setReadme(true); + } +}