PermissionBackend#filter: Use Collections instead of Maps
Up until now, the ref filter method in PermissionBackend used Maps instead of Collections. The only place where this is useful is a JGit method for advertising refs. For most parts of Gerrit, we waste effort in compiling the map but then only use #values. This commit rewrites the interface and internals to use Collections instead. Change-Id: I1f60c3ca04db6d5cebc978768c5580697df7350d
This commit is contained in:
		@@ -15,15 +15,17 @@
 | 
			
		||||
package com.google.gerrit.server.git;
 | 
			
		||||
 | 
			
		||||
import static java.util.stream.Collectors.toList;
 | 
			
		||||
import static java.util.stream.Collectors.toMap;
 | 
			
		||||
 | 
			
		||||
import com.google.common.base.Preconditions;
 | 
			
		||||
import com.google.common.collect.ImmutableMap;
 | 
			
		||||
import com.google.common.collect.ImmutableList;
 | 
			
		||||
import com.google.common.collect.Iterables;
 | 
			
		||||
import com.google.gerrit.server.permissions.PermissionBackend;
 | 
			
		||||
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
 | 
			
		||||
import com.google.gerrit.server.permissions.PermissionBackendException;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.Collections;
 | 
			
		||||
import java.util.HashMap;
 | 
			
		||||
import java.util.HashSet;
 | 
			
		||||
@@ -75,10 +77,9 @@ public class PermissionAwareReadOnlyRefDatabase extends DelegateRefDatabase {
 | 
			
		||||
      return null;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    Map<String, Ref> result;
 | 
			
		||||
    Collection<Ref> result;
 | 
			
		||||
    try {
 | 
			
		||||
      result =
 | 
			
		||||
          forProject.filter(ImmutableMap.of(name, ref), getDelegate(), RefFilterOptions.defaults());
 | 
			
		||||
      result = forProject.filter(ImmutableList.of(ref), getDelegate(), RefFilterOptions.defaults());
 | 
			
		||||
    } catch (PermissionBackendException e) {
 | 
			
		||||
      if (e.getCause() instanceof IOException) {
 | 
			
		||||
        throw (IOException) e.getCause();
 | 
			
		||||
@@ -91,7 +92,7 @@ public class PermissionAwareReadOnlyRefDatabase extends DelegateRefDatabase {
 | 
			
		||||
 | 
			
		||||
    Preconditions.checkState(
 | 
			
		||||
        result.size() == 1, "Only one element expected, but was: " + result.size());
 | 
			
		||||
    return Iterables.getOnlyElement(result.values());
 | 
			
		||||
    return Iterables.getOnlyElement(result);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @SuppressWarnings("deprecation")
 | 
			
		||||
@@ -102,13 +103,13 @@ public class PermissionAwareReadOnlyRefDatabase extends DelegateRefDatabase {
 | 
			
		||||
      return refs;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    Map<String, Ref> result;
 | 
			
		||||
    Collection<Ref> result;
 | 
			
		||||
    try {
 | 
			
		||||
      result = forProject.filter(refs, getDelegate(), RefFilterOptions.defaults());
 | 
			
		||||
      result = forProject.filter(refs.values(), getDelegate(), RefFilterOptions.defaults());
 | 
			
		||||
    } catch (PermissionBackendException e) {
 | 
			
		||||
      throw new IOException("");
 | 
			
		||||
    }
 | 
			
		||||
    return result;
 | 
			
		||||
    return result.stream().collect(toMap(Ref::getName, r -> r));
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
 
 | 
			
		||||
@@ -20,11 +20,10 @@ import static com.google.gerrit.entities.RefNames.REFS_CACHE_AUTOMERGE;
 | 
			
		||||
import static com.google.gerrit.entities.RefNames.REFS_CONFIG;
 | 
			
		||||
import static com.google.gerrit.entities.RefNames.REFS_USERS_SELF;
 | 
			
		||||
import static java.util.Objects.requireNonNull;
 | 
			
		||||
import static java.util.stream.Collectors.toMap;
 | 
			
		||||
import static java.util.stream.Collectors.toCollection;
 | 
			
		||||
 | 
			
		||||
import com.google.auto.value.AutoValue;
 | 
			
		||||
import com.google.common.collect.ImmutableList;
 | 
			
		||||
import com.google.common.collect.ImmutableMap;
 | 
			
		||||
import com.google.common.collect.ImmutableSet;
 | 
			
		||||
import com.google.common.collect.Iterables;
 | 
			
		||||
import com.google.common.collect.Maps;
 | 
			
		||||
@@ -60,6 +59,7 @@ import com.google.inject.Inject;
 | 
			
		||||
import com.google.inject.assistedinject.Assisted;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.Collections;
 | 
			
		||||
import java.util.HashMap;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
@@ -129,7 +129,7 @@ class DefaultRefFilter {
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /** Filters given refs and tags by visibility. */
 | 
			
		||||
  Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
  Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
    logger.atFinest().log(
 | 
			
		||||
        "Filter refs for repository %s by visibility (options = %s, refs = %s)",
 | 
			
		||||
@@ -145,10 +145,10 @@ class DefaultRefFilter {
 | 
			
		||||
 | 
			
		||||
    // See if we can get away with a single, cheap ref evaluation.
 | 
			
		||||
    if (refs.size() == 1) {
 | 
			
		||||
      String refName = Iterables.getOnlyElement(refs.values()).getName();
 | 
			
		||||
      String refName = Iterables.getOnlyElement(refs).getName();
 | 
			
		||||
      if (opts.filterMeta() && isMetadata(refName)) {
 | 
			
		||||
        logger.atFinest().log("Filter out metadata ref %s", refName);
 | 
			
		||||
        return ImmutableMap.of();
 | 
			
		||||
        return ImmutableList.of();
 | 
			
		||||
      }
 | 
			
		||||
      if (RefNames.isRefsChanges(refName)) {
 | 
			
		||||
        boolean isChangeRefVisisble = canSeeSingleChangeRef(refName);
 | 
			
		||||
@@ -157,18 +157,18 @@ class DefaultRefFilter {
 | 
			
		||||
          return refs;
 | 
			
		||||
        }
 | 
			
		||||
        logger.atFinest().log("Filter out non-visible change ref %s", refName);
 | 
			
		||||
        return ImmutableMap.of();
 | 
			
		||||
        return ImmutableList.of();
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    // Perform an initial ref filtering with all the refs the caller asked for. If we find tags that
 | 
			
		||||
    // we have to investigate separately (deferred tags) then perform a reachability check starting
 | 
			
		||||
    // from all visible branches (refs/heads/*).
 | 
			
		||||
    Result initialRefFilter = filterRefs(refs, repo, opts);
 | 
			
		||||
    Map<String, Ref> visibleRefs = initialRefFilter.visibleRefs();
 | 
			
		||||
    Result initialRefFilter = filterRefs(new ArrayList<>(refs), repo, opts);
 | 
			
		||||
    List<Ref> visibleRefs = initialRefFilter.visibleRefs();
 | 
			
		||||
    if (!initialRefFilter.deferredTags().isEmpty()) {
 | 
			
		||||
      try (TraceTimer traceTimer = TraceContext.newTimer("Check visibility of deferred tags")) {
 | 
			
		||||
        Result allVisibleBranches = filterRefs(getTaggableRefsMap(repo), repo, opts);
 | 
			
		||||
        Result allVisibleBranches = filterRefs(getTaggableRefs(repo), repo, opts);
 | 
			
		||||
        checkState(
 | 
			
		||||
            allVisibleBranches.deferredTags().isEmpty(),
 | 
			
		||||
            "unexpected tags found when filtering refs/heads/* "
 | 
			
		||||
@@ -177,12 +177,12 @@ class DefaultRefFilter {
 | 
			
		||||
        TagMatcher tags =
 | 
			
		||||
            tagCache
 | 
			
		||||
                .get(projectState.getNameKey())
 | 
			
		||||
                .matcher(tagCache, repo, allVisibleBranches.visibleRefs().values());
 | 
			
		||||
                .matcher(tagCache, repo, allVisibleBranches.visibleRefs());
 | 
			
		||||
        for (Ref tag : initialRefFilter.deferredTags()) {
 | 
			
		||||
          try {
 | 
			
		||||
            if (tags.isReachable(tag)) {
 | 
			
		||||
              logger.atFinest().log("Include reachable tag %s", tag.getName());
 | 
			
		||||
              visibleRefs.put(tag.getName(), tag);
 | 
			
		||||
              visibleRefs.add(tag);
 | 
			
		||||
            } else {
 | 
			
		||||
              logger.atFinest().log("Filter out non-reachable tag %s", tag.getName());
 | 
			
		||||
            }
 | 
			
		||||
@@ -202,7 +202,7 @@ class DefaultRefFilter {
 | 
			
		||||
   * separately for later rev-walk-based visibility computation. Tags where visibility is trivial to
 | 
			
		||||
   * compute will be returned as part of {@link Result#visibleRefs()}.
 | 
			
		||||
   */
 | 
			
		||||
  Result filterRefs(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
  Result filterRefs(List<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
    logger.atFinest().log("Filter refs (refs = %s)", refs);
 | 
			
		||||
 | 
			
		||||
@@ -252,9 +252,9 @@ class DefaultRefFilter {
 | 
			
		||||
      identifiedUser = null;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    Map<String, Ref> resultRefs = new HashMap<>();
 | 
			
		||||
    List<Ref> resultRefs = new ArrayList<>(refs.size());
 | 
			
		||||
    List<Ref> deferredTags = new ArrayList<>();
 | 
			
		||||
    for (Ref ref : refs.values()) {
 | 
			
		||||
    for (Ref ref : refs) {
 | 
			
		||||
      String name = ref.getName();
 | 
			
		||||
      Change.Id changeId;
 | 
			
		||||
      Account.Id accountId;
 | 
			
		||||
@@ -268,7 +268,7 @@ class DefaultRefFilter {
 | 
			
		||||
        // Edits are visible only to the owning user, if change is visible.
 | 
			
		||||
        if (viewMetadata || visibleEdit(repo, name)) {
 | 
			
		||||
          logger.atFinest().log("Include edit ref %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out edit ref %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -276,7 +276,7 @@ class DefaultRefFilter {
 | 
			
		||||
        // Change ref is visible only if the change is visible.
 | 
			
		||||
        if (viewMetadata || visible(repo, changeId)) {
 | 
			
		||||
          logger.atFinest().log("Include change ref %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out change ref %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -284,7 +284,7 @@ class DefaultRefFilter {
 | 
			
		||||
        // Account ref is visible only to the corresponding account.
 | 
			
		||||
        if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
 | 
			
		||||
          logger.atFinest().log("Include user ref %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out user ref %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -296,7 +296,7 @@ class DefaultRefFilter {
 | 
			
		||||
                && isGroupOwner(group, identifiedUser, isAdmin)
 | 
			
		||||
                && canReadRef(name))) {
 | 
			
		||||
          logger.atFinest().log("Include group ref %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out group ref %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -312,7 +312,7 @@ class DefaultRefFilter {
 | 
			
		||||
          // the regular Git tree that users interact with, not on any of the Gerrit trees, so this
 | 
			
		||||
          // is a negligible risk.
 | 
			
		||||
          logger.atFinest().log("Include tag ref %s because user has read on refs/*", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          // If its a tag, consider it later.
 | 
			
		||||
          if (ref.getObjectId() != null) {
 | 
			
		||||
@@ -326,7 +326,7 @@ class DefaultRefFilter {
 | 
			
		||||
        // Sequences are internal database implementation details.
 | 
			
		||||
        if (viewMetadata) {
 | 
			
		||||
          logger.atFinest().log("Include sequence ref %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out sequence ref %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -336,7 +336,7 @@ class DefaultRefFilter {
 | 
			
		||||
        // users.
 | 
			
		||||
        if (viewMetadata) {
 | 
			
		||||
          logger.atFinest().log("Include external IDs branch %s", name);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        } else {
 | 
			
		||||
          logger.atFinest().log("Filter out external IDs branch %s", name);
 | 
			
		||||
        }
 | 
			
		||||
@@ -346,13 +346,13 @@ class DefaultRefFilter {
 | 
			
		||||
        // not symbolic then getLeaf() is a no-op returning ref itself.
 | 
			
		||||
        logger.atFinest().log(
 | 
			
		||||
            "Include ref %s because its leaf %s is readable", name, ref.getLeaf().getName());
 | 
			
		||||
        resultRefs.put(name, ref);
 | 
			
		||||
        resultRefs.add(ref);
 | 
			
		||||
      } else if (isRefsUsersSelf(ref)) {
 | 
			
		||||
        // viewMetadata allows to see all account refs, hence refs/users/self should be included as
 | 
			
		||||
        // well
 | 
			
		||||
        if (viewMetadata) {
 | 
			
		||||
          logger.atFinest().log("Include ref %s", REFS_USERS_SELF);
 | 
			
		||||
          resultRefs.put(name, ref);
 | 
			
		||||
          resultRefs.add(ref);
 | 
			
		||||
        }
 | 
			
		||||
      } else {
 | 
			
		||||
        logger.atFinest().log("Filter out ref %s", name);
 | 
			
		||||
@@ -370,38 +370,39 @@ class DefaultRefFilter {
 | 
			
		||||
   * <p>We exclude symbolic refs because their target will be included and this will suffice for
 | 
			
		||||
   * computing reachability.
 | 
			
		||||
   */
 | 
			
		||||
  private static Map<String, Ref> getTaggableRefsMap(Repository repo)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
  private static List<Ref> getTaggableRefs(Repository repo) throws PermissionBackendException {
 | 
			
		||||
    try {
 | 
			
		||||
      return repo.getRefDatabase().getRefs().stream()
 | 
			
		||||
      List<Ref> allRefs = repo.getRefDatabase().getRefs();
 | 
			
		||||
      return allRefs.stream()
 | 
			
		||||
          .filter(
 | 
			
		||||
              r ->
 | 
			
		||||
                  !RefNames.isGerritRef(r.getName())
 | 
			
		||||
                      && !r.getName().startsWith(RefNames.REFS_TAGS)
 | 
			
		||||
                      && !r.isSymbolic())
 | 
			
		||||
          .collect(toMap(Ref::getName, r -> r));
 | 
			
		||||
          // Don't use the default Java Collections.toList() as that is not size-aware and would
 | 
			
		||||
          // expand an array list as new elements are added. Instead, provide a list that has the
 | 
			
		||||
          // right size. This spares incremental list expansion which is quadratic in complexity.
 | 
			
		||||
          .collect(toCollection(() -> new ArrayList<>(allRefs.size())));
 | 
			
		||||
    } catch (IOException e) {
 | 
			
		||||
      throw new PermissionBackendException(e);
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private Map<String, Ref> fastHideRefsMetaConfig(Map<String, Ref> refs)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
    if (refs.containsKey(REFS_CONFIG) && !canReadRef(REFS_CONFIG)) {
 | 
			
		||||
      Map<String, Ref> r = new HashMap<>(refs);
 | 
			
		||||
      r.remove(REFS_CONFIG);
 | 
			
		||||
      return r;
 | 
			
		||||
  private List<Ref> fastHideRefsMetaConfig(List<Ref> refs) throws PermissionBackendException {
 | 
			
		||||
    if (!canReadRef(REFS_CONFIG)) {
 | 
			
		||||
      return refs.stream()
 | 
			
		||||
          .filter(r -> !r.getName().equals(REFS_CONFIG))
 | 
			
		||||
          // Don't use the default Java Collections.toList() as that is not size-aware and would
 | 
			
		||||
          // expand an array list as new elements are added. Instead, provide a list that has the
 | 
			
		||||
          // right size. This spares incremental list expansion which is quadratic in complexity.
 | 
			
		||||
          .collect(toCollection(() -> new ArrayList<>(refs.size())));
 | 
			
		||||
    }
 | 
			
		||||
    return refs;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private Map<String, Ref> addUsersSelfSymref(Repository repo, Map<String, Ref> refs)
 | 
			
		||||
  private List<Ref> addUsersSelfSymref(Repository repo, List<Ref> refs)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
    if (user.isIdentifiedUser()) {
 | 
			
		||||
      // User self symref is already there
 | 
			
		||||
      if (refs.containsKey(REFS_USERS_SELF)) {
 | 
			
		||||
        return refs;
 | 
			
		||||
      }
 | 
			
		||||
      String refName = RefNames.refsUsers(user.getAccountId());
 | 
			
		||||
      try {
 | 
			
		||||
        Ref r = repo.exactRef(refName);
 | 
			
		||||
@@ -411,8 +412,8 @@ class DefaultRefFilter {
 | 
			
		||||
        }
 | 
			
		||||
 | 
			
		||||
        SymbolicRef s = new SymbolicRef(REFS_USERS_SELF, r);
 | 
			
		||||
        refs = new HashMap<>(refs);
 | 
			
		||||
        refs.put(s.getName(), s);
 | 
			
		||||
        refs = new ArrayList<>(refs);
 | 
			
		||||
        refs.add(s);
 | 
			
		||||
        logger.atFinest().log("Added %s as alias for user ref %s", REFS_USERS_SELF, refName);
 | 
			
		||||
      } catch (IOException e) {
 | 
			
		||||
        throw new PermissionBackendException(e);
 | 
			
		||||
@@ -614,7 +615,7 @@ class DefaultRefFilter {
 | 
			
		||||
  @AutoValue
 | 
			
		||||
  abstract static class Result {
 | 
			
		||||
    /** Subset of the refs passed into the computation that is visible to the user. */
 | 
			
		||||
    abstract Map<String, Ref> visibleRefs();
 | 
			
		||||
    abstract List<Ref> visibleRefs();
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * List of tags where we couldn't figure out visibility in the first pass and need to do an
 | 
			
		||||
 
 | 
			
		||||
@@ -26,7 +26,6 @@ import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
 | 
			
		||||
import com.google.gerrit.server.permissions.PermissionBackend.WithUser;
 | 
			
		||||
import com.google.gerrit.server.query.change.ChangeData;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Set;
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
import org.eclipse.jgit.lib.Repository;
 | 
			
		||||
@@ -142,7 +141,7 @@ public class FailedPermissionBackend {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
    public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException {
 | 
			
		||||
      throw new PermissionBackendException(message, cause);
 | 
			
		||||
    }
 | 
			
		||||
 
 | 
			
		||||
@@ -15,7 +15,6 @@
 | 
			
		||||
package com.google.gerrit.server.permissions;
 | 
			
		||||
 | 
			
		||||
import static java.util.Objects.requireNonNull;
 | 
			
		||||
import static java.util.stream.Collectors.toMap;
 | 
			
		||||
import static java.util.stream.Collectors.toSet;
 | 
			
		||||
 | 
			
		||||
import com.google.auto.value.AutoValue;
 | 
			
		||||
@@ -41,7 +40,6 @@ import java.util.Collections;
 | 
			
		||||
import java.util.EnumSet;
 | 
			
		||||
import java.util.Iterator;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Set;
 | 
			
		||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
@@ -328,34 +326,19 @@ public abstract class PermissionBackend {
 | 
			
		||||
 | 
			
		||||
    public abstract BooleanCondition testCond(CoreOrPluginProjectPermission perm);
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Filter a map of references by visibility.
 | 
			
		||||
     *
 | 
			
		||||
     * @param refs a map of references to filter.
 | 
			
		||||
     * @param repo an open {@link Repository} handle for this instance's project
 | 
			
		||||
     * @param opts further options for filtering.
 | 
			
		||||
     * @return a partition of the provided refs that are visible to the user that this instance is
 | 
			
		||||
     *     scoped to.
 | 
			
		||||
     * @throws PermissionBackendException if failure consulting backend configuration.
 | 
			
		||||
     */
 | 
			
		||||
    public abstract Map<String, Ref> filter(
 | 
			
		||||
        Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException;
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Filter a list of references by visibility.
 | 
			
		||||
     *
 | 
			
		||||
     * @param refs a list of references to filter.
 | 
			
		||||
     * @param refs a collection of references to filter.
 | 
			
		||||
     * @param repo an open {@link Repository} handle for this instance's project
 | 
			
		||||
     * @param opts further options for filtering.
 | 
			
		||||
     * @return a partition of the provided refs that are visible to the user that this instance is
 | 
			
		||||
     *     scoped to.
 | 
			
		||||
     * @throws PermissionBackendException if failure consulting backend configuration.
 | 
			
		||||
     */
 | 
			
		||||
    public Map<String, Ref> filter(List<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException {
 | 
			
		||||
      return filter(refs.stream().collect(toMap(Ref::getName, r -> r, (a, b) -> b)), repo, opts);
 | 
			
		||||
    }
 | 
			
		||||
    public abstract Collection<Ref> filter(
 | 
			
		||||
        Collection<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /** Options for filtering refs using {@link ForProject}. */
 | 
			
		||||
 
 | 
			
		||||
@@ -404,7 +404,7 @@ class ProjectControl {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
    public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException {
 | 
			
		||||
      if (refFilter == null) {
 | 
			
		||||
        refFilter = refFilterFactory.create(ProjectControl.this);
 | 
			
		||||
 
 | 
			
		||||
@@ -26,8 +26,8 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import com.google.inject.Singleton;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
import org.eclipse.jgit.lib.Repository;
 | 
			
		||||
import org.eclipse.jgit.revwalk.RevCommit;
 | 
			
		||||
@@ -56,7 +56,7 @@ public class Reachable {
 | 
			
		||||
  public boolean fromRefs(
 | 
			
		||||
      Project.NameKey project, Repository repo, RevCommit commit, List<Ref> refs) {
 | 
			
		||||
    try (RevWalk rw = new RevWalk(repo)) {
 | 
			
		||||
      Map<String, Ref> filtered =
 | 
			
		||||
      Collection<Ref> filtered =
 | 
			
		||||
          permissionBackend
 | 
			
		||||
              .currentUser()
 | 
			
		||||
              .project(project)
 | 
			
		||||
@@ -68,7 +68,7 @@ public class Reachable {
 | 
			
		||||
          TraceContext.newTimer(
 | 
			
		||||
              "IncludedInResolver.includedInAny",
 | 
			
		||||
              Metadata.builder().projectName(project.get()).resourceCount(refs.size()).build())) {
 | 
			
		||||
        return IncludedInResolver.includedInAny(repo, rw, commit, filtered.values());
 | 
			
		||||
        return IncludedInResolver.includedInAny(repo, rw, commit, filtered);
 | 
			
		||||
      }
 | 
			
		||||
    } catch (IOException | PermissionBackendException e) {
 | 
			
		||||
      logger.atSevere().withCause(e).log(
 | 
			
		||||
 
 | 
			
		||||
@@ -41,8 +41,8 @@ import com.google.inject.Inject;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.sql.Timestamp;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
 | 
			
		||||
import org.eclipse.jgit.lib.Constants;
 | 
			
		||||
import org.eclipse.jgit.lib.PersonIdent;
 | 
			
		||||
@@ -127,10 +127,10 @@ public class ListTags implements RestReadView<ProjectResource> {
 | 
			
		||||
        permissionBackend.currentUser().project(resource.getNameKey());
 | 
			
		||||
    try (Repository repo = getRepository(resource.getNameKey());
 | 
			
		||||
        RevWalk rw = new RevWalk(repo)) {
 | 
			
		||||
      Map<String, Ref> all =
 | 
			
		||||
      Collection<Ref> all =
 | 
			
		||||
          visibleTags(
 | 
			
		||||
              resource.getNameKey(), repo, repo.getRefDatabase().getRefsByPrefix(Constants.R_TAGS));
 | 
			
		||||
      for (Ref ref : all.values()) {
 | 
			
		||||
      for (Ref ref : all) {
 | 
			
		||||
        tags.add(
 | 
			
		||||
            createTagInfo(perm.ref(ref.getName()), ref, rw, resource.getProjectState(), links));
 | 
			
		||||
      }
 | 
			
		||||
@@ -223,7 +223,7 @@ public class ListTags implements RestReadView<ProjectResource> {
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private Map<String, Ref> visibleTags(Project.NameKey project, Repository repo, List<Ref> tags)
 | 
			
		||||
  private Collection<Ref> visibleTags(Project.NameKey project, Repository repo, List<Ref> tags)
 | 
			
		||||
      throws PermissionBackendException {
 | 
			
		||||
    return permissionBackend
 | 
			
		||||
        .currentUser()
 | 
			
		||||
 
 | 
			
		||||
@@ -35,7 +35,7 @@ import com.google.gerrit.sshd.CommandMetaData;
 | 
			
		||||
import com.google.gerrit.sshd.SshCommand;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import org.eclipse.jgit.errors.ConfigInvalidException;
 | 
			
		||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
@@ -89,14 +89,14 @@ public class LsUserRefs extends SshCommand {
 | 
			
		||||
    try (Repository repo = repoManager.openRepository(projectName);
 | 
			
		||||
        ManualRequestContext ctx = requestContext.openAs(userAccountId)) {
 | 
			
		||||
      try {
 | 
			
		||||
        Map<String, Ref> refsMap =
 | 
			
		||||
        Collection<Ref> refsMap =
 | 
			
		||||
            permissionBackend
 | 
			
		||||
                .user(ctx.getUser())
 | 
			
		||||
                .project(projectName)
 | 
			
		||||
                .filter(repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults());
 | 
			
		||||
 | 
			
		||||
        for (String ref : refsMap.keySet()) {
 | 
			
		||||
          if (!onlyRefsHeads || ref.startsWith(RefNames.REFS_HEADS)) {
 | 
			
		||||
        for (Ref ref : refsMap) {
 | 
			
		||||
          if (!onlyRefsHeads || ref.getName().startsWith(RefNames.REFS_HEADS)) {
 | 
			
		||||
            stdout.println(ref);
 | 
			
		||||
          }
 | 
			
		||||
        }
 | 
			
		||||
 
 | 
			
		||||
@@ -1478,11 +1478,9 @@ public class AccountIT extends AbstractDaemonTest {
 | 
			
		||||
  public void refsUsersSelfIsAdvertised() throws Exception {
 | 
			
		||||
    try (Repository allUsersRepo = repoManager.openRepository(allUsers)) {
 | 
			
		||||
      assertThat(
 | 
			
		||||
              permissionBackend
 | 
			
		||||
                  .currentUser()
 | 
			
		||||
                  .project(allUsers)
 | 
			
		||||
                  .filter(ImmutableList.of(), allUsersRepo, RefFilterOptions.defaults())
 | 
			
		||||
                  .keySet())
 | 
			
		||||
              permissionBackend.currentUser().project(allUsers)
 | 
			
		||||
                  .filter(ImmutableList.of(), allUsersRepo, RefFilterOptions.defaults()).stream()
 | 
			
		||||
                  .map(Ref::getName))
 | 
			
		||||
          .containsExactly(RefNames.REFS_USERS_SELF);
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -14,6 +14,7 @@
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.acceptance.git;
 | 
			
		||||
 | 
			
		||||
import static com.google.common.collect.ImmutableList.toImmutableList;
 | 
			
		||||
import static com.google.common.truth.Truth.assertThat;
 | 
			
		||||
import static com.google.common.truth.Truth.assertWithMessage;
 | 
			
		||||
import static com.google.common.truth.TruthJUnit.assume;
 | 
			
		||||
@@ -24,10 +25,8 @@ import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.d
 | 
			
		||||
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
 | 
			
		||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 | 
			
		||||
import static java.util.stream.Collectors.toList;
 | 
			
		||||
import static java.util.stream.Collectors.toMap;
 | 
			
		||||
 | 
			
		||||
import com.google.common.collect.ImmutableList;
 | 
			
		||||
import com.google.common.collect.ImmutableMap;
 | 
			
		||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
 | 
			
		||||
import com.google.gerrit.acceptance.NoHttpd;
 | 
			
		||||
import com.google.gerrit.acceptance.PushOneCommit;
 | 
			
		||||
@@ -59,12 +58,10 @@ import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
 | 
			
		||||
import com.google.gerrit.server.query.change.ChangeData;
 | 
			
		||||
import com.google.gerrit.testing.ConfigSuite;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.Arrays;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.function.Function;
 | 
			
		||||
import java.util.function.Predicate;
 | 
			
		||||
import org.eclipse.jgit.api.Git;
 | 
			
		||||
import org.eclipse.jgit.junit.TestRepository;
 | 
			
		||||
@@ -1365,15 +1362,18 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 | 
			
		||||
    expectedAllRefs.addAll(expectedMetaRefs);
 | 
			
		||||
 | 
			
		||||
    try (Repository repo = repoManager.openRepository(allUsers)) {
 | 
			
		||||
      Map<String, Ref> all = getAllRefs(repo);
 | 
			
		||||
 | 
			
		||||
      PermissionBackend.ForProject forProject = newFilter(allUsers, admin);
 | 
			
		||||
      assertThat(forProject.filter(all, repo, RefFilterOptions.defaults()).keySet())
 | 
			
		||||
      assertThat(
 | 
			
		||||
              names(
 | 
			
		||||
                  forProject.filter(
 | 
			
		||||
                      repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults())))
 | 
			
		||||
          .containsExactlyElementsIn(expectedAllRefs);
 | 
			
		||||
      assertThat(
 | 
			
		||||
              forProject
 | 
			
		||||
                  .filter(all, repo, RefFilterOptions.builder().setFilterMeta(true).build())
 | 
			
		||||
                  .keySet())
 | 
			
		||||
              names(
 | 
			
		||||
                  forProject.filter(
 | 
			
		||||
                      repo.getRefDatabase().getRefs(),
 | 
			
		||||
                      repo,
 | 
			
		||||
                      RefFilterOptions.builder().setFilterMeta(true).build())))
 | 
			
		||||
          .containsExactlyElementsIn(expectedNonMetaRefs);
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
@@ -1384,8 +1384,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 | 
			
		||||
    String patchSetRef = change.getPatchSetId().toRefName();
 | 
			
		||||
    try (AutoCloseable ignored = disableChangeIndex();
 | 
			
		||||
        Repository repo = repoManager.openRepository(project)) {
 | 
			
		||||
      Map<String, Ref> singleRef = ImmutableMap.of(patchSetRef, repo.exactRef(patchSetRef));
 | 
			
		||||
      Map<String, Ref> filteredRefs =
 | 
			
		||||
      Collection<Ref> singleRef = ImmutableList.of(repo.exactRef(patchSetRef));
 | 
			
		||||
      Collection<Ref> filteredRefs =
 | 
			
		||||
          permissionBackend
 | 
			
		||||
              .user(user(admin))
 | 
			
		||||
              .project(project)
 | 
			
		||||
@@ -1482,8 +1482,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 | 
			
		||||
    return AccountGroup.uuid(gApi.groups().create(groupInput).get().id);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private static Map<String, Ref> getAllRefs(Repository repo) throws IOException {
 | 
			
		||||
    return repo.getRefDatabase().getRefs().stream()
 | 
			
		||||
        .collect(toMap(Ref::getName, Function.identity()));
 | 
			
		||||
  private static Collection<String> names(Collection<Ref> refs) {
 | 
			
		||||
    return refs.stream().map(Ref::getName).collect(toImmutableList());
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -36,7 +36,6 @@ import com.google.gerrit.server.permissions.PermissionBackendCondition;
 | 
			
		||||
import com.google.gerrit.server.permissions.PermissionBackendException;
 | 
			
		||||
import com.google.gerrit.server.permissions.ProjectPermission;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Set;
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
import org.eclipse.jgit.lib.Repository;
 | 
			
		||||
@@ -85,7 +84,7 @@ public class UiActionsTest {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
    public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
 | 
			
		||||
        throws PermissionBackendException {
 | 
			
		||||
      throw new UnsupportedOperationException("not implemented");
 | 
			
		||||
    }
 | 
			
		||||
 
 | 
			
		||||
 Submodule plugins/gitiles updated: 0f8de56cc0...3531010e04
									
								
							 Submodule plugins/replication updated: 4689b419ea...ef5ffc7508
									
								
							
		Reference in New Issue
	
	Block a user