Merge "Fix project-watchers visibility checks."
diff --git a/java/com/google/gerrit/server/account/AccountControl.java b/java/com/google/gerrit/server/account/AccountControl.java
index c80059b..ca63565 100644
--- a/java/com/google/gerrit/server/account/AccountControl.java
+++ b/java/com/google/gerrit/server/account/AccountControl.java
@@ -82,12 +82,12 @@
* accounts.
*/
@UsedAt(UsedAt.Project.PLUGIN_CODE_OWNERS)
- public AccountControl get(IdentifiedUser identifiedUser) {
+ public AccountControl get(CurrentUser user) {
return new AccountControl(
permissionBackend,
projectCache,
groupControlFactory,
- identifiedUser,
+ user,
userFactory,
accountVisibility);
}
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 65eb332..389b292 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -132,12 +132,18 @@
private final String input;
private final ImmutableList<AccountState> list;
private final ImmutableList<AccountState> filteredInactive;
+ private final CurrentUser searchedAsUser;
@VisibleForTesting
- Result(String input, List<AccountState> list, List<AccountState> filteredInactive) {
+ Result(
+ String input,
+ List<AccountState> list,
+ List<AccountState> filteredInactive,
+ CurrentUser searchedAsUser) {
this.input = requireNonNull(input);
this.list = canonicalize(list);
this.filteredInactive = canonicalize(filteredInactive);
+ this.searchedAsUser = requireNonNull(searchedAsUser);
}
private ImmutableList<AccountState> canonicalize(List<AccountState> list) {
@@ -180,13 +186,21 @@
}
}
- public IdentifiedUser asUniqueUser() throws UnresolvableAccountException {
+ private void ensureSelfIsUniqueIdentifiedUser() throws UnresolvableAccountException {
ensureUnique();
+ if (!searchedAsUser.isIdentifiedUser()) {
+ throw new UnresolvableAccountException(this);
+ }
+ }
+
+ public IdentifiedUser asUniqueUser() throws UnresolvableAccountException {
if (isSelf()) {
+ ensureSelfIsUniqueIdentifiedUser();
// In the special case of "self", use the exact IdentifiedUser from the request context, to
// preserve the peer address and any other per-request state.
- return self.get().asIdentifiedUser();
+ return searchedAsUser.asIdentifiedUser();
}
+ ensureUnique();
return userFactory.create(asUnique());
}
@@ -194,8 +208,7 @@
throws UnresolvableAccountException {
ensureUnique();
if (isSelf()) {
- // TODO(dborowitz): This preserves old behavior, but it seems wrong to discard the caller.
- return self.get().asIdentifiedUser();
+ return searchedAsUser.asIdentifiedUser();
}
return userFactory.runAs(
null, list.get(0).account().id(), requireNonNull(caller).getRealUser());
@@ -221,16 +234,57 @@
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.
+ */
+ default boolean requiresContextUser() {
+ return false;
+ }
+
Optional<I> tryParse(String input) throws IOException;
- Stream<AccountState> search(I input) throws IOException, ConfigInvalidException;
+ /**
+ * This method should be implemented for every searcher which doesn't require a context user.
+ *
+ * @param input to search for
+ * @return stream of the matching accounts
+ * @throws IOException by some subclasses
+ * @throws ConfigInvalidException by some subclasses
+ */
+ default Stream<AccountState> search(I input) throws IOException, ConfigInvalidException {
+ throw new IllegalStateException("search(I) default implementation should never be called.");
+ }
+
+ /**
+ * This method should be implemented for every searcher which requires a context user.
+ *
+ * @param input to search for
+ * @param asUser the context user for the search
+ * @return stream of the matching accounts
+ * @throws IOException by some subclasses
+ * @throws ConfigInvalidException by some subclasses
+ */
+ default Stream<AccountState> search(I input, CurrentUser asUser)
+ throws IOException, ConfigInvalidException {
+ if (!requiresContextUser()) {
+ return search(input);
+ }
+ throw new IllegalStateException(
+ "The searcher requires a context user, but doesn't implement search(input, asUser).");
+ }
boolean shortCircuitIfNoResults();
- default Optional<Stream<AccountState>> trySearch(String input)
+ default Optional<Stream<AccountState>> trySearch(String input, CurrentUser asUser)
throws IOException, ConfigInvalidException {
Optional<I> parsed = tryParse(input);
- return parsed.isPresent() ? Optional.of(search(parsed.get())) : Optional.empty();
+ if (parsed.isEmpty()) {
+ return Optional.empty();
+ }
+ return requiresContextUser()
+ ? Optional.of(search(parsed.get(), asUser))
+ : Optional.of(search(parsed.get()));
}
}
@@ -251,7 +305,7 @@
}
}
- private class BySelf extends StringSearcher {
+ private static class BySelf extends StringSearcher {
@Override
public boolean callerShouldFilterOutInactiveCandidates() {
return false;
@@ -263,17 +317,21 @@
}
@Override
+ public boolean requiresContextUser() {
+ return true;
+ }
+
+ @Override
protected boolean matches(String input) {
return "self".equals(input) || "me".equals(input);
}
@Override
- public Stream<AccountState> search(String input) {
- CurrentUser user = self.get();
- if (!user.isIdentifiedUser()) {
+ public Stream<AccountState> search(String input, CurrentUser asUser) {
+ if (!asUser.isIdentifiedUser()) {
return Stream.empty();
}
- return Stream.of(user.asIdentifiedUser().state());
+ return Stream.of(asUser.asIdentifiedUser().state());
}
@Override
@@ -400,9 +458,20 @@
}
private class ByFullName implements Searcher<AccountState> {
+ boolean allowSkippingVisibilityCheck = true;
+
+ ByFullName() {
+ super();
+ }
+
+ ByFullName(boolean allowSkippingVisibilityCheck) {
+ this();
+ this.allowSkippingVisibilityCheck = allowSkippingVisibilityCheck;
+ }
+
@Override
public boolean callerMayAssumeCandidatesAreVisible() {
- return true; // Rely on enforceVisibility from the index.
+ return allowSkippingVisibilityCheck;
}
@Override
@@ -424,9 +493,25 @@
}
private class ByDefaultSearch extends StringSearcher {
+ boolean allowSkippingVisibilityCheck = true;
+
+ ByDefaultSearch() {
+ super();
+ }
+
+ ByDefaultSearch(boolean allowSkippingVisibilityCheck) {
+ this();
+ this.allowSkippingVisibilityCheck = allowSkippingVisibilityCheck;
+ }
+
@Override
public boolean callerMayAssumeCandidatesAreVisible() {
- return true; // Rely on enforceVisibility from the index.
+ return allowSkippingVisibilityCheck;
+ }
+
+ @Override
+ public boolean requiresContextUser() {
+ return true;
}
@Override
@@ -435,14 +520,14 @@
}
@Override
- public Stream<AccountState> search(String input) {
+ public Stream<AccountState> search(String input, CurrentUser asUser) {
// At this point we have no clue. Just perform a whole bunch of suggestions and pray we come
// up with a reasonable result list.
// TODO(dborowitz): This doesn't match the documentation; consider whether it's possible to be
// more strict here.
boolean canSeeSecondaryEmails = false;
try {
- if (permissionBackend.user(self.get()).test(GlobalPermission.MODIFY_ACCOUNT)) {
+ if (permissionBackend.user(asUser).test(GlobalPermission.MODIFY_ACCOUNT)) {
canSeeSecondaryEmails = true;
}
} catch (PermissionBackendException e) {
@@ -477,6 +562,18 @@
.addAll(nameOrEmailSearchers)
.build();
+ private final ImmutableList<Searcher<?>> forcedVisibilitySearchers =
+ ImmutableList.of(
+ new ByNameAndEmail(),
+ new ByEmail(),
+ new FromRealm(),
+ new ByFullName(false),
+ new ByDefaultSearch(false),
+ new BySelf(),
+ new ByExactAccountId(),
+ new ByParenthesizedAccountId(),
+ new ByUsername());
+
private final AccountCache accountCache;
private final AccountControl.Factory accountControlFactory;
private final Emails emails;
@@ -538,12 +635,63 @@
* @throws IOException if an error occurs.
*/
public Result resolve(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::canSeePredicate, AccountResolver::isActive);
+ return searchImpl(
+ input, searchers, self.get(), this::currentUserCanSeePredicate, AccountResolver::isActive);
}
public Result resolve(String input, Predicate<AccountState> accountActivityPredicate)
throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::canSeePredicate, accountActivityPredicate);
+ return searchImpl(
+ input, searchers, self.get(), this::currentUserCanSeePredicate, accountActivityPredicate);
+ }
+
+ /**
+ * Resolves all accounts matching the input string, visible to the provided user.
+ *
+ * <p>The following input formats are recognized:
+ *
+ * <ul>
+ * <li>The strings {@code "self"} and {@code "me"}, if the provided user is an {@link
+ * IdentifiedUser}. In this case, may return exactly one inactive account.
+ * <li>A bare account ID ({@code "18419"}). In this case, may return exactly one inactive
+ * account. This case short-circuits if the input matches.
+ * <li>An account ID in parentheses following a full name ({@code "Full Name (18419)"}). This
+ * case short-circuits if the input matches.
+ * <li>A username ({@code "username"}).
+ * <li>A full name and email address ({@code "Full Name <email@example>"}). This case
+ * short-circuits if the input matches.
+ * <li>An email address ({@code "email@example"}. This case short-circuits if the input matches.
+ * <li>An account name recognized by the configured {@link Realm#lookup(String)} Realm}.
+ * <li>A full name ({@code "Full Name"}).
+ * <li>As a fallback, a {@link
+ * com.google.gerrit.server.query.account.AccountPredicates#defaultPredicate(Schema,
+ * boolean, String) default search} against the account index.
+ * </ul>
+ *
+ * @param asUser user to resolve the users by.
+ * @param input input string.
+ * @param forceVisibilityCheck whether to force all searchers to check for visibility.
+ * @return a result describing matching accounts. Never null even if the result set is empty.
+ * @throws ConfigInvalidException if an error occurs.
+ * @throws IOException if an error occurs.
+ */
+ public Result resolveAsUser(CurrentUser asUser, String input, boolean forceVisibilityCheck)
+ throws ConfigInvalidException, IOException {
+ return resolveAsUser(asUser, input, AccountResolver::isActive, forceVisibilityCheck);
+ }
+
+ public Result resolveAsUser(
+ CurrentUser asUser,
+ String input,
+ Predicate<AccountState> accountActivityPredicate,
+ boolean forceVisibilityCheck)
+ throws ConfigInvalidException, IOException {
+ return searchImpl(
+ input,
+ forceVisibilityCheck ? forcedVisibilitySearchers : searchers,
+ asUser,
+ new ProvidedUserCanSeePredicate(asUser),
+ accountActivityPredicate);
}
/**
@@ -556,22 +704,23 @@
* instead will be stored as a link to the corresponding Gerrit Account.
*/
public Result resolveIncludeInactive(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::canSeePredicate, AccountResolver::allVisible);
+ return searchImpl(
+ input,
+ searchers,
+ self.get(),
+ this::currentUserCanSeePredicate,
+ AccountResolver::allVisible);
}
public Result resolveIncludeInactiveIgnoreVisibility(String input)
throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::allVisiblePredicate, AccountResolver::allVisible);
+ return searchImpl(
+ input, searchers, self.get(), this::allVisiblePredicate, AccountResolver::allVisible);
}
public Result resolveIgnoreVisibility(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::allVisiblePredicate, AccountResolver::isActive);
- }
-
- public Result resolveIgnoreVisibility(
- String input, Predicate<AccountState> accountActivityPredicate)
- throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, this::allVisiblePredicate, accountActivityPredicate);
+ return searchImpl(
+ input, searchers, self.get(), this::allVisiblePredicate, AccountResolver::isActive);
}
/**
@@ -600,7 +749,11 @@
@Deprecated
public Result resolveByNameOrEmail(String input) throws ConfigInvalidException, IOException {
return searchImpl(
- input, nameOrEmailSearchers, this::canSeePredicate, AccountResolver::isActive);
+ input,
+ nameOrEmailSearchers,
+ self.get(),
+ this::currentUserCanSeePredicate,
+ AccountResolver::isActive);
}
/**
@@ -619,16 +772,26 @@
return searchImpl(
input,
ImmutableList.of(new ByNameAndEmail(), new ByEmail(), new ByFullName(), new ByUsername()),
- this::canSeePredicate,
+ self.get(),
+ this::currentUserCanSeePredicate,
AccountResolver::isActive);
}
- private Predicate<AccountState> canSeePredicate() {
- return this::canSee;
+ private Predicate<AccountState> currentUserCanSeePredicate() {
+ return accountControlFactory.get()::canSee;
}
- private boolean canSee(AccountState accountState) {
- return accountControlFactory.get().canSee(accountState);
+ private class ProvidedUserCanSeePredicate implements Supplier<Predicate<AccountState>> {
+ CurrentUser asUser;
+
+ ProvidedUserCanSeePredicate(CurrentUser asUser) {
+ this.asUser = asUser;
+ }
+
+ @Override
+ public Predicate<AccountState> get() {
+ return accountControlFactory.get(asUser.asIdentifiedUser())::canSee;
+ }
}
private Predicate<AccountState> allVisiblePredicate() {
@@ -648,14 +811,16 @@
Result searchImpl(
String input,
ImmutableList<Searcher<?>> searchers,
+ CurrentUser asUser,
Supplier<Predicate<AccountState>> visibilitySupplier,
Predicate<AccountState> accountActivityPredicate)
throws ConfigInvalidException, IOException {
+ requireNonNull(asUser);
visibilitySupplier = Suppliers.memoize(visibilitySupplier::get);
List<AccountState> inactive = new ArrayList<>();
for (Searcher<?> searcher : searchers) {
- Optional<Stream<AccountState>> maybeResults = searcher.trySearch(input);
+ Optional<Stream<AccountState>> maybeResults = searcher.trySearch(input, asUser);
if (!maybeResults.isPresent()) {
continue;
}
@@ -677,22 +842,25 @@
}
if (!list.isEmpty()) {
- return createResult(input, list);
+ return createResult(input, list, asUser);
}
if (searcher.shortCircuitIfNoResults()) {
// For a short-circuiting searcher, return results even if empty.
- return !inactive.isEmpty() ? emptyResult(input, inactive) : createResult(input, list);
+ return !inactive.isEmpty()
+ ? emptyResult(input, inactive, asUser)
+ : createResult(input, list, asUser);
}
}
- return emptyResult(input, inactive);
+ return emptyResult(input, inactive, asUser);
}
- private Result createResult(String input, List<AccountState> list) {
- return new Result(input, list, ImmutableList.of());
+ private Result createResult(String input, List<AccountState> list, CurrentUser searchedAsUser) {
+ return new Result(input, list, ImmutableList.of(), searchedAsUser);
}
- private Result emptyResult(String input, List<AccountState> inactive) {
- return new Result(input, ImmutableList.of(), inactive);
+ private Result emptyResult(
+ String input, List<AccountState> inactive, CurrentUser searchedAsUser) {
+ return new Result(input, ImmutableList.of(), inactive, searchedAsUser);
}
private Stream<AccountState> toAccountStates(Set<Account.Id> ids) {
diff --git a/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index f4c211d..cbf47c5 100644
--- a/java/com/google/gerrit/server/mail/send/ProjectWatch.java
+++ b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
@@ -253,6 +253,7 @@
qb = WatcherChangeQueryBuilder.asUser(args.queryBuilder.get(), user);
p = qb.isVisible();
}
+ qb.forceAccountVisibilityCheck();
if (filter != null) {
Predicate<ChangeData> filterPredicate = qb.parse(filter);
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 510ca01..714516c 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -488,6 +488,7 @@
private final Arguments args;
protected Map<String, String> hasOperandAliases = Collections.emptyMap();
private Map<Account.Id, DestinationList> destinationListByAccount = new HashMap<>();
+ private boolean forceAccountVisibilityCheck = false;
private static final Splitter RULE_SPLITTER = Splitter.on("=");
private static final Splitter PLUGIN_SPLITTER = Splitter.on("_");
@@ -518,6 +519,11 @@
return args;
}
+ /** Whether to force account visibility check when searching for changes by account(s). */
+ public void forceAccountVisibilityCheck() {
+ forceAccountVisibilityCheck = true;
+ }
+
@Operator
public Predicate<ChangeData> age(String value) {
return new AgePredicate(value);
@@ -1697,7 +1703,9 @@
private Set<Account.Id> parseAccount(String who)
throws QueryParseException, IOException, ConfigInvalidException {
try {
- return args.accountResolver.resolve(who).asNonEmptyIdSet();
+ return args.accountResolver
+ .resolveAsUser(args.getUser(), who, forceAccountVisibilityCheck)
+ .asNonEmptyIdSet();
} catch (UnresolvableAccountException e) {
if (e.isSelf()) {
throw new QueryRequiresAuthException(e.getMessage(), e);
@@ -1710,7 +1718,9 @@
String who, java.util.function.Predicate<AccountState> activityFilter)
throws QueryParseException, IOException, ConfigInvalidException {
try {
- return args.accountResolver.resolve(who, activityFilter).asNonEmptyIdSet();
+ return args.accountResolver
+ .resolveAsUser(args.getUser(), who, activityFilter, forceAccountVisibilityCheck)
+ .asNonEmptyIdSet();
} catch (UnresolvableAccountException e) {
if (e.isSelf()) {
throw new QueryRequiresAuthException(e.getMessage(), e);
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index 16617b4..cf1eee0 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -22,6 +22,7 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.AccountGroup;
@@ -400,6 +401,111 @@
}
@Test
+ public void watchOwner() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+
+ // watch keyword in project as user
+ watch(watchedProject, "owner:admin");
+
+ // push a change with keyword -> should trigger email notification
+ requestScopeOperations.setApiUser(admin.id());
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(Project.nameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "subject", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification for user
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
+ assertThat(m.body()).contains("Change subject: subject\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
+ public void watchNonVisibleOwner() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+
+ // watch keyword in project as user
+ watch(watchedProject, "owner:admin");
+
+ // Verify that 'user' can't see 'admin'
+ assertThatAccountIsNotVisible(admin);
+
+ // push a change with keyword -> should trigger email notification
+ requestScopeOperations.setApiUser(admin.id());
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(Project.nameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "subject", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert no email notifications for user
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void watchChangesCommentedBySelf() throws Exception {
+ String watchedProject = projectOperations.newProject().create().get();
+ requestScopeOperations.setApiUser(user.id());
+
+ // user watches all changes that have a comment by themselves
+ watch(watchedProject, "commentby:self");
+
+ // pushing a change as admin should not trigger an email to user
+ requestScopeOperations.setApiUser(admin.id());
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(Project.nameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), watchedRepo, "subject", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+ assertThat(sender.getMessages()).isEmpty();
+
+ // commenting by admin should not trigger an email to user
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.message = "A Comment";
+ gApi.changes().id(r.getChangeId()).current().review(reviewInput);
+ assertThat(sender.getMessages()).isEmpty();
+
+ // commenting by user matches the project watch, but doesn't send an email to user because
+ // CC_ON_OWN_COMMENTS is false by default, so the user is removed from the TO list, but an email
+ // is sent to the admin user
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(r.getChangeId()).current().review(reviewInput);
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(admin.getNameEmail());
+ assertThat(m.body()).contains("Change subject: subject\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+
+ // commenting by admin now triggers an email to user because the change has a comment by user
+ // and hence matches the project watch
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.changes().id(r.getChangeId()).current().review(reviewInput);
+ messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
+ assertThat(m.body()).contains("Change subject: subject\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+ }
+
+ @Test
public void watchAllProjects() throws Exception {
String anyProject = projectOperations.newProject().create().get();
requestScopeOperations.setApiUser(user.id());
diff --git a/javatests/com/google/gerrit/server/account/AccountResolverTest.java b/javatests/com/google/gerrit/server/account/AccountResolverTest.java
index 3658834..37728f7 100644
--- a/javatests/com/google/gerrit/server/account/AccountResolverTest.java
+++ b/javatests/com/google/gerrit/server/account/AccountResolverTest.java
@@ -23,6 +23,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResolver.Result;
import com.google.gerrit.server.account.AccountResolver.Searcher;
import com.google.gerrit.server.account.AccountResolver.StringSearcher;
@@ -35,6 +37,8 @@
import org.junit.Test;
public class AccountResolverTest {
+ private final CurrentUser user = new AnonymousUser();
+
private static class TestSearcher extends StringSearcher {
private final String pattern;
private final boolean shortCircuit;
@@ -282,7 +286,7 @@
AccountResolver resolver = newAccountResolver();
assertThat(
new UnresolvableAccountException(
- resolver.new Result("foo", ImmutableList.of(), ImmutableList.of())))
+ resolver.new Result("foo", ImmutableList.of(), ImmutableList.of(), user)))
.hasMessageThat()
.isEqualTo("Account 'foo' not found");
}
@@ -292,7 +296,7 @@
AccountResolver resolver = newAccountResolver();
UnresolvableAccountException e =
new UnresolvableAccountException(
- resolver.new Result("self", ImmutableList.of(), ImmutableList.of()));
+ resolver.new Result("self", ImmutableList.of(), ImmutableList.of(), user));
assertThat(e.isSelf()).isTrue();
assertThat(e).hasMessageThat().isEqualTo("Resolving account 'self' requires login");
}
@@ -302,7 +306,7 @@
AccountResolver resolver = newAccountResolver();
UnresolvableAccountException e =
new UnresolvableAccountException(
- resolver.new Result("me", ImmutableList.of(), ImmutableList.of()));
+ resolver.new Result("me", ImmutableList.of(), ImmutableList.of(), user));
assertThat(e.isSelf()).isTrue();
assertThat(e).hasMessageThat().isEqualTo("Resolving account 'me' requires login");
}
@@ -314,7 +318,10 @@
new UnresolvableAccountException(
resolver
.new Result(
- "foo", ImmutableList.of(newAccount(3), newAccount(1)), ImmutableList.of())))
+ "foo",
+ ImmutableList.of(newAccount(3), newAccount(1)),
+ ImmutableList.of(),
+ user)))
.hasMessageThat()
.isEqualTo(
"Account 'foo' is ambiguous (at most 3 shown):\n1: Anonymous Name (1)\n3: Anonymous Name (3)");
@@ -329,7 +336,8 @@
.new Result(
"foo",
ImmutableList.of(),
- ImmutableList.of(newInactiveAccount(3), newInactiveAccount(1)))))
+ ImmutableList.of(newInactiveAccount(3), newInactiveAccount(1)),
+ user)))
.hasMessageThat()
.isEqualTo(
"Account 'foo' only matches inactive accounts. To use an inactive account, retry"
@@ -352,10 +360,11 @@
Supplier<Predicate<AccountState>> visibilitySupplier,
Predicate<AccountState> activityPredicate)
throws Exception {
- return newAccountResolver().searchImpl(input, searchers, visibilitySupplier, activityPredicate);
+ return newAccountResolver()
+ .searchImpl(input, searchers, user, visibilitySupplier, activityPredicate);
}
- private static AccountResolver newAccountResolver() {
+ private AccountResolver newAccountResolver() {
return new AccountResolver(null, null, null, null, null, null, null, null, "Anonymous Name");
}
