Page MenuHomePhabricator

D21022.diff
No OneTemporary

D21022.diff

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';
}

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 9, 6:17 PM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390506
Default Alt Text
D21022.diff (6 KB)

Event Timeline