@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Remove all readers and writers of "accountID" on "ExternalAccount"

Summary: Depends on D21019. Ref T13493. There are no more barriers to removing readers and writers of "accountID"; the new "ExternalAccountIdentity" table can replace it completely.

Test Plan: Linked and unlinked OAuth accounts, logged in with OAuth accounts, tried to double-link OAuth accounts, grepped for affected symbols.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21022

+76 -26
+1 -1
src/applications/auth/adapter/PhutilOAuth1AuthAdapter.php
··· 156 156 $authorize_token_uri = new PhutilURI($this->getAuthorizeTokenURI()); 157 157 $authorize_token_uri->replaceQueryParam('oauth_token', $this->getToken()); 158 158 159 - return (string)$authorize_token_uri; 159 + return phutil_string_cast($authorize_token_uri); 160 160 } 161 161 162 162 protected function finishOAuthHandshake() {
+7 -7
src/applications/auth/provider/PhabricatorAuthProvider.php
··· 219 219 $accounts = id(new PhabricatorExternalAccountQuery()) 220 220 ->setViewer($viewer) 221 221 ->withProviderConfigPHIDs(array($config->getPHID())) 222 - ->withAccountIDs($raw_identifiers) 222 + ->withRawAccountIdentifiers($raw_identifiers) 223 223 ->needAccountIdentifiers(true) 224 224 ->execute(); 225 225 if (!$accounts) { 226 - $account = $this->newExternalAccount() 227 - ->setAccountID(head($raw_identifiers)); 226 + $account = $this->newExternalAccount(); 228 227 } else if (count($accounts) === 1) { 229 228 $account = head($accounts); 230 229 } else { ··· 270 269 if (!$account) { 271 270 $account = $this->newExternalAccount() 272 271 ->setUserPHID($user->getPHID()); 273 - 274 - // TODO: Remove this when "accountID" is removed; the column is not 275 - // nullable. 276 - $account->setAccountID(''); 277 272 } 278 273 279 274 return $this->didUpdateAccount($account); ··· 374 369 $account 375 370 ->setAccountType($adapter->getAdapterType()) 376 371 ->setAccountDomain($adapter->getAdapterDomain()); 372 + 373 + // TODO: Remove this when "accountID" is removed; the column is not 374 + // nullable. 375 + 376 + $account->setAccountID(''); 377 377 378 378 return $account; 379 379 }
+68 -18
src/applications/auth/query/PhabricatorExternalAccountQuery.php
··· 15 15 16 16 private $ids; 17 17 private $phids; 18 - private $accountIDs; 19 18 private $userPHIDs; 20 19 private $needImages; 21 20 private $accountSecrets; 22 21 private $providerConfigPHIDs; 23 22 private $needAccountIdentifiers; 23 + private $rawAccountIdentifiers; 24 24 25 25 public function withUserPHIDs(array $user_phids) { 26 26 $this->userPHIDs = $user_phids; 27 - return $this; 28 - } 29 - 30 - public function withAccountIDs(array $account_ids) { 31 - $this->accountIDs = $account_ids; 32 27 return $this; 33 28 } 34 29 ··· 59 54 60 55 public function withProviderConfigPHIDs(array $phids) { 61 56 $this->providerConfigPHIDs = $phids; 57 + return $this; 58 + } 59 + 60 + public function withRawAccountIdentifiers(array $identifiers) { 61 + $this->rawAccountIdentifiers = $identifiers; 62 62 return $this; 63 63 } 64 64 ··· 152 152 if ($this->ids !== null) { 153 153 $where[] = qsprintf( 154 154 $conn, 155 - 'id IN (%Ld)', 155 + 'account.id IN (%Ld)', 156 156 $this->ids); 157 157 } 158 158 159 159 if ($this->phids !== null) { 160 160 $where[] = qsprintf( 161 161 $conn, 162 - 'phid IN (%Ls)', 162 + 'account.phid IN (%Ls)', 163 163 $this->phids); 164 164 } 165 165 166 - if ($this->accountIDs !== null) { 167 - $where[] = qsprintf( 168 - $conn, 169 - 'accountID IN (%Ls)', 170 - $this->accountIDs); 171 - } 172 - 173 166 if ($this->userPHIDs !== null) { 174 167 $where[] = qsprintf( 175 168 $conn, 176 - 'userPHID IN (%Ls)', 169 + 'account.userPHID IN (%Ls)', 177 170 $this->userPHIDs); 178 171 } 179 172 180 173 if ($this->accountSecrets !== null) { 181 174 $where[] = qsprintf( 182 175 $conn, 183 - 'accountSecret IN (%Ls)', 176 + 'account.accountSecret IN (%Ls)', 184 177 $this->accountSecrets); 185 178 } 186 179 187 180 if ($this->providerConfigPHIDs !== null) { 188 181 $where[] = qsprintf( 189 182 $conn, 190 - 'providerConfigPHID IN (%Ls)', 183 + 'account.providerConfigPHID IN (%Ls)', 191 184 $this->providerConfigPHIDs); 185 + 186 + // If we have a list of ProviderConfig PHIDs and are joining the 187 + // identifiers table, also include the list as an additional constraint 188 + // on the identifiers table. 189 + 190 + // This does not change the query results (an Account and its 191 + // Identifiers always have the same ProviderConfig PHID) but it allows 192 + // us to use keys on the Identifier table more efficiently. 193 + 194 + if ($this->shouldJoinIdentifiersTable()) { 195 + $where[] = qsprintf( 196 + $conn, 197 + 'identifier.providerConfigPHID IN (%Ls)', 198 + $this->providerConfigPHIDs); 199 + } 200 + } 201 + 202 + if ($this->rawAccountIdentifiers !== null) { 203 + $hashes = array(); 204 + 205 + foreach ($this->rawAccountIdentifiers as $raw_identifier) { 206 + $hashes[] = PhabricatorHash::digestForIndex($raw_identifier); 207 + } 208 + 209 + $where[] = qsprintf( 210 + $conn, 211 + 'identifier.identifierHash IN (%Ls)', 212 + $hashes); 192 213 } 193 214 194 215 return $where; 216 + } 217 + 218 + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { 219 + $joins = parent::buildJoinClauseParts($conn); 220 + 221 + if ($this->shouldJoinIdentifiersTable()) { 222 + $joins[] = qsprintf( 223 + $conn, 224 + 'JOIN %R identifier ON account.phid = identifier.externalAccountPHID', 225 + new PhabricatorExternalAccountIdentifier()); 226 + } 227 + 228 + return $joins; 229 + } 230 + 231 + protected function shouldJoinIdentifiersTable() { 232 + return ($this->rawAccountIdentifiers !== null); 233 + } 234 + 235 + protected function shouldGroupQueryResultRows() { 236 + if ($this->shouldJoinIdentifiersTable()) { 237 + return true; 238 + } 239 + 240 + return parent::shouldGroupQueryResultRows(); 241 + } 242 + 243 + protected function getPrimaryTableAlias() { 244 + return 'account'; 195 245 } 196 246 197 247 public function getQueryApplicationClass() {