diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedInResolver.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedInResolver.java index 9c4817e5b9..201ee149a6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedInResolver.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedInResolver.java @@ -49,7 +49,31 @@ public class IncludedInResolver { public static IncludedInDetail resolve(final Repository repo, final RevWalk rw, final RevCommit commit) throws IOException { + return new IncludedInResolver(repo, rw, commit).resolve(); + } + public static boolean includedInOne(final Repository repo, final RevWalk rw, + final RevCommit commit, final Collection refs) throws IOException { + return new IncludedInResolver(repo, rw, commit).includedInOne(refs); + } + + private final Repository repo; + private final RevWalk rw; + private final RevCommit target; + + private final RevFlag containsTarget; + private Multimap commitToRef; + private List tipsByCommitTime; + + private IncludedInResolver(final Repository repo, final RevWalk rw, + final RevCommit target) { + this.repo = repo; + this.rw = rw; + this.target = target; + this.containsTarget = rw.newFlag("CONTAINS_TARGET"); + } + + private IncludedInDetail resolve() throws IOException { RefDatabase refDb = repo.getRefDatabase(); Collection tags = refDb.getRefs(Constants.R_TAGS).values(); Collection branches = refDb.getRefs(Constants.R_HEADS).values(); @@ -57,8 +81,8 @@ public class IncludedInResolver { tags.size() + branches.size()); allTagsAndBranches.addAll(tags); allTagsAndBranches.addAll(branches); - Set allMatchingTagsAndBranches = - includedIn(repo, rw, commit, allTagsAndBranches); + parseCommits(allTagsAndBranches); + Set allMatchingTagsAndBranches = includedIn(tipsByCommitTime, 0); IncludedInDetail detail = new IncludedInDetail(); detail @@ -68,27 +92,22 @@ public class IncludedInResolver { return detail; } + private boolean includedInOne(final Collection refs) throws IOException { + parseCommits(refs); + List before = Lists.newLinkedList(); + List after = Lists.newLinkedList(); + partition(before, after); + // It is highly likely that the target is reachable from the "after" set + // Within the "before" set we are trying to handle cases arising from clock skew + return !includedIn(after, 1).isEmpty() || !includedIn(before, 1).isEmpty(); + } + /** * Resolves which tip refs include the target commit. */ - private static Set includedIn(final Repository repo, final RevWalk rw, - final RevCommit target, final Collection tipRefs) throws IOException, - MissingObjectException, IncorrectObjectTypeException { - + private Set includedIn(final Collection tips, int limit) + throws IOException, MissingObjectException, IncorrectObjectTypeException { Set result = Sets.newHashSet(); - - Multimap tipsAndCommits = parseCommits(repo, rw, tipRefs); - - List tips = Lists.newArrayList(tipsAndCommits.keySet()); - Collections.sort(tips, new Comparator() { - @Override - public int compare(RevCommit c1, RevCommit c2) { - return c1.getCommitTime() - c2.getCommitTime(); - } - }); - - RevFlag containsTarget = rw.newFlag("CONTAINS_TARGET"); - for (RevCommit tip : tips) { boolean commitFound = false; rw.resetRetain(RevFlag.UNINTERESTING, containsTarget); @@ -97,18 +116,51 @@ public class IncludedInResolver { if (commit.equals(target) || commit.has(containsTarget)) { commitFound = true; tip.add(containsTarget); - result.addAll(tipsAndCommits.get(tip)); + result.addAll(commitToRef.get(tip)); break; } } if (!commitFound) { rw.markUninteresting(tip); + } else if (0 < limit && limit < result.size()) { + break; } } - return result; } + /** + * Partition the reference tips into two sets: + *
    + *
  • before = commits with time < target.getCommitTime() + *
  • after = commits with time >= target.getCommitTime() + *
+ * + * Each of the before/after lists is sorted by the the commit time. + * + * @param before + * @param after + */ + private void partition(final List before, + final List after) { + int insertionPoint = Collections.binarySearch(tipsByCommitTime, target, + new Comparator() { + @Override + public int compare(RevCommit c1, RevCommit c2) { + return c1.getCommitTime() - c2.getCommitTime(); + } + }); + if (insertionPoint < 0) { + insertionPoint = - (insertionPoint + 1); + } + if (0 < insertionPoint) { + before.addAll(tipsByCommitTime.subList(0, insertionPoint)); + } + if (insertionPoint < tipsByCommitTime.size()) { + after.addAll(tipsByCommitTime.subList(insertionPoint, tipsByCommitTime.size())); + } + } + /** * Returns the short names of refs which are as well in the matchingRefs list * as well as in the allRef list. @@ -127,9 +179,11 @@ public class IncludedInResolver { /** * Parse commit of ref and store the relation between ref and commit. */ - private static Multimap parseCommits(final Repository repo, - final RevWalk rw, final Collection refs) throws IOException { - Multimap result = LinkedListMultimap.create(); + private void parseCommits(final Collection refs) throws IOException { + if (commitToRef != null) { + return; + } + commitToRef = LinkedListMultimap.create(); for (Ref ref : refs) { final RevCommit commit; try { @@ -146,8 +200,18 @@ public class IncludedInResolver { + " points to dangling object " + ref.getObjectId()); continue; } - result.put(commit, ref.getName()); + commitToRef.put(commit, ref.getName()); } - return result; + tipsByCommitTime = Lists.newArrayList(commitToRef.keySet()); + sortOlderFirst(tipsByCommitTime); + } + + private void sortOlderFirst(final List tips) { + Collections.sort(tips, new Comparator() { + @Override + public int compare(RevCommit c1, RevCommit c2) { + return c1.getCommitTime() - c2.getCommitTime(); + } + }); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java index 6c6cba490e..cc929fd234 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java @@ -14,8 +14,6 @@ package com.google.gerrit.server.project; -import static org.eclipse.jgit.lib.RefDatabase.ALL; - import com.google.common.collect.Lists; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.PageLinks; @@ -34,6 +32,7 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.InternalUser; +import com.google.gerrit.server.change.IncludedInResolver; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.GitReceivePackGroups; import com.google.gerrit.server.config.GitUploadPackGroups; @@ -42,8 +41,9 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; -import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -58,7 +58,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Map.Entry; /** Access control management for a user accessing a project's data. */ public class ProjectControl { @@ -498,23 +497,21 @@ public class ProjectControl { try { Repository repo = repoManager.openRepository(projName); try { - Map allRefs = repo.getRefDatabase().getRefs(ALL); - for (Entry entry : allRefs.entrySet()) { - String refName = entry.getKey(); - if (!refName.startsWith("refs/heads") && !refName.startsWith("refs/tags")) { - continue; - } - RevCommit tip; - try { - tip = rw.parseCommit(entry.getValue().getObjectId()); - } catch (IncorrectObjectTypeException e) { - continue; - } - if (controlForRef(entry.getKey()).canPerform(Permission.READ) - && rw.isMergedInto(commit, tip)) { - return true; + RefDatabase refDb = repo.getRefDatabase(); + List allRefs = Lists.newLinkedList(); + allRefs.addAll(refDb.getRefs(Constants.R_HEADS).values()); + allRefs.addAll(refDb.getRefs(Constants.R_TAGS).values()); + List canReadRefs = Lists.newLinkedList(); + for (Ref r : allRefs) { + if (controlForRef(r.getName()).canPerform(Permission.READ)) { + canReadRefs.add(r); } } + + if (!canReadRefs.isEmpty() && IncludedInResolver.includedInOne( + repo, rw, commit, canReadRefs)) { + return true; + } } finally { repo.close(); }