Resolve "self" even if inactive

Different systems may implement reactivation of accounts in different
ways. On googlesource.com, for example, accounts are automatically
reactivated on login through the web UI, but there are other codepaths
where reactivation may not happen automatically. Filtering out inactive
users when resolving "self" is therefore a regression: it would return
no results where prior to the AccountResolver rewrite it would have
returned results.

Fix the regression by resolving "self" even when inactive. I'm not
arguing that not auto-reactivating is the best behavior for
googlesource.com, just switching back to the old behavior of
AccountResolver.

The regressed behavior was especially confusing to debug because it
resulted in an exception message saying "Resolving account 'self'
requires login", even though the user was technically logged in.

Change-Id: I5aaf20eeb94ea6de268a96230d14ac85c4eb1863

Documentation/rest-api-accounts.txt

@@ -2153,8 +2153,8 @@ In all cases, accounts that are not
 link:config-gerrit.txt#accounts.visibility[visible] to the calling user are not
 considered.
 
-In all cases _except_ a bare account ID, inactive accounts are not considered.
-Inactive accounts may only be referenced by bare ID.
+In all cases _except_ a bare account ID and `self`/`me`, inactive accounts are
+not considered. Inactive accounts should only be referenced by bare ID.
 
 If the input is a bare account ID, this will always resolve to exactly
 one account if there is a visible account with that ID, and zero accounts

java/com/google/gerrit/server/account/AccountResolver.java

@@ -68,7 +68,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
  * </ol>
  *
  * <p>The result never includes accounts that are not visible to the calling user. It also never
- * includes inactive accounts, with one specific exception noted in method Javadoc.
+ * includes inactive accounts, with a small number of specific exceptions noted in method Javadoc.
  */
 @Singleton
 public class AccountResolver {
@@ -251,6 +251,11 @@ public class AccountResolver {
   }
 
   private class BySelf extends StringSearcher {
+    @Override
+    public boolean callerShouldFilterOutInactiveCandidates() {
+      return false;
+    }
+
     @Override
     public boolean callerMayAssumeCandidatesAreVisible() {
       return true;
@@ -279,8 +284,6 @@ public class AccountResolver {
   private class ByExactAccountId extends AccountIdSearcher {
     @Override
     public boolean callerShouldFilterOutInactiveCandidates() {
-      // The only case where we *don't* enforce that the account is active is when passing an exact
-      // numeric account ID.
       return false;
     }
 
@@ -497,9 +500,9 @@ public class AccountResolver {
    *
    * <ul>
    *   <li>The strings {@code "self"} and {@code "me"}, if the current user is an {@link
-   *       IdentifiedUser}.
-   *   <li>A bare account ID ({@code "18419"}). In this case, and <strong>only</strong> this case,
-   *       may return exactly one inactive account. This case short-circuits if the input matches.
+   *       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"}).

javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java

@@ -104,6 +104,19 @@ public class AccountResolverIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void bySelfInactive() throws Exception {
+    gApi.accounts().id(user.id.get()).setActive(false);
+
+    requestScopeOperations.setApiUser(user.id);
+    assertThat(gApi.accounts().id("self").getActive()).isFalse();
+
+    Result result = resolveAsResult("self");
+    assertThat(result.asIdSet()).containsExactly(user.id);
+    assertThat(result.isSelf()).isTrue();
+    assertThat(result.asUniqueUser()).isSameAs(self.get());
+  }
+
   @Test
   public void byExactAccountId() throws Exception {
     Account.Id existingId = accountOperations.newAccount().create();

javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -3113,6 +3113,27 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     }
   }
 
+  @Test
+  public void selfSucceedsForInactiveAccount() throws Exception {
+    Account.Id user2 =
+        accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
+
+    TestRepository<Repo> repo = createProject("repo");
+    Change change = insert(repo, newChange(repo));
+    AssigneeInput ain = new AssigneeInput();
+    ain.assignee = user2.toString();
+    gApi.changes().id(change.getId().get()).setAssignee(ain);
+
+    RequestContext adminContext = requestContext.setContext(newRequestContext(user2));
+    assertQuery("assignee:self", change);
+
+    requestContext.setContext(adminContext);
+    gApi.accounts().id(user2.get()).setActive(false);
+
+    requestContext.setContext(newRequestContext(user2));
+    assertQuery("assignee:self", change);
+  }
+
   protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
     return newChange(repo, null, null, null, null, false);
   }