Merge changes from topic "get-all-refs"
* changes:
DefaultAdvertiseRefsHook: Avoid Repository#getAllRefs
DefaultRefFilter: Avoid Repository#getAllRefs
CommitsCollection: Avoid Repository#getAllRefs
AbstractDaemonTest: Avoid Repository#getAllRefs
Group checker: Avoid Repository#getAllRefs
ProjectResetter: Avoid Repository#getAllRefs
ChangeUtil: Remove javadoc references to deprecated JGit methods
* submodules:
* Update plugins/replication from branch 'master'
to 0268da0f5f275eef613014548f60771d891f761d
- PushOne: Avoid deprecated Repository#getAllRefs method
The replacement is a stream expression. I think this implementation
still smells, and there must be a better way to do it with a list-based
approach, especially now that ForProject#filter has a variant that takes
a List. But this fix is quick and easy to reason about.
Change-Id: Idd783eed0b0e2ac98f93790af62b5ae288629fcb
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 82ff535..699b2f9 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
@@ -1233,7 +1234,13 @@
throws Exception {
TestRepository<?> localRepo = cloneProject(proj);
GitUtil.fetch(localRepo, "refs/*:refs/*");
- Map<String, Ref> refs = localRepo.getRepository().getAllRefs();
+ ImmutableMap<String, Ref> refs =
+ localRepo
+ .getRepository()
+ .getRefDatabase()
+ .getRefs()
+ .stream()
+ .collect(toImmutableMap(Ref::getName, r -> r));
Map<Branch.NameKey, RevTree> refValues = new HashMap<>();
for (Branch.NameKey b : trees.keySet()) {
diff --git a/java/com/google/gerrit/acceptance/ProjectResetter.java b/java/com/google/gerrit/acceptance/ProjectResetter.java
index 76ae4b0..7d0a59c 100644
--- a/java/com/google/gerrit/acceptance/ProjectResetter.java
+++ b/java/com/google/gerrit/acceptance/ProjectResetter.java
@@ -46,6 +46,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
@@ -259,8 +260,8 @@
refsPatternByProject.asMap().entrySet()) {
try (Repository repo = repoManager.openRepository(e.getKey())) {
Collection<Ref> nonRestoredRefs =
- repo.getAllRefs()
- .values()
+ repo.getRefDatabase()
+ .getRefs()
.stream()
.filter(
r ->
@@ -324,21 +325,20 @@
/** Evict accounts that were modified. */
private void evictAndReindexAccounts() throws IOException {
- Set<Account.Id> deletedAccounts = accountIds(deletedRefsByProject.get(allUsersName));
+ Set<Account.Id> deletedAccounts = accountIds(deletedRefsByProject.get(allUsersName).stream());
if (accountCreator != null) {
accountCreator.evict(deletedAccounts);
}
if (accountCache != null || accountIndexer != null) {
Set<Account.Id> modifiedAccounts =
- new HashSet<>(accountIds(restoredRefsByProject.get(allUsersName)));
+ new HashSet<>(accountIds(restoredRefsByProject.get(allUsersName).stream()));
if (restoredRefsByProject.get(allUsersName).contains(RefNames.REFS_EXTERNAL_IDS)
|| deletedRefsByProject.get(allUsersName).contains(RefNames.REFS_EXTERNAL_IDS)) {
// The external IDs have been modified but we don't know which accounts were affected.
// Make sure all accounts are evicted and reindexed.
try (Repository repo = repoManager.openRepository(allUsersName)) {
- for (Account.Id id :
- accountIds(repo.getAllRefs().values().stream().map(Ref::getName).collect(toSet()))) {
+ for (Account.Id id : accountIds(repo)) {
evictAndReindexAccount(id);
}
}
@@ -397,9 +397,12 @@
}
}
- private Set<Account.Id> accountIds(Collection<String> refs) {
- return refs.stream()
- .filter(r -> r.startsWith(REFS_USERS))
+ private static Set<Account.Id> accountIds(Repository repo) throws IOException {
+ return accountIds(repo.getRefDatabase().getRefsByPrefix(REFS_USERS).stream().map(Ref::getName));
+ }
+
+ private static Set<Account.Id> accountIds(Stream<String> refs) {
+ return refs.filter(r -> r.startsWith(REFS_USERS))
.map(Account.Id::fromRef)
.filter(Objects::nonNull)
.collect(toSet());
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index 571f322..a13f105 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -50,8 +50,7 @@
/**
* Get the next patch set ID from a previously-read map of all refs.
*
- * @param allRefs map of full ref name to ref, in the same format returned by {@link
- * org.eclipse.jgit.lib.RefDatabase#getRefs(String)} when passing {@code ""}.
+ * @param allRefs map of full ref name to ref.
* @param id previous patch set ID.
* @return next unused patch set ID for the same change, skipping any IDs whose corresponding ref
* names appear in the {@code allRefs} map.
@@ -69,9 +68,7 @@
*
* @param changeRefs map of ref suffix to SHA-1, where the keys are ref names with the {@code
* refs/changes/CD/ABCD/} prefix stripped. All refs should be under {@code id}'s change ref
- * prefix. The keys match the format returned by {@link
- * org.eclipse.jgit.lib.RefDatabase#getRefs(String)} when passing the appropriate {@code
- * refs/changes/CD/ABCD}.
+ * prefix.
* @param id previous patch set ID.
* @return next unused patch set ID for the same change, skipping any IDs whose corresponding ref
* names appear in the {@code changeRefs} map.
diff --git a/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
index f255ea2..a8594e7 100644
--- a/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
@@ -14,13 +14,14 @@
package com.google.gerrit.server.git;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import java.io.IOException;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
@@ -44,19 +45,10 @@
protected Map<String, Ref> getAdvertisedRefs(Repository repo, RevWalk revWalk)
throws ServiceMayNotContinueException {
try {
- Map<String, Ref> refs;
- List<String> prefixes = opts.prefixes();
- if (prefixes.isEmpty() || prefixes.get(0).isEmpty()) {
- refs = repo.getAllRefs();
- } else {
- refs = new HashMap<>();
- for (String prefix : prefixes) {
- for (Ref ref : repo.getRefDatabase().getRefsByPrefix(prefix)) {
- refs.put(ref.getName(), ref);
- }
- }
- }
- return perm.filter(refs, repo, opts);
+ List<String> prefixes =
+ !opts.prefixes().isEmpty() ? opts.prefixes() : ImmutableList.of(RefDatabase.ALL);
+ return perm.filter(
+ repo.getRefDatabase().getRefsByPrefix(prefixes.toArray(new String[0])), repo, opts);
} catch (IOException | PermissionBackendException e) {
ServiceMayNotContinueException ex = new ServiceMayNotContinueException();
ex.initCause(e);
diff --git a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
index 58b55ee..c3ca60b 100644
--- a/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
+++ b/java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
@@ -87,8 +87,12 @@
BiMap<AccountGroup.UUID, String> uuidNameBiMap = HashBiMap.create();
- // Get all refs in an attempt to avoid seeing half committed group updates.
- Map<String, Ref> refs = allUsersRepo.getAllRefs();
+ // Get group refs and group names ref using the most atomic API available, in an attempt to
+ // avoid seeing half-committed group updates.
+ List<Ref> refs =
+ allUsersRepo
+ .getRefDatabase()
+ .getRefsByPrefix(RefNames.REFS_GROUPS, RefNames.REFS_GROUPNAMES);
readGroups(allUsersRepo, refs, result);
readGroupNames(allUsersRepo, refs, result, uuidNameBiMap);
// The sequential IDs are not keys in NoteDb, so no need to check them.
@@ -103,22 +107,21 @@
return result;
}
- private void readGroups(Repository allUsersRepo, Map<String, Ref> refs, Result result)
+ private void readGroups(Repository allUsersRepo, List<Ref> refs, Result result)
throws IOException {
- for (Map.Entry<String, Ref> entry : refs.entrySet()) {
- if (!entry.getKey().startsWith(RefNames.REFS_GROUPS)) {
+ for (Ref ref : refs) {
+ if (!ref.getName().startsWith(RefNames.REFS_GROUPS)) {
continue;
}
- AccountGroup.UUID uuid = AccountGroup.UUID.fromRef(entry.getKey());
+ AccountGroup.UUID uuid = AccountGroup.UUID.fromRef(ref.getName());
if (uuid == null) {
- result.problems.add(error("null UUID from %s", entry.getKey()));
+ result.problems.add(error("null UUID from %s", ref.getName()));
continue;
}
try {
GroupConfig cfg =
- GroupConfig.loadForGroupSnapshot(
- allUsersName, allUsersRepo, uuid, entry.getValue().getObjectId());
+ GroupConfig.loadForGroupSnapshot(allUsersName, allUsersRepo, uuid, ref.getObjectId());
result.uuidToGroupMap.put(uuid, cfg.getLoadedGroup().get());
} catch (ConfigInvalidException e) {
result.problems.add(error("group %s does not parse: %s", uuid, e.getMessage()));
@@ -128,16 +131,18 @@
private void readGroupNames(
Repository repo,
- Map<String, Ref> refs,
+ List<Ref> refs,
Result result,
BiMap<AccountGroup.UUID, String> uuidNameBiMap)
throws IOException {
- Ref ref = refs.get(RefNames.REFS_GROUPNAMES);
- if (ref == null) {
+ Optional<Ref> maybeRef =
+ refs.stream().filter(r -> r.getName().equals(RefNames.REFS_GROUPNAMES)).findFirst();
+ if (!maybeRef.isPresent()) {
String msg = String.format("ref %s does not exist", RefNames.REFS_GROUPNAMES);
result.problems.add(error(msg));
return;
}
+ Ref ref = maybeRef.get();
try (RevWalk rw = new RevWalk(repo)) {
RevCommit c = rw.parseCommit(ref.getObjectId());
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 75652d1..d36cbd6 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_USERS_SELF;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
@@ -248,7 +249,7 @@
repo,
opts.filterTagsSeparately()
? filter(
- repo.getAllRefs(),
+ getAllRefsMap(repo),
repo,
opts.toBuilder().setFilterTagsSeparately(false).build())
.values()
@@ -263,6 +264,14 @@
return result;
}
+ private static Map<String, Ref> getAllRefsMap(Repository repo) throws PermissionBackendException {
+ try {
+ return repo.getRefDatabase().getRefs().stream().collect(toMap(Ref::getName, r -> r));
+ } 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)) {
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index e6fe3a5..f0f7f42 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -15,6 +15,7 @@
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;
@@ -334,6 +335,21 @@
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 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)), repo, opts);
+ }
}
/** Options for filtering refs using {@link ForProject}. */
diff --git a/java/com/google/gerrit/server/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java
index 63fda19..76ee8c9 100644
--- a/java/com/google/gerrit/server/project/Reachable.java
+++ b/java/com/google/gerrit/server/project/Reachable.java
@@ -14,10 +14,9 @@
package com.google.gerrit.server.project;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
@@ -25,11 +24,10 @@
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.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;
@@ -50,8 +48,7 @@
}
/** @return true if a commit is reachable from a given set of refs. */
- public boolean fromRefs(
- Project.NameKey project, Repository repo, RevCommit commit, Map<String, Ref> refs) {
+ public boolean fromRefs(NameKey project, Repository repo, RevCommit commit, List<Ref> refs) {
try (RevWalk rw = new RevWalk(repo)) {
Map<String, Ref> filtered =
permissionBackend
@@ -69,13 +66,7 @@
/** @return true if a commit is reachable from a repo's branches and tags. */
boolean fromHeadsOrTags(Project.NameKey project, Repository repo, RevCommit commit) {
try {
- RefDatabase refdb = repo.getRefDatabase();
- Collection<Ref> heads = refdb.getRefsByPrefix(Constants.R_HEADS);
- Collection<Ref> tags = refdb.getRefsByPrefix(Constants.R_TAGS);
- Map<String, Ref> refs = Maps.newHashMapWithExpectedSize(heads.size() + tags.size());
- for (Ref r : Iterables.concat(heads, tags)) {
- refs.put(r.getName(), r);
- }
+ List<Ref> refs = repo.getRefDatabase().getRefsByPrefix(Constants.R_HEADS, Constants.R_TAGS);
return fromRefs(project, repo, commit, refs);
} catch (IOException e) {
logger.atSevere().withCause(e).log(
diff --git a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
index 15cd824..63dcf5c 100644
--- a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
@@ -105,7 +105,7 @@
}
/** @return true if {@code commit} is visible to the caller. */
- public boolean canRead(ProjectState state, Repository repo, RevCommit commit) {
+ public boolean canRead(ProjectState state, Repository repo, RevCommit commit) throws IOException {
Project.NameKey project = state.getNameKey();
// Look for changes associated with the commit.
@@ -122,6 +122,6 @@
}
}
- return reachable.fromRefs(project, repo, commit, repo.getAllRefs());
+ return reachable.fromRefs(project, repo, commit, repo.getRefDatabase().getRefs());
}
}
diff --git a/plugins/replication b/plugins/replication
index 4fad0c8..0268da0 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4fad0c870d80daf274d6aa542a2be554cf4a1044
+Subproject commit 0268da0f5f275eef613014548f60771d891f761d
