diff --git a/src/applications/auth/adapter/PhutilOAuth1AuthAdapter.php b/src/applications/auth/adapter/PhutilOAuth1AuthAdapter.php --- a/src/applications/auth/adapter/PhutilOAuth1AuthAdapter.php +++ b/src/applications/auth/adapter/PhutilOAuth1AuthAdapter.php @@ -156,7 +156,7 @@ $authorize_token_uri = new PhutilURI($this->getAuthorizeTokenURI()); $authorize_token_uri->replaceQueryParam('oauth_token', $this->getToken()); - return (string)$authorize_token_uri; + return phutil_string_cast($authorize_token_uri); } protected function finishOAuthHandshake() { diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -219,12 +219,11 @@ $accounts = id(new PhabricatorExternalAccountQuery()) ->setViewer($viewer) ->withProviderConfigPHIDs(array($config->getPHID())) - ->withAccountIDs($raw_identifiers) + ->withRawAccountIdentifiers($raw_identifiers) ->needAccountIdentifiers(true) ->execute(); if (!$accounts) { - $account = $this->newExternalAccount() - ->setAccountID(head($raw_identifiers)); + $account = $this->newExternalAccount(); } else if (count($accounts) === 1) { $account = head($accounts); } else { @@ -270,10 +269,6 @@ if (!$account) { $account = $this->newExternalAccount() ->setUserPHID($user->getPHID()); - - // TODO: Remove this when "accountID" is removed; the column is not - // nullable. - $account->setAccountID(''); } return $this->didUpdateAccount($account); @@ -375,6 +370,11 @@ ->setAccountType($adapter->getAdapterType()) ->setAccountDomain($adapter->getAdapterDomain()); + // TODO: Remove this when "accountID" is removed; the column is not + // nullable. + + $account->setAccountID(''); + return $account; } diff --git a/src/applications/auth/query/PhabricatorExternalAccountQuery.php b/src/applications/auth/query/PhabricatorExternalAccountQuery.php --- a/src/applications/auth/query/PhabricatorExternalAccountQuery.php +++ b/src/applications/auth/query/PhabricatorExternalAccountQuery.php @@ -15,23 +15,18 @@ private $ids; private $phids; - private $accountIDs; private $userPHIDs; private $needImages; private $accountSecrets; private $providerConfigPHIDs; private $needAccountIdentifiers; + private $rawAccountIdentifiers; public function withUserPHIDs(array $user_phids) { $this->userPHIDs = $user_phids; return $this; } - public function withAccountIDs(array $account_ids) { - $this->accountIDs = $account_ids; - return $this; - } - public function withPHIDs(array $phids) { $this->phids = $phids; return $this; @@ -62,6 +57,11 @@ return $this; } + public function withRawAccountIdentifiers(array $identifiers) { + $this->rawAccountIdentifiers = $identifiers; + return $this; + } + public function newResultObject() { return new PhabricatorExternalAccount(); } @@ -152,48 +152,98 @@ if ($this->ids !== null) { $where[] = qsprintf( $conn, - 'id IN (%Ld)', + 'account.id IN (%Ld)', $this->ids); } if ($this->phids !== null) { $where[] = qsprintf( $conn, - 'phid IN (%Ls)', + 'account.phid IN (%Ls)', $this->phids); } - if ($this->accountIDs !== null) { - $where[] = qsprintf( - $conn, - 'accountID IN (%Ls)', - $this->accountIDs); - } - if ($this->userPHIDs !== null) { $where[] = qsprintf( $conn, - 'userPHID IN (%Ls)', + 'account.userPHID IN (%Ls)', $this->userPHIDs); } if ($this->accountSecrets !== null) { $where[] = qsprintf( $conn, - 'accountSecret IN (%Ls)', + 'account.accountSecret IN (%Ls)', $this->accountSecrets); } if ($this->providerConfigPHIDs !== null) { $where[] = qsprintf( $conn, - 'providerConfigPHID IN (%Ls)', + 'account.providerConfigPHID IN (%Ls)', $this->providerConfigPHIDs); + + // If we have a list of ProviderConfig PHIDs and are joining the + // identifiers table, also include the list as an additional constraint + // on the identifiers table. + + // This does not change the query results (an Account and its + // Identifiers always have the same ProviderConfig PHID) but it allows + // us to use keys on the Identifier table more efficiently. + + if ($this->shouldJoinIdentifiersTable()) { + $where[] = qsprintf( + $conn, + 'identifier.providerConfigPHID IN (%Ls)', + $this->providerConfigPHIDs); + } + } + + if ($this->rawAccountIdentifiers !== null) { + $hashes = array(); + + foreach ($this->rawAccountIdentifiers as $raw_identifier) { + $hashes[] = PhabricatorHash::digestForIndex($raw_identifier); + } + + $where[] = qsprintf( + $conn, + 'identifier.identifierHash IN (%Ls)', + $hashes); } return $where; } + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); + + if ($this->shouldJoinIdentifiersTable()) { + $joins[] = qsprintf( + $conn, + 'JOIN %R identifier ON account.phid = identifier.externalAccountPHID', + new PhabricatorExternalAccountIdentifier()); + } + + return $joins; + } + + protected function shouldJoinIdentifiersTable() { + return ($this->rawAccountIdentifiers !== null); + } + + protected function shouldGroupQueryResultRows() { + if ($this->shouldJoinIdentifiersTable()) { + return true; + } + + return parent::shouldGroupQueryResultRows(); + } + + protected function getPrimaryTableAlias() { + return 'account'; + } + public function getQueryApplicationClass() { return 'PhabricatorPeopleApplication'; }