RefServlet: Use full refname in link
The servlet is checking whether the shortname of each reference expands
to some other refname so that it can make a short link that points to
the right target. For example, "refs/heads/foo" can be linked as
"test/+/foo" instead of "test/+/refs/heads/foo" when there is no other
expansion for "foo".
It is nice to have a short, comprehensible URL that the user can
manipulate. However, this check is expensive, specially when there are
many branches and tags.
Always use full reference names in the links. It is barely noticeable to
the user and speeds up page generation.
Note that the short links are still valid. They are just not shown in
this page.
Change-Id: I1b6a15b3f584e85c56c08e31a24cdddc3129d883
diff --git a/java/com/google/gitiles/RefServlet.java b/java/com/google/gitiles/RefServlet.java
index 8a199a6..2b16ef5 100644
--- a/java/com/google/gitiles/RefServlet.java
+++ b/java/com/google/gitiles/RefServlet.java
@@ -169,24 +169,20 @@
for (Ref ref : refs) {
String name = ref.getName().substring(prefix.length());
- Ref refForName = refdb.getRef(name);
- if (refForName != null) {
- boolean needPrefix = !ref.getName().equals(refForName.getName());
- Map<String, Object> value = Maps.newHashMapWithExpectedSize(3);
- value.put(
- "url",
- GitilesView.revision()
- .copyFrom(view)
- .setRevision(
- Revision.unpeeled(needPrefix ? ref.getName() : name, ref.getObjectId()))
- .toUrl());
- value.put("name", name);
- if (headLeaf != null) {
- value.put("isHead", headLeaf.equals(ref));
- }
- result.add(value);
+ Map<String, Object> value = Maps.newHashMapWithExpectedSize(3);
+ value.put(
+ "url",
+ GitilesView.revision()
+ .copyFrom(view)
+ .setRevision(Revision.unpeeled(ref.getName(), ref.getObjectId()))
+ .toUrl());
+ value.put("name", name);
+ if (headLeaf != null) {
+ value.put("isHead", headLeaf.equals(ref));
}
+ result.add(value);
}
+
return result;
}
diff --git a/javatests/com/google/gitiles/RefServletTest.java b/javatests/com/google/gitiles/RefServletTest.java
index b0c0929..947fbd7 100644
--- a/javatests/com/google/gitiles/RefServletTest.java
+++ b/javatests/com/google/gitiles/RefServletTest.java
@@ -242,9 +242,10 @@
repo.branch("refs/nope/quux").commit().create();
assertThat(buildBranchesSoyData())
- .containsExactly(ref("/b/test/+/bar", "bar"), ref("/b/test/+/foo", "foo"))
+ .containsExactly(
+ ref("/b/test/+/refs/heads/bar", "bar"), ref("/b/test/+/refs/heads/foo", "foo"))
.inOrder();
- assertThat(buildTagsSoyData()).containsExactly(ref("/b/test/+/baz", "baz")).inOrder();
+ assertThat(buildTagsSoyData()).containsExactly(ref("/b/test/+/refs/tags/baz", "baz")).inOrder();
}
@Test
@@ -254,14 +255,10 @@
repo.branch("refs/tags/foo").commit().create();
assertThat(buildBranchesSoyData())
- .containsExactly(ref("/b/test/+/bar", "bar"), ref("/b/test/+/refs/heads/foo", "foo"))
- .inOrder();
- assertThat(buildTagsSoyData())
.containsExactly(
- // refs/tags/ is searched before refs/heads/, so this does not
- // appear ambiguous.
- ref("/b/test/+/foo", "foo"))
+ ref("/b/test/+/refs/heads/bar", "bar"), ref("/b/test/+/refs/heads/foo", "foo"))
.inOrder();
+ assertThat(buildTagsSoyData()).containsExactly(ref("/b/test/+/refs/tags/foo", "foo")).inOrder();
}
@Test
@@ -283,7 +280,7 @@
assertThat(buildBranchesSoyData())
.containsExactly(
- ref("/b/test/+/foo", "foo"),
+ ref("/b/test/+/refs/heads/foo", "foo"),
ref("/b/test/+/refs/heads/refs/heads/foo", "refs/heads/foo"))
.inOrder();
}
