Merge "LogServlet: use RevSort.TOPO_KEEP_BRANCH_TOGETHER"
diff --git a/Documentation/developer-guide.md b/Documentation/developer-guide.md
index 398a2e7..1716e80 100644
--- a/Documentation/developer-guide.md
+++ b/Documentation/developer-guide.md
@@ -115,7 +115,10 @@
Gitiles artifacts are published to the [gerrit-maven
bucket](http://gerrit-maven.storage.googleapis.com/). To release a new version,
-you must have write access to this bucket.
+you must have write access to this bucket. See
+[Deploy Gerrit
+Artifacts](https://gerrit-review.googlesource.com/Documentation/dev-release-deploy-config.html)
+for PGP key setup and Google Cloud Storage access setup.
First, increment `GITILES_VERSION` in `version.bzl`. Technically, Gitiles uses
the
@@ -129,7 +132,8 @@
./tools/maven/mvn.sh deploy
```
-Tag the release with an annotated tag matching the version number.
+Tag the release with a signed, annotated tag matching the version number, for
+example "v0.4-1".
Once released, Maven projects can consume the new version as long as they point
at the proper repository URL. Similarly, Bazel projects using the `maven_jar`
diff --git a/Documentation/markdown.md b/Documentation/markdown.md
index e6b766c..4c76388 100644
--- a/Documentation/markdown.md
+++ b/Documentation/markdown.md
@@ -446,7 +446,7 @@
PNG (`*.png`), JPEG (`*.jpg` or `*.jpeg`), GIF (`*.gif`) and WebP (`*.webp`)
image formats are supported when referenced from the Git repository.
-Unsupported extensions or files larger than [image size](#Image-size)
+Unsupported extensions or files larger than [image size](config.md#Image-size)
limit (default 256K) will display a broken image.
*** note
diff --git a/java/com/google/gitiles/BranchRedirect.java b/java/com/google/gitiles/BranchRedirect.java
new file mode 100644
index 0000000..3a34f65
--- /dev/null
+++ b/java/com/google/gitiles/BranchRedirect.java
@@ -0,0 +1,62 @@
+// Copyright 2021 Google LLC. 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.FormatType.HTML;
+
+import java.util.Optional;
+import javax.servlet.http.HttpServletRequest;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Utility that provides information to replace the URL string that contains a branch name to a new
+ * branch name. The updated branch mapping is provided by {@code BranchRedirect#getRedirectBranch}
+ * method. If it should update the branch then it is the caller's responsibility to update the URL
+ * with updated branch name as redirect.
+ *
+ * <p>This implementation does not provide a branch redirect mapping. Hence, including this as-is
+ * would be a no-op. To make this effective {@code BranchRedirect#getRedirectBranch} needs to be
+ * overridden that provides a mapping to the requested repo/branch.
+ */
+public class BranchRedirect {
+
+ static final BranchRedirect EMPTY = new BranchRedirect();
+
+ /**
+ * Provides an extendable interface that can be used to provide implementation for determining
+ * redirect branch
+ *
+ * @param repo Repository
+ * @param sourceBranch full branch name eg. refs/heads/master
+ * @return Returns the branch that should be redirected to on a given repo. {@code
+ * Optional.empty()} means no redirect.
+ */
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ return Optional.empty();
+ }
+
+ static boolean isForAutomation(HttpServletRequest req) {
+ FormatType formatType = FormatType.getFormatType(req).orElse(HTML);
+ switch (formatType) {
+ case HTML:
+ case DEFAULT:
+ return false;
+ case JSON:
+ case TEXT:
+ default:
+ return true;
+ }
+ }
+}
diff --git a/java/com/google/gitiles/BranchRedirectFilter.java b/java/com/google/gitiles/BranchRedirectFilter.java
deleted file mode 100644
index 12dc2b4..0000000
--- a/java/com/google/gitiles/BranchRedirectFilter.java
+++ /dev/null
@@ -1,121 +0,0 @@
-// Copyright 2021 Google LLC. 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.common.net.HttpHeaders.LOCATION;
-import static com.google.gitiles.FormatType.HTML;
-import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY;
-import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
-
-import java.io.IOException;
-import java.util.Optional;
-import javax.servlet.FilterChain;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jgit.http.server.ServletUtils;
-import org.eclipse.jgit.lib.Repository;
-
-/**
- * Filter to replace the URL string that contains a branch name to a new branch name. The updated
- * branch mapping is provided by {@code BranchRedirectFilter#getRedirectBranch} method If it updates
- * the branch it then returns the new URL with updated branch name as redirect.
- *
- * <p>This implementation does not provide a branch redirect mapping. Hence including this filter
- * as-is would be a no-op. To make this effective {@code BranchRedirectFilter#getRedirectBranch}
- * needs to be overridden that provides a mapping to the requested repo/branch.
- */
-public class BranchRedirectFilter extends AbstractHttpFilter {
- /**
- * Provides an extendable interface that can be used to provide implementation for determining
- * redirect branch
- *
- * @param repo Repository
- * @param sourceBranch full branch name eg. refs/heads/master
- * @return Returns the branch that should be redirected to on a given repo. {@code
- * Optional.empty()} means no redirect.
- */
- protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
- return Optional.empty();
- }
-
- private Optional<String> rewriteRevision(Repository repo, Revision rev) {
- if (Revision.isNull(rev)) {
- return Optional.empty();
- }
- return getRedirectBranch(repo, rev.getName());
- }
-
- private static Revision rewriteRevision(Revision revision, Optional<String> targetBranch) {
- if (!targetBranch.isPresent()) {
- return revision;
- }
-
- return new Revision(
- targetBranch.get(),
- revision.getId(),
- revision.getType(),
- revision.getPeeledId(),
- revision.getPeeledType());
- }
-
- @Override
- public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
- throws IOException, ServletException {
- if (!hasRepository(req) || isForAutomation(req)) {
- chain.doFilter(req, res);
- return;
- }
-
- GitilesView view = ViewFilter.getView(req);
- Repository repo = ServletUtils.getRepository(req);
-
- Optional<String> rewrittenRevision = rewriteRevision(repo, view.getRevision());
- Optional<String> rewrittenOldRevision = rewriteRevision(repo, view.getOldRevision());
-
- if (!rewrittenRevision.isPresent() && !rewrittenOldRevision.isPresent()) {
- chain.doFilter(req, res);
- return;
- }
-
- Revision rev = rewriteRevision(view.getRevision(), rewrittenRevision);
- Revision oldRev = rewriteRevision(view.getOldRevision(), rewrittenOldRevision);
- if (rev.equals(view.getRevision()) && oldRev.equals(view.getOldRevision())) {
- chain.doFilter(req, res);
- return;
- }
-
- String url = view.toBuilder().setRevision(rev).setOldRevision(oldRev).toUrl();
- res.setStatus(SC_MOVED_PERMANENTLY);
- res.setHeader(LOCATION, url);
- }
-
- private static boolean hasRepository(HttpServletRequest req) {
- return req.getAttribute(ATTRIBUTE_REPOSITORY) != null;
- }
-
- private static boolean isForAutomation(HttpServletRequest req) {
- FormatType formatType = FormatType.getFormatType(req).orElse(HTML);
- switch (formatType) {
- case HTML:
- case DEFAULT:
- return false;
- case JSON:
- case TEXT:
- default:
- return true;
- }
- }
-}
diff --git a/java/com/google/gitiles/CommitData.java b/java/com/google/gitiles/CommitData.java
index 705240e..23f03e8 100644
--- a/java/com/google/gitiles/CommitData.java
+++ b/java/com/google/gitiles/CommitData.java
@@ -41,6 +41,7 @@
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
@@ -60,6 +61,7 @@
DIFF_TREE,
LOG_URL,
MESSAGE,
+ NOTES,
PARENTS,
PARENT_BLAME_URL,
SHA,
@@ -83,6 +85,7 @@
static class Builder {
private ArchiveFormat archiveFormat;
private Map<AnyObjectId, Set<Ref>> refsById;
+ private static final int MAX_NOTE_SIZE = 524288;
Builder setArchiveFormat(@Nullable ArchiveFormat archiveFormat) {
this.archiveFormat = archiveFormat;
@@ -145,6 +148,19 @@
if (fs.contains(Field.TAGS)) {
result.tags = getRefsById(repo, c, Constants.R_TAGS);
}
+ if (fs.contains(Field.NOTES)) {
+ Ref notesRef = repo.getRefDatabase().exactRef(Constants.R_NOTES_COMMITS);
+ if (notesRef != null) {
+ try {
+ byte[] data =
+ NoteMap.read(walk.getObjectReader(), walk.parseCommit(notesRef.getObjectId()))
+ .getCachedBytes(c, MAX_NOTE_SIZE);
+ result.notes = new String(data, "utf-8");
+ } catch (Exception e) {
+ result.notes = "";
+ }
+ }
+ }
if (fs.contains(Field.MESSAGE)) {
walk.parseBody(c);
result.message = c.getFullMessage();
@@ -167,7 +183,6 @@
if (fs.contains(Field.DIFF_TREE)) {
result.diffEntries = computeDiffEntries(repo, view, walk, c);
}
-
return result;
}
@@ -251,6 +266,7 @@
List<RevCommit> parents;
String shortMessage;
String message;
+ String notes;
List<Ref> branches;
List<Ref> tags;
diff --git a/java/com/google/gitiles/CommitJsonData.java b/java/com/google/gitiles/CommitJsonData.java
index 953c19c..5f94a70 100644
--- a/java/com/google/gitiles/CommitJsonData.java
+++ b/java/com/google/gitiles/CommitJsonData.java
@@ -32,7 +32,13 @@
public class CommitJsonData {
static final ImmutableSet<Field> DEFAULT_FIELDS =
Sets.immutableEnumSet(
- Field.SHA, Field.TREE, Field.PARENTS, Field.AUTHOR, Field.COMMITTER, Field.MESSAGE);
+ Field.SHA,
+ Field.TREE,
+ Field.PARENTS,
+ Field.AUTHOR,
+ Field.COMMITTER,
+ Field.MESSAGE,
+ Field.NOTES);
public static class Log {
public List<Commit> log;
@@ -53,6 +59,7 @@
Ident author;
Ident committer;
String message;
+ String notes;
List<Diff> treeDiff;
}
@@ -101,6 +108,9 @@
if (cd.message != null) {
result.message = cd.message;
}
+ if (cd.notes != null && !cd.notes.isEmpty()){
+ result.notes = cd.notes;
+ }
if (cd.diffEntries != null) {
result.treeDiff = toJsonData(cd.diffEntries);
}
diff --git a/java/com/google/gitiles/CommitSoyData.java b/java/com/google/gitiles/CommitSoyData.java
index c4087a6..ac81f80 100644
--- a/java/com/google/gitiles/CommitSoyData.java
+++ b/java/com/google/gitiles/CommitSoyData.java
@@ -53,6 +53,7 @@
Field.TREE_URL,
Field.PARENTS,
Field.MESSAGE,
+ Field.NOTES, // Optional Field
Field.LOG_URL,
Field.ARCHIVE_URL,
Field.ARCHIVE_TYPE);
@@ -137,11 +138,20 @@
if (cd.diffEntries != null) {
data.put("diffTree", toSoyData(view, cd.diffEntries));
}
- checkState(
- Sets.difference(fs, NESTED_FIELDS).size() == data.size(),
- "bad commit data fields: %s != %s",
- fs,
- data.keySet());
+
+ int diffSetSize = Sets.difference(fs, NESTED_FIELDS).size();
+
+ if (fs.contains(
+ Field.NOTES)) { // Since NOTES is optional this is required for checkState to pass
+ diffSetSize -= 1;
+ }
+
+ checkState(diffSetSize == data.size(), "bad commit data fields: %s != %s", fs, data.keySet());
+
+ if (cd.notes != null && !cd.notes.isEmpty()) {
+ data.put("notes", cd.notes);
+ }
+
return data;
}
diff --git a/java/com/google/gitiles/GitilesFilter.java b/java/com/google/gitiles/GitilesFilter.java
index b347e08..da6044b 100644
--- a/java/com/google/gitiles/GitilesFilter.java
+++ b/java/com/google/gitiles/GitilesFilter.java
@@ -176,7 +176,7 @@
private BlameCache blameCache;
private GitwebRedirectFilter gitwebRedirect;
private Filter errorHandler;
- private BranchRedirectFilter branchRedirect;
+ private BranchRedirect branchRedirect;
private boolean initialized;
GitilesFilter() {}
@@ -191,7 +191,7 @@
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
@Nullable GitwebRedirectFilter gitwebRedirect,
- BranchRedirectFilter branchRedirect,
+ BranchRedirect branchRedirect,
@Nullable Filter errorHandler) {
this.config = checkNotNull(config, "config");
this.renderer = renderer;
@@ -220,28 +220,23 @@
}
Filter repositoryFilter = new RepositoryFilter(resolver);
- Filter viewFilter = new ViewFilter(accessFactory, urls, visibilityCache);
+ Filter viewFilter = new ViewFilter(accessFactory, urls, visibilityCache, branchRedirect);
Filter dispatchFilter = new DispatchFilter(filters, servlets);
ServletBinder root = serveRegex(ROOT_REGEX).through(viewFilter);
if (gitwebRedirect != null) {
root.through(gitwebRedirect);
}
- if (branchRedirect != null) {
- root.through(branchRedirect);
- }
root.through(dispatchFilter);
serveRegex(REPO_REGEX)
.through(repositoryFilter)
.through(viewFilter)
- .through(branchRedirect)
.through(dispatchFilter);
serveRegex(REPO_PATH_REGEX)
.through(repositoryFilter)
.through(viewFilter)
- .through(branchRedirect)
.through(dispatchFilter);
initialized = true;
diff --git a/java/com/google/gitiles/GitilesServlet.java b/java/com/google/gitiles/GitilesServlet.java
index 4467cfa..ee66753 100644
--- a/java/com/google/gitiles/GitilesServlet.java
+++ b/java/com/google/gitiles/GitilesServlet.java
@@ -52,7 +52,7 @@
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
@Nullable GitwebRedirectFilter gitwebRedirect,
- BranchRedirectFilter branchRedirect) {
+ BranchRedirect branchRedirect) {
this(
config,
renderer,
@@ -77,7 +77,7 @@
@Nullable TimeCache timeCache,
@Nullable BlameCache blameCache,
@Nullable GitwebRedirectFilter gitwebRedirect,
- BranchRedirectFilter branchRedirect,
+ BranchRedirect branchRedirect,
@Nullable Filter errorHandler) {
super(
new GitilesFilter(
diff --git a/java/com/google/gitiles/RevisionParser.java b/java/com/google/gitiles/RevisionParser.java
index b736f7f..8196267 100644
--- a/java/com/google/gitiles/RevisionParser.java
+++ b/java/com/google/gitiles/RevisionParser.java
@@ -21,8 +21,12 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
import java.io.IOException;
import java.util.Objects;
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.eclipse.jgit.errors.AmbiguousObjectException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.RevisionSyntaxException;
@@ -37,6 +41,10 @@
class RevisionParser {
private static final Splitter OPERATOR_SPLITTER = Splitter.on(CharMatcher.anyOf("^~"));
+ // The ref name part of a revision expression ends at the first
+ // appearance of ^, ~, :, or @{ (see git-check-ref-format(1)).
+ private static final Pattern END_OF_REF = Pattern.compile("[\\^~:]|@\\{");
+
static class Result {
private final Revision revision;
private final Revision oldRevision;
@@ -96,21 +104,28 @@
private final Repository repo;
private final GitilesAccess access;
private final VisibilityCache cache;
+ private final BranchRedirect branchRedirect;
- RevisionParser(Repository repo, GitilesAccess access, VisibilityCache cache) {
+ RevisionParser(
+ Repository repo, GitilesAccess access, VisibilityCache cache, BranchRedirect branchRedirect) {
this.repo = checkNotNull(repo, "repo");
this.access = checkNotNull(access, "access");
this.cache = checkNotNull(cache, "cache");
+ this.branchRedirect = checkNotNull(branchRedirect, "branchRedirect");
}
Result parse(String path) throws IOException {
if (path.startsWith("/")) {
path = path.substring(1);
}
+ if (Strings.isNullOrEmpty(path)) {
+ return null;
+ }
try (RevWalk walk = new RevWalk(repo)) {
walk.setRetainBody(false);
Revision oldRevision = null;
+ Revision oldRevisionRedirected = null;
StringBuilder b = new StringBuilder();
boolean first = true;
@@ -130,14 +145,24 @@
} else if (dots > 0) {
b.append(part, 0, dots);
String oldName = b.toString();
- if (!isValidRevision(oldName)) {
+ String oldNameRedirect = getRedirectFor(oldName);
+
+ if (!isValidRevision(oldNameRedirect)) {
return null;
}
- RevObject old = resolve(oldName, walk);
+ RevObject old = resolve(oldNameRedirect, walk);
if (old == null) {
return null;
}
+ /*
+ * Retain oldRevision with the old name (non-redirected-path) since it is used in
+ * determining the Revision path (start index of the path from the name).
+ * For example: For a master -> main redirect,
+ * original path: /master/index.c is updated to /main/index.c
+ * To parse the ref/path to build Revision object we look at the original path.
+ */
oldRevision = Revision.peel(oldName, old, walk);
+ oldRevisionRedirected = Revision.peel(oldNameRedirect, old, walk);
part = part.substring(dots + 2);
b = new StringBuilder();
} else if (firstParent > 0) {
@@ -149,7 +174,9 @@
if (!isValidRevision(name)) {
return null;
}
- RevObject obj = resolve(name, walk);
+
+ String nameRedirected = getRedirectFor(name);
+ RevObject obj = resolve(nameRedirected, walk);
if (obj == null) {
return null;
}
@@ -162,13 +189,15 @@
}
RevCommit c = (RevCommit) obj;
if (c.getParentCount() > 0) {
- oldRevision = Revision.peeled(name + "^", c.getParent(0));
+ oldRevisionRedirected = Revision.peeled(nameRedirected + "^", c.getParent(0));
} else {
- oldRevision = Revision.NULL;
+ oldRevisionRedirected = Revision.NULL;
}
Result result =
new Result(
- Revision.peeled(name, c), oldRevision, path.substring(name.length() + 2));
+ Revision.peeled(nameRedirected, c),
+ oldRevisionRedirected,
+ path.substring(name.length() + 2));
return isVisible(walk, result) ? result : null;
}
}
@@ -178,7 +207,9 @@
if (!isValidRevision(name)) {
return null;
}
- RevObject obj = resolve(name, walk);
+ String nameRedirected = getRedirectFor(name);
+
+ RevObject obj = resolve(nameRedirected, walk);
if (obj != null) {
int pathStart;
if (oldRevision == null) {
@@ -188,7 +219,10 @@
pathStart = oldRevision.getName().length() + 2 + name.length();
}
Result result =
- new Result(Revision.peel(name, obj, walk), oldRevision, path.substring(pathStart));
+ new Result(
+ Revision.peel(nameRedirected, obj, walk),
+ oldRevisionRedirected,
+ path.substring(pathStart));
return isVisible(walk, result) ? result : null;
}
first = false;
@@ -233,4 +267,29 @@
}
return true;
}
+
+ /**
+ * It replaces the ref in the revision expression to the redirected refName, without changing the
+ * behavior of the expression.
+ *
+ * <p>For eg: branch redirect {master -> main} would yield {master -> main}, {refs/heads/master^
+ * -> refs/heads/main^}, {refs/heads/master^ -> refs/heads/main^}. It does expand to a full
+ * refName even for shorter refNames.
+ */
+ private String getRedirectFor(String revisionExpression) {
+ String refName = refPart(revisionExpression);
+ Optional<String> redirect = branchRedirect.getRedirectBranch(repo, refName);
+ if (redirect.isPresent()) {
+ return redirect.get() + revisionExpression.substring(refName.length());
+ }
+ return revisionExpression;
+ }
+
+ private static String refPart(String revisionExpression) {
+ Matcher m = END_OF_REF.matcher(revisionExpression);
+ if (!m.find()) { // no terminator -> the whole string is a ref name.
+ return revisionExpression;
+ }
+ return revisionExpression.substring(0, m.start());
+ }
}
diff --git a/java/com/google/gitiles/TreeSoyData.java b/java/com/google/gitiles/TreeSoyData.java
index 1e4e4b7..e1aeb21 100644
--- a/java/com/google/gitiles/TreeSoyData.java
+++ b/java/com/google/gitiles/TreeSoyData.java
@@ -41,6 +41,14 @@
*/
private static final int MAX_SYMLINK_TARGET_LENGTH = 72;
+ private static final Map<String, Integer> TYPE_WEIGHT =
+ Map.of(
+ "TREE", 0,
+ "GITLINK", 1,
+ "SYMLINK", 2,
+ "REGULAR_FILE", 3,
+ "EXECUTABLE_FILE", 3);
+
/**
* Maximum number of bytes to load from a blob that claims to be a symlink. If the blob is larger
* than this byte limit it will be displayed as a binary file instead of as a symlink.
@@ -65,6 +73,10 @@
return lastSlash >= 0 ? "..." + target.substring(lastSlash) : target;
}
+ static int sortByType(Map<String, String> m1, Map<String, String> m2) {
+ return TYPE_WEIGHT.get(m1.get("type")).compareTo(TYPE_WEIGHT.get(m2.get("type")));
+ }
+
private final ObjectReader reader;
private final GitilesView view;
private final Config cfg;
@@ -90,7 +102,7 @@
throws MissingObjectException, IOException {
ReadmeHelper readme =
new ReadmeHelper(reader, view, MarkdownConfig.get(cfg), rootTree, requestUri);
- List<Object> entries = Lists.newArrayList();
+ List<Map<String, String>> entries = Lists.newArrayList();
GitilesView.Builder urlBuilder = GitilesView.path().copyFrom(view);
while (tw.next()) {
FileType type = FileType.forEntry(tw);
@@ -129,6 +141,8 @@
entries.add(entry);
}
+ entries.sort(TreeSoyData::sortByType);
+
Map<String, Object> data = Maps.newHashMapWithExpectedSize(3);
data.put("sha", treeId.name());
data.put("entries", entries);
diff --git a/java/com/google/gitiles/ViewFilter.java b/java/com/google/gitiles/ViewFilter.java
index 25d6d7e..dd3d54a 100644
--- a/java/com/google/gitiles/ViewFilter.java
+++ b/java/com/google/gitiles/ViewFilter.java
@@ -85,12 +85,17 @@
private final GitilesUrls urls;
private final GitilesAccess.Factory accessFactory;
private final VisibilityCache visibilityCache;
+ private final BranchRedirect branchRedirect;
public ViewFilter(
- GitilesAccess.Factory accessFactory, GitilesUrls urls, VisibilityCache visibilityCache) {
+ GitilesAccess.Factory accessFactory,
+ GitilesUrls urls,
+ VisibilityCache visibilityCache,
+ BranchRedirect branchRedirect) {
this.urls = checkNotNull(urls, "urls");
this.accessFactory = checkNotNull(accessFactory, "accessFactory");
this.visibilityCache = checkNotNull(visibilityCache, "visibilityCache");
+ this.branchRedirect = checkNotNull(branchRedirect, "branchRedirect");
}
@Override
@@ -101,7 +106,6 @@
throw new GitilesRequestFailureException(FailureReason.CANNOT_PARSE_GITILES_VIEW);
}
- @SuppressWarnings("unchecked")
Map<String, String[]> params = req.getParameterMap();
view.setHostName(urls.getHostName(req))
.setServletPath(req.getContextPath() + req.getServletPath())
@@ -320,7 +324,14 @@
throws IOException {
RevisionParser revParser =
new RevisionParser(
- ServletUtils.getRepository(req), accessFactory.forRequest(req), visibilityCache);
+ ServletUtils.getRepository(req),
+ accessFactory.forRequest(req),
+ visibilityCache,
+ getBranchRedirect(req));
return revParser.parse(checkLeadingSlash(path));
}
+
+ private BranchRedirect getBranchRedirect(HttpServletRequest req) {
+ return BranchRedirect.isForAutomation(req) ? BranchRedirect.EMPTY : branchRedirect;
+ }
}
diff --git a/java/com/google/gitiles/dev/DevServer.java b/java/com/google/gitiles/dev/DevServer.java
index 5a8fed5..028edd6 100644
--- a/java/com/google/gitiles/dev/DevServer.java
+++ b/java/com/google/gitiles/dev/DevServer.java
@@ -19,7 +19,7 @@
import com.google.common.base.Strings;
import com.google.common.html.types.UncheckedConversions;
-import com.google.gitiles.BranchRedirectFilter;
+import com.google.gitiles.BranchRedirect;
import com.google.gitiles.DebugRenderer;
import com.google.gitiles.GitilesAccess;
import com.google.gitiles.GitilesServlet;
@@ -140,7 +140,7 @@
} else {
servlet =
new GitilesServlet(
- cfg, renderer, null, null, null, null, null, null, null, new BranchRedirectFilter());
+ cfg, renderer, null, null, null, null, null, null, null, new BranchRedirect());
}
ServletContextHandler handler = new ServletContextHandler();
diff --git a/javatests/com/google/gitiles/BranchRedirectFilterTest.java b/javatests/com/google/gitiles/BranchRedirectTest.java
similarity index 69%
rename from javatests/com/google/gitiles/BranchRedirectFilterTest.java
rename to javatests/com/google/gitiles/BranchRedirectTest.java
index 76c6198..b9e4890 100644
--- a/javatests/com/google/gitiles/BranchRedirectFilterTest.java
+++ b/javatests/com/google/gitiles/BranchRedirectTest.java
@@ -15,7 +15,6 @@
package com.google.gitiles;
import static com.google.common.truth.Truth.assertThat;
-import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY;
import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
import static javax.servlet.http.HttpServletResponse.SC_OK;
@@ -31,14 +30,13 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/** Tests for BranchRedirect. */
@RunWith(JUnit4.class)
-public class BranchRedirectFilterTest {
+public class BranchRedirectTest {
private static final String MASTER = "refs/heads/master";
private static final String MAIN = "refs/heads/main";
private static final String DEVELOP = "refs/heads/develop";
@@ -54,8 +52,8 @@
@Before
public void setUp() throws Exception {
repo = new TestRepository<>(new InMemoryRepository(new DfsRepositoryDescription("repo")));
- BranchRedirectFilter branchRedirectFilter =
- new BranchRedirectFilter() {
+ BranchRedirect branchRedirect =
+ new BranchRedirect() {
@Override
protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
if (MASTER.equals(toFullBranchName(sourceBranch))) {
@@ -67,7 +65,7 @@
return Optional.empty();
}
};
- servlet = TestGitilesServlet.create(repo, new GitwebRedirectFilter(), branchRedirectFilter);
+ servlet = TestGitilesServlet.create(repo, new GitwebRedirectFilter(), branchRedirect);
}
@Test
@@ -84,48 +82,52 @@
@Test
public void show_withRedirect() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+/refs/heads/master/foo";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main/foo?format=html");
+ assertThat(res.getActualBodyString()).contains("repo/+/refs/heads/main/foo");
+ assertThat(res.getActualBodyString()).doesNotContain("repo/+/refs/heads/master/foo");
}
@Test
public void show_withRedirect_onDefaultFormatType() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+/refs/heads/master/foo";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, null);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION)).isEqualTo("/b/repo/+/refs/heads/main/foo");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("repo/+/refs/heads/main/foo");
+ assertThat(res.getActualBodyString()).doesNotContain("repo/+/refs/heads/master/foo");
}
@Test
public void show_withRedirect_usingShortRefInUrl() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+/master/foo";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main/foo?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("repo/+/refs/heads/main/foo");
+ assertThat(res.getActualBodyString()).doesNotContain("repo/+/master/foo");
}
@Test
public void show_onAutomationRequest() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+/refs/heads/master/foo";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_JSON);
@@ -133,12 +135,15 @@
servlet.service(req, res);
assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("\"revision\": \"refs/heads/master\"");
+ assertThat(res.getActualBodyString()).contains("\"path\": \"foo\"");
}
@Test
public void showParent_withRedirect() throws Exception {
RevCommit parent = repo.branch(MASTER).commit().add("foo", "contents").create();
repo.branch(MASTER).commit().add("bar", "contents").parent(parent).create();
+ repo.branch(MAIN).commit().parent(parent).create();
String path = "/repo/+/refs/heads/master^";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
@@ -153,7 +158,8 @@
@Test
public void diff_withRedirect_onSingleBranch() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
repo.branch(DEVELOP).commit().add("foo", "contents").create();
String path = "/repo/+/refs/heads/master..refs/heads/develop";
@@ -161,107 +167,108 @@
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main..refs/heads/develop/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString())
+ .contains("/b/repo/+/refs/heads/main..refs/heads/develop/?format=html");
}
@Test
- // @Ignore
public void diff_withRedirect_onBothBranch() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
- repo.branch(FOO).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
+ RevCommit foo = repo.branch(FOO).commit().add("foo", "contents").create();
+ repo.branch(BAR).commit().parent(foo).create();
String path = "/repo/+/refs/heads/foo..refs/heads/master";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/bar..refs/heads/main/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString())
+ .contains("/b/repo/+/refs/heads/bar..refs/heads/main/?format=html");
}
@Test
public void diff_withRedirect() throws Exception {
- repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit master = repo.branch(MASTER).commit().add("foo", "contents").create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+diff/refs/heads/master^!";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main%5E%21/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("/b/repo/+/refs/heads/main%5E%21/?format=html");
}
@Test
public void log_withRedirect() throws Exception {
repo.branch(MASTER).commit().add("foo", "contents").create();
+ RevCommit main = repo.branch(MAIN).commit().create();
String path = "/repo/+log/refs/heads/master";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+log/refs/heads/main/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("Log - refs/heads/main");
+ assertThat(res.getActualBodyString()).contains("/b/repo/+/" + main.toObjectId().getName());
}
@Test
- @Ignore
public void diff_withGrandParent_redirect() throws Exception {
RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
RevCommit parent2 =
repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
- repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ RevCommit master = repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+diff/refs/heads/master^^..refs/heads/master";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main%5E%5E..refs/heads/main/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString())
+ .contains("/b/repo/+/refs/heads/main%5E%5E..refs/heads/main/?format=html");
}
@Test
- @Ignore
public void diff_withRelativeParent_redirect() throws Exception {
RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
RevCommit parent2 =
repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
- repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ RevCommit master = repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+diff/refs/heads/master~1..refs/heads/master";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main%5E%21/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString()).contains("/b/repo/+/refs/heads/main%5E%21/?format=html");
}
@Test
- @Ignore
public void diff_withRelativeGrandParent_redirect() throws Exception {
RevCommit parent1 = repo.branch(MASTER).commit().add("foo", "contents").create();
RevCommit parent2 =
repo.branch(MASTER).commit().add("bar", "contents").parent(parent1).create();
- repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ RevCommit master = repo.branch(MASTER).commit().add("bar", "contents").parent(parent2).create();
+ repo.branch(MAIN).commit().parent(master).create();
String path = "/repo/+diff/refs/heads/master~2..refs/heads/master";
FakeHttpServletRequest req = newHttpRequest(path, ORIGIN, QUERY_STRING_HTML);
FakeHttpServletResponse res = new FakeHttpServletResponse();
servlet.service(req, res);
- assertThat(res.getStatus()).isEqualTo(SC_MOVED_PERMANENTLY);
- assertThat(res.getHeader(HttpHeaders.LOCATION))
- .isEqualTo("/b/repo/+/refs/heads/main%7E2..refs/heads/main/?format=html");
+ assertThat(res.getStatus()).isEqualTo(SC_OK);
+ assertThat(res.getActualBodyString())
+ .contains("/b/repo/+/refs/heads/main%7E2..refs/heads/main/?format=html");
}
private static String toFullBranchName(String sourceBranch) {
diff --git a/javatests/com/google/gitiles/PathServletTest.java b/javatests/com/google/gitiles/PathServletTest.java
index a32de9f..68b1e3e 100644
--- a/javatests/com/google/gitiles/PathServletTest.java
+++ b/javatests/com/google/gitiles/PathServletTest.java
@@ -64,8 +64,8 @@
assertThat(data).containsEntry("type", "TREE");
List<Map<String, ?>> entries = getTreeEntries(data);
assertThat(entries).hasSize(2);
- assertThat(entries.get(0).get("name")).isEqualTo("baz");
- assertThat(entries.get(1).get("name")).isEqualTo("foo/");
+ assertThat(entries.get(0).get("name")).isEqualTo("foo/");
+ assertThat(entries.get(1).get("name")).isEqualTo("baz");
data = buildData("/repo/+/master/foo");
assertThat(data).containsEntry("type", "TREE");
diff --git a/javatests/com/google/gitiles/RevisionParserTest.java b/javatests/com/google/gitiles/RevisionParserTest.java
index 16e76a7..b8ed94d 100644
--- a/javatests/com/google/gitiles/RevisionParserTest.java
+++ b/javatests/com/google/gitiles/RevisionParserTest.java
@@ -46,7 +46,8 @@
new RevisionParser(
repo.getRepository(),
new TestGitilesAccess(repo.getRepository()).forRequest(null),
- new VisibilityCache(CacheBuilder.newBuilder().maximumSize(0)));
+ new VisibilityCache(CacheBuilder.newBuilder().maximumSize(0)),
+ new BranchRedirect());
}
@Test
diff --git a/javatests/com/google/gitiles/TestGitilesServlet.java b/javatests/com/google/gitiles/TestGitilesServlet.java
index a35e76f..036707c 100644
--- a/javatests/com/google/gitiles/TestGitilesServlet.java
+++ b/javatests/com/google/gitiles/TestGitilesServlet.java
@@ -31,17 +31,17 @@
/** Static utility methods for creating {@link GitilesServlet}s for testing. */
public class TestGitilesServlet {
- /** @see #create(TestRepository,GitwebRedirectFilter,BranchRedirectFilter) */
+ /** @see #create(TestRepository,GitwebRedirectFilter, BranchRedirect) */
public static GitilesServlet create(final TestRepository<DfsRepository> repo)
throws ServletException {
- return create(repo, new GitwebRedirectFilter(), new BranchRedirectFilter());
+ return create(repo, new GitwebRedirectFilter(), new BranchRedirect());
}
- /** @see #create(TestRepository,GitwebRedirectFilter,BranchRedirectFilter) */
+ /** @see #create(TestRepository,GitwebRedirectFilter, BranchRedirect) */
public static GitilesServlet create(
final TestRepository<DfsRepository> repo, GitwebRedirectFilter gitwebRedirect)
throws ServletException {
- return create(repo, gitwebRedirect, new BranchRedirectFilter());
+ return create(repo, gitwebRedirect, new BranchRedirect());
}
/**
@@ -61,7 +61,7 @@
public static GitilesServlet create(
final TestRepository<DfsRepository> repo,
GitwebRedirectFilter gitwebRedirect,
- BranchRedirectFilter branchRedirect)
+ BranchRedirect branchRedirect)
throws ServletException {
final String repoName = repo.getRepository().getDescription().getRepositoryName();
GitilesServlet servlet =
diff --git a/javatests/com/google/gitiles/TestViewFilter.java b/javatests/com/google/gitiles/TestViewFilter.java
index 4f67efc..6b6a006 100644
--- a/javatests/com/google/gitiles/TestViewFilter.java
+++ b/javatests/com/google/gitiles/TestViewFilter.java
@@ -58,14 +58,18 @@
}
}
- public static Result service(TestRepository<? extends DfsRepository> repo, String pathAndQuery)
+ public static Result service(
+ TestRepository<? extends DfsRepository> repo,
+ String pathAndQuery,
+ BranchRedirect branchRedirect)
throws IOException, ServletException {
TestServlet servlet = new TestServlet();
ViewFilter vf =
new ViewFilter(
new TestGitilesAccess(repo.getRepository()),
TestGitilesUrls.URLS,
- new VisibilityCache());
+ new VisibilityCache(),
+ branchRedirect);
MetaFilter mf = new MetaFilter();
for (Pattern p : ImmutableList.of(ROOT_REGEX, REPO_REGEX, REPO_PATH_REGEX)) {
diff --git a/javatests/com/google/gitiles/TreeSoyDataTest.java b/javatests/com/google/gitiles/TreeSoyDataTest.java
index 98d416a..cafe4e2 100644
--- a/javatests/com/google/gitiles/TreeSoyDataTest.java
+++ b/javatests/com/google/gitiles/TreeSoyDataTest.java
@@ -17,8 +17,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gitiles.TreeSoyData.getTargetDisplayName;
import static com.google.gitiles.TreeSoyData.resolveTargetUrl;
+import static com.google.gitiles.TreeSoyData.sortByType;
import com.google.common.base.Strings;
+import java.util.Map;
+import java.util.HashMap;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -64,4 +67,25 @@
assertThat(resolveTargetUrl(view, "../../../../")).isNull();
assertThat(resolveTargetUrl(view, "../../a/../../..")).isNull();
}
+
+ @Test
+ public void sortByTypeSortsCorrect() throws Exception {
+ Map<String, String> m1 = new HashMap<String, String>();
+ Map<String, String> m2 = new HashMap<String, String>();
+ Map<String, String> m3 = new HashMap<String, String>();
+ Map<String, String> m4 = new HashMap<String, String>();
+ Map<String, String> m5 = new HashMap<String, String>();
+ m1.put("type", "TREE");
+ m2.put("type", "TREE");
+ m3.put("type", "SYMLINK");
+ m4.put("type", "REGULAR_FILE");
+ m5.put("type", "GITLINK");
+ assertThat(sortByType(m1, m2)).isEqualTo(0);
+ assertThat(sortByType(m2, m3)).isEqualTo(-1);
+ assertThat(sortByType(m3, m4)).isEqualTo(-1);
+ assertThat(sortByType(m4, m1)).isEqualTo(1);
+ assertThat(sortByType(m1, m4)).isEqualTo(-1);
+ assertThat(sortByType(m5, m2)).isEqualTo(1);
+ assertThat(sortByType(m2, m5)).isEqualTo(-1);
+ }
}
diff --git a/javatests/com/google/gitiles/ViewFilterTest.java b/javatests/com/google/gitiles/ViewFilterTest.java
index 66cd5d8..fb001cd 100644
--- a/javatests/com/google/gitiles/ViewFilterTest.java
+++ b/javatests/com/google/gitiles/ViewFilterTest.java
@@ -21,11 +21,14 @@
import com.google.common.net.HttpHeaders;
import com.google.gitiles.GitilesView.Type;
import java.io.IOException;
+import java.util.Optional;
import javax.servlet.ServletException;
import org.eclipse.jgit.internal.storage.dfs.DfsRepository;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -53,7 +56,7 @@
@Test
public void autoCommand() throws Exception {
RevCommit parent = repo.commit().create();
- RevCommit master = repo.branch("refs/heads/master").commit().parent(parent).create();
+ RevCommit master = repo.branch(MASTER).commit().parent(parent).create();
String hex = master.name();
String hexBranch = hex.substring(0, 10);
repo.branch(hexBranch).commit().create();
@@ -157,7 +160,7 @@
@Test
public void showBranches() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
RevCommit stable = repo.branch("refs/heads/stable").commit().create();
GitilesView view;
@@ -175,7 +178,7 @@
view = getView("/repo/+show/refs/heads/master");
assertThat(view.getType()).isEqualTo(Type.REVISION);
- assertThat(view.getRevision().getName()).isEqualTo("refs/heads/master");
+ assertThat(view.getRevision().getName()).isEqualTo(MASTER);
assertThat(view.getRevision().getId()).isEqualTo(master);
assertThat(view.getPathPart()).isNull();
@@ -227,7 +230,7 @@
@Test
public void path() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
repo.branch("refs/heads/stable").commit().create();
GitilesView view;
@@ -257,7 +260,7 @@
@Test
public void doc() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
repo.branch("refs/heads/stable").commit().create();
GitilesView view;
@@ -287,7 +290,7 @@
@Test
public void multipleSlashes() throws Exception {
- repo.branch("refs/heads/master").commit().create();
+ repo.branch(MASTER).commit().create();
assertThat(getView("//").getType()).isEqualTo(Type.HOST_INDEX);
assertThat(getView("//repo").getType()).isEqualTo(Type.REPOSITORY_INDEX);
assertThat(getView("//repo//").getType()).isEqualTo(Type.REPOSITORY_INDEX);
@@ -301,7 +304,7 @@
@Test
public void diff() throws Exception {
RevCommit parent = repo.commit().create();
- RevCommit master = repo.branch("refs/heads/master").commit().parent(parent).create();
+ RevCommit master = repo.branch(MASTER).commit().parent(parent).create();
GitilesView view;
view = getView("/repo/+diff/master^..master");
@@ -330,7 +333,7 @@
view = getView("/repo/+diff/refs/heads/master^..refs/heads/master");
assertThat(view.getType()).isEqualTo(Type.DIFF);
- assertThat(view.getRevision().getName()).isEqualTo("refs/heads/master");
+ assertThat(view.getRevision().getName()).isEqualTo(MASTER);
assertThat(view.getRevision().getId()).isEqualTo(master);
assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/master^");
assertThat(view.getOldRevision().getId()).isEqualTo(parent);
@@ -339,7 +342,7 @@
@Test
public void diffAgainstEmptyCommit() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
GitilesView view = getView("/repo/+diff/master^!");
assertThat(view.getType()).isEqualTo(Type.DIFF);
assertThat(view.getRevision().getName()).isEqualTo("master");
@@ -351,7 +354,7 @@
@Test
public void log() throws Exception {
RevCommit parent = repo.commit().create();
- RevCommit master = repo.branch("refs/heads/master").commit().parent(parent).create();
+ RevCommit master = repo.branch(MASTER).commit().parent(parent).create();
GitilesView view;
view = getView("/repo/+log");
@@ -411,7 +414,7 @@
view = getView("/repo/+log/refs/heads/master^..refs/heads/master");
assertThat(view.getType()).isEqualTo(Type.LOG);
- assertThat(view.getRevision().getName()).isEqualTo("refs/heads/master");
+ assertThat(view.getRevision().getName()).isEqualTo(MASTER);
assertThat(view.getRevision().getId()).isEqualTo(master);
assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/master^");
assertThat(view.getOldRevision().getId()).isEqualTo(parent);
@@ -420,7 +423,7 @@
@Test
public void archive() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
repo.branch("refs/heads/branch").commit().create();
GitilesView view;
@@ -465,7 +468,7 @@
@Test
public void blame() throws Exception {
- RevCommit master = repo.branch("refs/heads/master").commit().create();
+ RevCommit master = repo.branch(MASTER).commit().create();
repo.branch("refs/heads/branch").commit().create();
GitilesView view;
@@ -487,7 +490,7 @@
@Test
public void testNormalizeParents() throws Exception {
RevCommit parent = repo.commit().create();
- RevCommit master = repo.branch("refs/heads/master").commit().parent(parent).create();
+ RevCommit master = repo.branch(MASTER).commit().parent(parent).create();
GitilesView view;
assertThat(getView("/repo/+/master").toUrl()).isEqualTo("/b/repo/+/master");
@@ -504,18 +507,208 @@
assertThat(view.getOldRevision().getName()).isEqualTo("master^");
}
+ private static final String MASTER = "refs/heads/master";
+ private static final String MAIN = "refs/heads/main";
+
+ @Test
+ public void autoCommand_branchRedirect() throws Exception {
+ RevCommit parent = repo.commit().create();
+ RevCommit master = repo.branch(MASTER).commit().parent(parent).create();
+ RevCommit main = repo.branch(MAIN).commit().parent(parent).create();
+ RevCommit someBranch =
+ repo.branch("refs/heads/some@branch")
+ .commit()
+ .parent(main)
+ .add("README", "This is a test README")
+ .create();
+ repo.branch("refs/heads/another@level").commit().parent(someBranch).create();
+
+ String hex = master.name();
+ String hexBranch = hex.substring(0, 10);
+ repo.branch(hexBranch).commit().create();
+
+ BranchRedirect branchRedirect =
+ new BranchRedirect() {
+ @Override
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ if (MASTER.equals(toFullBranchName(sourceBranch))) {
+ return Optional.of(MAIN);
+ }
+ if ("refs/heads/some@branch".equals(toFullBranchName(sourceBranch))) {
+ return Optional.of(MAIN);
+ }
+ return Optional.empty();
+ }
+ };
+
+ GitilesView view = getView("/repo/+/master", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.REVISION);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+
+ view = getView("/repo/+/master/index.c", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.PATH);
+ assertThat(view.getPathPart()).isEqualTo("index.c");
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+
+ view = getView("/repo/+/some@branch", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.REVISION);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+
+ view = getView("/repo/+/master/master", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.PATH);
+ assertThat(view.getPathPart()).isEqualTo("master");
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+
+ FakeHttpServletResponse response = getResponse("/repo/+/master^1", branchRedirect);
+ assertThat(response.getHeader(HttpHeaders.LOCATION))
+ .contains("/b/repo/+/" + parent.toObjectId().name());
+
+ response = getResponse("/repo/+/master~1", branchRedirect);
+ assertThat(response.getHeader(HttpHeaders.LOCATION))
+ .contains("/b/repo/+/" + parent.toObjectId().name());
+
+ response = getResponse("/repo/+/another@level^1~2", branchRedirect);
+ assertThat(response.getHeader(HttpHeaders.LOCATION))
+ .contains("/b/repo/+/" + parent.toObjectId().name());
+
+ assertThrows(
+ GitilesRequestFailureException.class,
+ () -> getView("/repo/+/some@branch:README", branchRedirect));
+
+ assertThrows(
+ GitilesRequestFailureException.class, () -> getView("/repo/+/master@{1}", branchRedirect));
+ }
+
+ @Test
+ public void diff_branchRedirect() throws Exception {
+ RevCommit parent = repo.commit().create();
+ repo.branch(MASTER).commit().parent(parent).create();
+ RevCommit main = repo.branch(MAIN).commit().parent(parent).create();
+ BranchRedirect branchRedirect =
+ new BranchRedirect() {
+ @Override
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ if (MASTER.equals(toFullBranchName(sourceBranch))) {
+ return Optional.of(MAIN);
+ }
+ return Optional.empty();
+ }
+ };
+
+ GitilesView view;
+
+ view = getView("/repo/+diff/master^..master", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.DIFF);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/main^");
+ assertThat(view.getOldRevision().getId()).isEqualTo(parent);
+ assertThat(view.getPathPart()).isEmpty();
+
+ view = getView("/repo/+diff/master..master^", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.DIFF);
+ assertThat(view.getRevision().getName()).isEqualTo("refs/heads/main^");
+ assertThat(view.getRevision().getId()).isEqualTo(parent);
+ assertThat(view.getOldRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getOldRevision().getId()).isEqualTo(main);
+ assertThat(view.getPathPart()).isEmpty();
+
+ view = getView("/repo/+diff/master^..master/", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.DIFF);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/main^");
+ assertThat(view.getOldRevision().getId()).isEqualTo(parent);
+ assertThat(view.getPathPart()).isEmpty();
+
+ view = getView("/repo/+diff/master^..master/foo", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.DIFF);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/main^");
+ assertThat(view.getOldRevision().getId()).isEqualTo(parent);
+ assertThat(view.getPathPart()).isEqualTo("foo");
+
+ view = getView("/repo/+diff/refs/heads/master^..refs/heads/master", branchRedirect);
+ assertThat(view.getType()).isEqualTo(Type.DIFF);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getOldRevision().getName()).isEqualTo("refs/heads/main^");
+ assertThat(view.getOldRevision().getId()).isEqualTo(parent);
+ assertThat(view.getPathPart()).isEmpty();
+ }
+
+ @Test
+ public void path_branchRedirect() throws Exception {
+ RevCommit parent = repo.commit().create();
+ RevCommit main = repo.branch(MAIN).commit().parent(parent).create();
+ repo.branch(MASTER).commit().parent(parent).create();
+ BranchRedirect branchRedirect =
+ new BranchRedirect() {
+ @Override
+ protected Optional<String> getRedirectBranch(Repository repo, String sourceBranch) {
+ if (MASTER.equals(toFullBranchName(sourceBranch))) {
+ return Optional.of(MAIN);
+ }
+ return Optional.empty();
+ }
+ };
+
+ repo.branch("refs/heads/stable").commit().create();
+ GitilesView view;
+
+ view = getView("/repo/+show/master/", branchRedirect);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getType()).isEqualTo(Type.PATH);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getPathPart()).isEmpty();
+
+ view = getView("/repo/+show/master/foo", branchRedirect);
+ assertThat(view.getRevision().getName()).isEqualTo(MAIN);
+ assertThat(view.getType()).isEqualTo(Type.PATH);
+ assertThat(view.getRevision().getId()).isEqualTo(main);
+ assertThat(view.getPathPart()).isEqualTo("foo");
+ }
+
+ private static String toFullBranchName(String sourceBranch) {
+ if (sourceBranch.startsWith(Constants.R_REFS)) {
+ return sourceBranch;
+ }
+ return Constants.R_HEADS + sourceBranch;
+ }
+
private String getRedirectUrl(String pathAndQuery) throws ServletException, IOException {
- TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery);
+ TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery, new BranchRedirect());
assertThat(result.getResponse().getStatus()).isEqualTo(302);
return result.getResponse().getHeader(HttpHeaders.LOCATION);
}
private GitilesView getView(String pathAndQuery) throws ServletException, IOException {
- TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery);
+ TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery, new BranchRedirect());
FakeHttpServletResponse resp = result.getResponse();
assertWithMessage("expected non-redirect status, got " + resp.getStatus())
.that(resp.getStatus() < 300 || resp.getStatus() >= 400)
.isTrue();
return result.getView();
}
+
+ private GitilesView getView(String pathAndQuery, BranchRedirect branchRedirect)
+ throws ServletException, IOException {
+ TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery, branchRedirect);
+ FakeHttpServletResponse resp = result.getResponse();
+ assertWithMessage("expected non-redirect status, got " + resp.getStatus())
+ .that(resp.getStatus() < 300 || resp.getStatus() >= 400 || resp.getStatus() == 302)
+ .isTrue();
+ return result.getView();
+ }
+
+ private FakeHttpServletResponse getResponse(String pathAndQuery, BranchRedirect branchRedirect)
+ throws ServletException, IOException {
+ TestViewFilter.Result result = TestViewFilter.service(repo, pathAndQuery, branchRedirect);
+ return result.getResponse();
+ }
}
diff --git a/resources/com/google/gitiles/static/base.css b/resources/com/google/gitiles/static/base.css
index 9719920..a1d5a36 100644
--- a/resources/com/google/gitiles/static/base.css
+++ b/resources/com/google/gitiles/static/base.css
@@ -296,7 +296,9 @@
.RefList-title {
margin: 0;
}
-.RefList-items {}
+.RefList-items {
+ word-wrap: break-word;
+}
.RefList-item {
padding: 2px 0;
}
diff --git a/resources/com/google/gitiles/templates/BlameDetail.soy b/resources/com/google/gitiles/templates/BlameDetail.soy
index 366419d..1589056 100644
--- a/resources/com/google/gitiles/templates/BlameDetail.soy
+++ b/resources/com/google/gitiles/templates/BlameDetail.soy
@@ -47,8 +47,7 @@
{call objDetail.blobHeader data="$data" /}
<table class="Blame">
- {for $line in $data.lines}
- {let $i: index($line) /}
+ {for $line, $i in $data.lines}
{let $region: $regions[$i] /}
<tr class="Blame-region {$region.class}">
{if $region.abbrevSha != null}
diff --git a/resources/com/google/gitiles/templates/Common.soy b/resources/com/google/gitiles/templates/Common.soy
index 4b52d3b..3a33299 100644
--- a/resources/com/google/gitiles/templates/Common.soy
+++ b/resources/com/google/gitiles/templates/Common.soy
@@ -76,9 +76,9 @@
<div class="Container {if $containerClass}{$containerClass}{/if}">
{if $breadcrumbs and length($breadcrumbs)}
<div class="Breadcrumbs">
- {for $entry in $breadcrumbs}
- {if not isFirst($entry)}{sp}/{sp}{/if}
- {if not isLast($entry)}
+ {for $entry, $index in $breadcrumbs}
+ {if $index > 0}{sp}/{sp}{/if}
+ {if $index < length($breadcrumbs) - 1}
<a class="Breadcrumbs-crumb" href="{$entry.url}">{$entry.text}</a>
{else}
<span class="Breadcrumbs-crumb">{$entry.text}</span>
diff --git a/resources/com/google/gitiles/templates/DiffDetail.soy b/resources/com/google/gitiles/templates/DiffDetail.soy
index 782d006..519ed77 100644
--- a/resources/com/google/gitiles/templates/DiffDetail.soy
+++ b/resources/com/google/gitiles/templates/DiffDetail.soy
@@ -49,8 +49,8 @@
{@param fileIndex: ?} /** position of the file within the difference. */
<pre class="u-pre u-monospace Diff">
<a name="F{$fileIndex}" class="Diff-fileIndex"></a>
- {for $part in $firstParts}
- {if not isFirst($part)}{sp}{/if}
+ {for $part, $index in $firstParts}
+ {if $index > 0}{sp}{/if}
{if $part.url}
<a href="{$part.url}">{$part.text}</a>
{else}
diff --git a/resources/com/google/gitiles/templates/ObjectDetail.soy b/resources/com/google/gitiles/templates/ObjectDetail.soy
index b4f4426..4fc4b99 100644
--- a/resources/com/google/gitiles/templates/ObjectDetail.soy
+++ b/resources/com/google/gitiles/templates/ObjectDetail.soy
@@ -32,6 +32,7 @@
text: raw text of the part.
url: optional URL that should be linked to from the part.
*/
+ {@param notes: ?} /** the notes for the corresponding commit */
{@param diffTree: ?} /** list of changed tree entries with the following keys:
changeType: string matching an org.eclipse.jgit.diff.DiffEntry.ChangeType constant.
path: (new) path of the tree entry.
@@ -95,6 +96,15 @@
{param message: $message /}
{/call}
+{if $notes}
+ <h4>Notes:</h4>
+ <div class="MetadataMessage">
+ <span>
+ {msg desc="Text for the git notes"}{$notes}{/msg}
+ </span>
+ </div>
+{/if}
+
{if $diffTree and length($diffTree)}
<ul class="DiffTree">
{for $entry in $diffTree}
@@ -256,8 +266,8 @@
{if $lines != null}
{if $lines}
<table class="FileContents">
- {for $line in $lines}
- {let $n: index($line) + 1 /}
+ {for $line, $index in $lines}
+ {let $n: $index + 1 /}
<tr class="u-pre u-monospace FileContents-line">
<td class="u-lineNum u-noSelect FileContents-lineNum"
data-line-number="{$n}" onclick="window.location.hash='#{$n}'"></td>
diff --git a/version.bzl b/version.bzl
index c5200a7..3c1285c 100644
--- a/version.bzl
+++ b/version.bzl
@@ -4,4 +4,4 @@
# we currently have no stable releases, we use the "build number" scheme
# described at:
# https://www.mojohaus.org/versions-maven-plugin/version-rules.html
-GITILES_VERSION = "0.4"
+GITILES_VERSION = "0.4-1"
