commit 5eb9439f9db0da0ce6fbf1c127dfc32e0b35175d
author Sven Selberg <svense@axis.com> Thu Aug 06 13:51:50 2020 +0200
committer Sven Selberg <svense@axis.com> Mon Aug 24 15:36:37 2020 +0200
tree 3f6b20ecd64ef1baa76fea383b0563648f6b9283
parent c143362d38be2c7b792dda6aa6ad77ced3b4a5ae

Set correct target url for symlinks

When attemting to resolv targetUrl for a symlink dirname was calculated
incorrectly.
After removing all trailing slashes the index of the last '/' was
calculated in an effort to get the dirname part from what was before
this last slash.
When attempting to remove the last slash and everything after it the
index was unnecesarily decremented by one which lead to a faulty
targetUrl and a faulty link to the symlink target in the UI.

   path = "foo/bar"
   lastSlashIndex = 3
   dirPath = "foo/bar".substring(0, 3-1) => "fo"

So if symlink "./foo/bar" had target "./foo/bar.md" the link to target
would be "./fo/bar.md".

Gitiles Bug: https://github.com/google/gitiles/issues/144

Change-Id: I14d6631b2ba7879ca66c091b90daf18e8763f314

java/com/google/gitiles/PathServlet.java

diff --git a/java/com/google/gitiles/PathServlet.java b/java/com/google/gitiles/PathServlet.java
index df24062..26de757 100644
--- a/java/com/google/gitiles/PathServlet.java
+++ b/java/com/google/gitiles/PathServlet.java
@@ -578,7 +578,7 @@
     }
     int lastSlash = path.lastIndexOf('/');
     if (lastSlash > 0) {
-      return path.substring(0, lastSlash - 1);
+      return path.substring(0, lastSlash);
     } else if (lastSlash == 0) {
       return "/";
     } else {

javatests/com/google/gitiles/PathServletTest.java

diff --git a/javatests/com/google/gitiles/PathServletTest.java b/javatests/com/google/gitiles/PathServletTest.java
index 4311833..a32de9f 100644
--- a/javatests/com/google/gitiles/PathServletTest.java
+++ b/javatests/com/google/gitiles/PathServletTest.java
@@ -149,23 +149,12 @@
 
   @Test
   public void symlinkHtml() throws Exception {
-    final RevBlob link = repo.blob("foo");
-    repo.branch("master")
-        .commit()
-        .add("foo", "contents")
-        .edit(
-            new PathEdit("bar") {
-              @Override
-              public void apply(DirCacheEntry ent) {
-                ent.setFileMode(FileMode.SYMLINK);
-                ent.setObjectId(link);
-              }
-            })
-        .create();
+    testSymlink("foo", "bar", "foo");
+  }
 
-    Map<String, ?> data = buildData("/repo/+/master/bar");
-    assertThat(data).containsEntry("type", "SYMLINK");
-    assertThat(getBlobData(data)).containsEntry("target", "foo");
+  @Test
+  public void relativeSymlinkHtml() throws Exception {
+    testSymlink("foo/bar", "foo/baz", "./bar");
   }
 
   @Test
@@ -410,6 +399,28 @@
     assertThat(res.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isEqualTo(null);
   }
 
+  private void testSymlink(String linkTarget, String linkName, String linkContent)
+      throws Exception {
+    final RevBlob linkBlob = repo.blob(linkContent);
+    repo.branch("master")
+        .commit()
+        .add(linkTarget, "contents")
+        .edit(
+            new PathEdit(linkName) {
+              @Override
+              public void apply(DirCacheEntry ent) {
+                ent.setFileMode(FileMode.SYMLINK);
+                ent.setObjectId(linkBlob);
+              }
+            })
+        .create();
+
+    Map<String, ?> data = buildData("/repo/+/master/" + linkName);
+    assertThat(data).containsEntry("type", "SYMLINK");
+    assertThat(getBlobData(data)).containsEntry("target", linkContent);
+    assertThat(getBlobData(data)).containsEntry("targetUrl", "/b/repo/+/master/" + linkTarget);
+  }
+
   private Map<String, ?> getBlobData(Map<String, ?> data) {
     return ((Map<String, Map<String, ?>>) data).get("data");
   }