Reverse sort tags by time in the repository index page Define "time" as the tag or commit time for each object, and negative infinity for non-tags and non-commits. This is potentially expensive to compute, especially on large repositories, but can be cached forever. Default the cache size to 10240 objects but make it configurable. Change-Id: Icf1b6c23c23d165ebe5a971529c51c8057bd3ebe
diff --git a/gitiles-dev/src/main/java/com/google/gitiles/dev/DevServer.java b/gitiles-dev/src/main/java/com/google/gitiles/dev/DevServer.java index 8502aac..7885f11 100644 --- a/gitiles-dev/src/main/java/com/google/gitiles/dev/DevServer.java +++ b/gitiles-dev/src/main/java/com/google/gitiles/dev/DevServer.java
@@ -187,7 +187,7 @@ new File(sourceRoot, "gitiles-servlet/src/main/resources/com/google/gitiles/templates") .getPath(), Objects.firstNonNull(cfg.getString("gitiles", null, "siteTitle"), "Gitiles")), - null, null, null, null); + null, null, null, null, null); ServletContextHandler handler = new ServletContextHandler(); handler.setContextPath("");
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java index b926666..f463445 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java
@@ -160,6 +160,7 @@ private GitilesAccess.Factory accessFactory; private RepositoryResolver<HttpServletRequest> resolver; private VisibilityCache visibilityCache; + private TimeCache timeCache; private boolean initialized; GitilesFilter() { @@ -171,12 +172,14 @@ GitilesUrls urls, GitilesAccess.Factory accessFactory, final RepositoryResolver<HttpServletRequest> resolver, - VisibilityCache visibilityCache) { + VisibilityCache visibilityCache, + TimeCache timeCache) { this.config = checkNotNull(config, "config"); this.renderer = renderer; this.urls = urls; this.accessFactory = accessFactory; this.visibilityCache = visibilityCache; + this.timeCache = timeCache; if (resolver != null) { this.resolver = wrapResolver(resolver); } @@ -220,7 +223,7 @@ case HOST_INDEX: return new HostIndexServlet(renderer, urls, accessFactory); case REPOSITORY_INDEX: - return new RepositoryIndexServlet(renderer, accessFactory); + return new RepositoryIndexServlet(renderer, accessFactory, timeCache); case REVISION: return new RevisionServlet(renderer, linkifier()); case PATH: @@ -277,7 +280,7 @@ private void setDefaultFields(FilterConfig filterConfig) throws ServletException { if (renderer != null && urls != null && accessFactory != null && resolver != null - && visibilityCache != null) { + && visibilityCache != null && timeCache != null) { return; } Config config; @@ -345,6 +348,13 @@ visibilityCache = new VisibilityCache(false); } } + if (timeCache == null) { + if (config.getSubsections("cache").contains("tagTime")) { + timeCache = new TimeCache(ConfigUtil.getCacheBuilder(config, "tagTime")); + } else { + timeCache = new TimeCache(); + } + } } private static String getBaseGitUrl(Config config) throws ServletException {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesServlet.java index be03293..38fb295 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesServlet.java
@@ -50,8 +50,10 @@ @Nullable GitilesUrls urls, @Nullable GitilesAccess.Factory accessFactory, @Nullable RepositoryResolver<HttpServletRequest> resolver, - @Nullable VisibilityCache visibilityCache) { - super(new GitilesFilter(config, renderer, urls, accessFactory, resolver, visibilityCache)); + @Nullable VisibilityCache visibilityCache, + @Nullable TimeCache timeCache) { + super(new GitilesFilter( + config, renderer, urls, accessFactory, resolver, visibilityCache, timeCache)); } public GitilesServlet() {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java index 0c5f97f..6659718 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -17,15 +17,19 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; +import com.google.common.collect.Ordering; +import com.google.common.util.concurrent.UncheckedExecutionException; import org.eclipse.jgit.http.server.ServletUtils; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefComparator; import org.eclipse.jgit.lib.RefDatabase; +import org.eclipse.jgit.revwalk.RevWalk; import java.io.IOException; import java.util.Collection; @@ -38,11 +42,15 @@ /** Serves the index page for a repository, if accessed directly by a browser. */ public class RepositoryIndexServlet extends BaseServlet { private static final long serialVersionUID = 1L; - private final GitilesAccess.Factory accessFactory; - public RepositoryIndexServlet(Renderer renderer, GitilesAccess.Factory accessFactory) { + private final GitilesAccess.Factory accessFactory; + private final TimeCache timeCache; + + public RepositoryIndexServlet(Renderer renderer, GitilesAccess.Factory accessFactory, + TimeCache timeCache) { super(renderer); this.accessFactory = checkNotNull(accessFactory, "accessFactory"); + this.timeCache = checkNotNull(timeCache, "timeCache"); } @Override @@ -53,17 +61,23 @@ @VisibleForTesting Map<String, ?> buildData(HttpServletRequest req) throws IOException { RepositoryDescription desc = accessFactory.forRequest(req).getRepositoryDescription(); - return ImmutableMap.of( - "cloneUrl", desc.cloneUrl, + RevWalk walk = new RevWalk(ServletUtils.getRepository(req)); + List<Map<String, String>> tags; + try { + tags = getRefs(req, Constants.R_TAGS, tagComparator(walk)); + } finally { + walk.release(); + } + return ImmutableMap.of("cloneUrl", desc.cloneUrl, "description", Strings.nullToEmpty(desc.description), - "branches", getRefs(req, Constants.R_HEADS), - "tags", getRefs(req, Constants.R_TAGS)); + "branches", getRefs(req, Constants.R_HEADS, Ordering.from(RefComparator.INSTANCE)), + "tags", tags); } - private List<Map<String, String>> getRefs(HttpServletRequest req, String prefix) - throws IOException { + private List<Map<String, String>> getRefs(HttpServletRequest req, String prefix, + Ordering<Ref> ordering) throws IOException { RefDatabase refdb = ServletUtils.getRepository(req).getRefDatabase(); - Collection<Ref> refs = RefComparator.sort(refdb.getRefs(prefix).values()); + Collection<Ref> refs = ordering.sortedCopy(refdb.getRefs(prefix).values()); List<Map<String, String>> result = Lists.newArrayListWithCapacity(refs.size()); for (Ref ref : refs) { @@ -77,4 +91,17 @@ return result; } + + private Ordering<Ref> tagComparator(final RevWalk walk) { + return Ordering.natural().onResultOf(new Function<Ref, Long>() { + @Override + public Long apply(Ref ref) { + try { + return timeCache.getTime(walk, ref.getObjectId()); + } catch (IOException e) { + throw new UncheckedExecutionException(e); + } + } + }).reverse().compound(RefComparator.INSTANCE); + } }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/TimeCache.java b/gitiles-servlet/src/main/java/com/google/gitiles/TimeCache.java new file mode 100644 index 0000000..9e78479 --- /dev/null +++ b/gitiles-servlet/src/main/java/com/google/gitiles/TimeCache.java
@@ -0,0 +1,85 @@ +// 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 com.google.common.base.Throwables; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; + +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevObject; +import org.eclipse.jgit.revwalk.RevTag; +import org.eclipse.jgit.revwalk.RevWalk; + +import java.io.IOException; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; + +/** + * Cache of the time associated with Git objects. + * <p> + * Uses the time as stored in annotated tags if available, or else the commit + * time of the tagged commit. Non-commits are given {@link Long#MIN_VALUE}, + * rather than searching for occurrences in the entire repository. + */ +public class TimeCache { + public static CacheBuilder<Object, Object> newBuilder() { + return CacheBuilder.newBuilder().maximumSize(10 << 10); + } + + private final Cache<ObjectId, Long> cache; + + public TimeCache() { + this(newBuilder()); + } + + public TimeCache(CacheBuilder<Object, Object> builder) { + this.cache = builder.build(); + } + + public Cache<?, ?> getCache() { + return cache; + } + + Long getTime(final RevWalk walk, final ObjectId id) throws IOException { + try { + return cache.get(id, new Callable<Long>() { + @Override + public Long call() throws IOException { + RevObject o = walk.parseAny(id); + while (o instanceof RevTag) { + RevTag tag = (RevTag) o; + PersonIdent ident = tag.getTaggerIdent(); + if (ident != null) { + return ident.getWhen().getTime() / 1000; + } + o = tag.getObject(); + walk.parseHeaders(o); + } + if (o.getType() == Constants.OBJ_COMMIT) { + return Long.valueOf(((RevCommit) o).getCommitTime()); + } + return Long.MIN_VALUE; + } + }); + } catch (ExecutionException e) { + Throwables.propagateIfInstanceOf(e.getCause(), IOException.class); + throw new IOException(e); + } + } +}
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/RepositoryIndexServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/RepositoryIndexServletTest.java index 3071f70..7a93191 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/RepositoryIndexServletTest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/RepositoryIndexServletTest.java
@@ -42,7 +42,8 @@ new InMemoryRepository(new DfsRepositoryDescription("test"))); servlet = new RepositoryIndexServlet( new DefaultRenderer(), - new TestGitilesAccess(repo.getRepository())); + new TestGitilesAccess(repo.getRepository()), + new TimeCache()); } public void testEmpty() throws Exception {
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/TimeCacheTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/TimeCacheTest.java new file mode 100644 index 0000000..f8c397c --- /dev/null +++ b/gitiles-servlet/src/test/java/com/google/gitiles/TimeCacheTest.java
@@ -0,0 +1,142 @@ +// 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 junit.framework.TestCase; + +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.TagBuilder; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevTag; +import org.eclipse.jgit.revwalk.RevTree; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.storage.dfs.DfsRepository; +import org.eclipse.jgit.storage.dfs.DfsRepositoryDescription; +import org.eclipse.jgit.storage.dfs.InMemoryRepository; + +import java.io.IOException; + +/** Unit tests for {@link TimeCache}. */ +public class TimeCacheTest extends TestCase { + private TestRepository<DfsRepository> repo; + private RevWalk walk; + private TimeCache cache; + + /** + * Start time of {@link #repo}. + * <p> + * Note that commits auto-increment the repo's ticker, but tags do not. + */ + private long start; + + @Override + protected void setUp() throws Exception { + repo = new TestRepository<DfsRepository>( + new InMemoryRepository(new DfsRepositoryDescription("test"))); + walk = new RevWalk(repo.getRepository()); + cache = new TimeCache(); + start = repo.getClock().getTime() / 1000; + } + + private long getTime(ObjectId id) throws IOException { + return cache.getTime(walk, id); + } + + public void testCommitTime() throws Exception { + RevCommit root = repo.commit().create(); + RevCommit master = repo.commit().parent(root).create(); + assertEquals(start + 1, getTime(root)); + assertEquals(start + 2, getTime(master)); + } + + public void testTaggedCommitTime() throws Exception { + RevCommit commit = repo.commit().create(); + repo.tick(1); + RevTag tag = repo.tag("tag", commit); + assertEquals(start + 1, getTime(commit)); + assertEquals(start + 2, getTime(tag)); + } + + public void testTaggedTreeAndBlobTime() throws Exception { + RevBlob blob = repo.blob("contents"); + RevTree tree = repo.tree(repo.file("foo", blob)); + repo.tick(1); + RevTag blobTag = repo.tag("blob", blob); + repo.tick(1); + RevTag treeTag = repo.tag("tree", tree); + assertEquals(start + 1, getTime(blobTag)); + assertEquals(start + 2, getTime(treeTag)); + } + + public void testTaggedTagTime() throws Exception { + repo.tick(2); + RevTag tag = repo.tag("tag", repo.commit().create()); + repo.tick(-1); + RevTag tagTag = repo.tag("tagtag", tag); + assertEquals(start + 3, getTime(tag)); + assertEquals(start + 2, getTime(tagTag)); + } + + public void testTreeAndBlobTime() throws Exception { + RevBlob blob = repo.blob("contents"); + RevTree tree = repo.tree(repo.file("foo", blob)); + assertEquals(Long.MIN_VALUE, getTime(blob)); + assertEquals(Long.MIN_VALUE, getTime(tree)); + } + + public void testTagMissingTime() throws Exception { + RevCommit commit = repo.commit().create(); + TagBuilder builder = new TagBuilder(); + builder.setObjectId(commit); + builder.setTag("tag"); + builder.setMessage(""); + ObjectInserter ins = repo.getRepository().newObjectInserter(); + ObjectId id; + try { + id = ins.insert(builder); + ins.flush(); + } finally { + ins.release(); + } + assertEquals(start + 1, getTime(commit)); + assertEquals(start + 1, getTime(id)); + } + + public void testFirstTagMissingTime() throws Exception { + RevCommit commit = repo.commit().create(); + repo.tick(1); + RevTag tag = repo.tag("tag", commit); + repo.tick(1); + + TagBuilder builder = new TagBuilder(); + builder.setObjectId(tag); + builder.setTag("tagtag"); + builder.setMessage(""); + ObjectInserter ins = repo.getRepository().newObjectInserter(); + ObjectId tagTagId; + try { + tagTagId = ins.insert(builder); + ins.flush(); + } finally { + ins.release(); + } + assertEquals(start + 1, getTime(commit)); + assertEquals(start + 2, getTime(tag)); + assertEquals(start + 2, getTime(tagTagId)); + } +}