AccountResolver: Remove callerMayAssumeCandidatesAreVisible() method
Readers are often confused about what this method means. It means that
the searcher only returns results that are visible to the user, and
hence the caller doesn't need to do another visibility check.
Nowadays only the BySelf searcher makes use of this. Since users can
always see themselves, the visibility check on the result can be skipped
for the BySelf searcher.
Simplify this by removing the callerMayAssumeCandidatesAreVisible()
method and instead hard-code the exception for the BySelf search when
checking the visibility of the results. This should make the code easier
to read.
FWIW before change I90c2cd624 also the ByFullName and ByDefault searches
made use of skipping the visibility check by returning true from the
callerMayAssumeCandidatesAreVisible() method. This was because they
already checked the visibility during search by calling
enforceVisibility(true) on the InternalAccountQuery and hence the caller
didn't need to check the visibility again.
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I9b9967ae71dc75783a6b60510cff0950eeb7f97e
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 827c8f7..703693d 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -230,10 +230,6 @@
return true;
}
- default boolean callerMayAssumeCandidatesAreVisible() {
- return false;
- }
-
/**
* Searches can be done on behalf of either the current user or another provided user. The
* results of some searchers, such as BySelf, are affected by the context user.
@@ -312,11 +308,6 @@
}
@Override
- public boolean callerMayAssumeCandidatesAreVisible() {
- return true;
- }
-
- @Override
public boolean requiresContextUser() {
return true;
}
@@ -783,9 +774,9 @@
}
Stream<AccountState> results = maybeResults.get();
- if (!searcher.callerMayAssumeCandidatesAreVisible()) {
- results = results.filter(visibilitySupplier.get());
- }
+ // Filter out non-visible results, except if it's the BySelf searcher. Since users can always
+ // see themselves checking the visibility is not needed for the BySelf searcher.
+ results = searcher instanceof BySelf ? results : results.filter(visibilitySupplier.get());
List<AccountState> list;
if (searcher.callerShouldFilterOutInactiveCandidates()) {
diff --git a/javatests/com/google/gerrit/server/account/AccountResolverTest.java b/javatests/com/google/gerrit/server/account/AccountResolverTest.java
index 37728f7..34f746a 100644
--- a/javatests/com/google/gerrit/server/account/AccountResolverTest.java
+++ b/javatests/com/google/gerrit/server/account/AccountResolverTest.java
@@ -43,7 +43,6 @@
private final String pattern;
private final boolean shortCircuit;
private final ImmutableList<AccountState> accounts;
- private boolean assumeVisible;
private boolean filterInactive;
private TestSearcher(String pattern, boolean shortCircuit, AccountState... accounts) {
@@ -68,15 +67,6 @@
}
@Override
- public boolean callerMayAssumeCandidatesAreVisible() {
- return assumeVisible;
- }
-
- void setCallerMayAssumeCandidatesAreVisible() {
- this.assumeVisible = true;
- }
-
- @Override
public boolean callerShouldFilterOutInactiveCandidates() {
return filterInactive;
}
@@ -139,17 +129,6 @@
}
@Test
- public void skipVisibilityCheck() throws Exception {
- TestSearcher searcher = new TestSearcher("foo", false, newAccount(1), newAccount(2));
- ImmutableList<Searcher<?>> searchers = ImmutableList.of(searcher);
-
- assertThat(search("foo", searchers, only(2)).asIdSet()).containsExactlyElementsIn(ids(2));
-
- searcher.setCallerMayAssumeCandidatesAreVisible();
- assertThat(search("foo", searchers, only(2)).asIdSet()).containsExactlyElementsIn(ids(1, 2));
- }
-
- @Test
public void dontFilterInactive() throws Exception {
ImmutableList<Searcher<?>> searchers =
ImmutableList.of(
