1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 00:32:42 +01:00

Make repository identity email address association case-insensitive

Summary:
Ref T13444. Currently, identities for a particular email address are queried with "LIKE" against a binary column, which makes the query case-sensitive.

  - Extract the email address into a separate "sort255" column.
  - Add a key for it.
  - Make the query a standard "IN (%Ls)" query.
  - Deal with weird cases where an email address is 10000 bytes long or full of binary junk.

Test Plan:
  - Ran migration, inspected database for general sanity.
  - Ran query script in T13444, saw it return the same hits for "git@" and "GIT@".

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13444

Differential Revision: https://secure.phabricator.com/D20907
This commit is contained in:
epriestley 2019-11-13 18:42:23 -08:00
parent d58eddcf0a
commit df0f5c6cee
7 changed files with 82 additions and 15 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_repository.repository_identity
ADD emailAddress VARCHAR(255) COLLATE {$COLLATE_SORT};

View file

@ -0,0 +1,26 @@
<?php
$table = new PhabricatorRepositoryIdentity();
$conn = $table->establishConnection('w');
$iterator = new LiskRawMigrationIterator($conn, $table->getTableName());
foreach ($iterator as $row) {
$name = $row['identityNameRaw'];
$name = phutil_utf8ize($name);
$email = new PhutilEmailAddress($name);
$address = $email->getAddress();
try {
queryfx(
$conn,
'UPDATE %R SET emailAddress = %ns WHERE id = %d',
$table,
$address,
$row['id']);
} catch (Exception $ex) {
// We may occasionally run into issues with binary or very long addresses.
// Just skip over them.
continue;
}
}

View file

@ -22,11 +22,10 @@ final class DiffusionIdentityViewController
$header = id(new PHUIHeaderView()) $header = id(new PHUIHeaderView())
->setUser($viewer) ->setUser($viewer)
->setHeader($identity->getIdentityShortName()) ->setHeader($identity->getIdentityShortName())
->setHeaderIcon('fa-globe') ->setHeaderIcon('fa-globe');
->setPolicyObject($identity);
$crumbs = $this->buildApplicationCrumbs(); $crumbs = $this->buildApplicationCrumbs();
$crumbs->addTextCrumb($identity->getID()); $crumbs->addTextCrumb($identity->getObjectName());
$crumbs->setBorder(true); $crumbs->setBorder(true);
$timeline = $this->buildTransactionTimeline( $timeline = $this->buildTransactionTimeline(
@ -83,7 +82,11 @@ final class DiffusionIdentityViewController
$viewer = $this->getViewer(); $viewer = $this->getViewer();
$properties = id(new PHUIPropertyListView()) $properties = id(new PHUIPropertyListView())
->setUser($viewer); ->setViewer($viewer);
$properties->addProperty(
pht('Email Address'),
$identity->getEmailAddress());
$effective_phid = $identity->getCurrentEffectiveUserPHID(); $effective_phid = $identity->getCurrentEffectiveUserPHID();
$automatic_phid = $identity->getAutomaticGuessedUserPHID(); $automatic_phid = $identity->getAutomaticGuessedUserPHID();
@ -109,7 +112,7 @@ final class DiffusionIdentityViewController
pht('Automatically Detected User'), pht('Automatically Detected User'),
$this->buildPropertyValue($automatic_phid)); $this->buildPropertyValue($automatic_phid));
$properties->addProperty( $properties->addProperty(
pht('Manually Set User'), pht('Assigned To'),
$this->buildPropertyValue($manual_phid)); $this->buildPropertyValue($manual_phid));
$header = id(new PHUIHeaderView()) $header = id(new PHUIHeaderView())
@ -127,7 +130,7 @@ final class DiffusionIdentityViewController
if ($value == DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN) { if ($value == DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN) {
return phutil_tag('em', array(), pht('Explicitly Unassigned')); return phutil_tag('em', array(), pht('Explicitly Unassigned'));
} else if (!$value) { } else if (!$value) {
return null; return phutil_tag('em', array(), pht('None'));
} else { } else {
return $viewer->renderHandle($value); return $viewer->renderHandle($value);
} }

View file

@ -17,6 +17,14 @@ final class DiffusionRepositoryListController extends DiffusionController {
->setName(pht('Browse Commits')) ->setName(pht('Browse Commits'))
->setHref($this->getApplicationURI('commit/')); ->setHref($this->getApplicationURI('commit/'));
$items[] = id(new PHUIListItemView())
->setType(PHUIListItemView::TYPE_LABEL)
->setName(pht('Identities'));
$items[] = id(new PHUIListItemView())
->setName(pht('Browse Identities'))
->setHref($this->getApplicationURI('identity/'));
return id(new PhabricatorRepositorySearchEngine()) return id(new PhabricatorRepositorySearchEngine())
->setController($this) ->setController($this)
->setNavigationItems($items) ->setNavigationItems($items)

View file

@ -6,7 +6,7 @@ final class PhabricatorRepositoryIdentityQuery
private $ids; private $ids;
private $phids; private $phids;
private $identityNames; private $identityNames;
private $emailAddress; private $emailAddresses;
private $assigneePHIDs; private $assigneePHIDs;
private $identityNameLike; private $identityNameLike;
private $hasEffectivePHID; private $hasEffectivePHID;
@ -31,8 +31,8 @@ final class PhabricatorRepositoryIdentityQuery
return $this; return $this;
} }
public function withEmailAddress($address) { public function withEmailAddresses(array $addresses) {
$this->emailAddress = $address; $this->emailAddresses = $addresses;
return $this; return $this;
} }
@ -106,12 +106,11 @@ final class PhabricatorRepositoryIdentityQuery
$name_hashes); $name_hashes);
} }
if ($this->emailAddress !== null) { if ($this->emailAddresses !== null) {
$identity_style = "<{$this->emailAddress}>";
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'repository_identity.identityNameRaw LIKE %<', 'repository_identity.emailAddress IN (%Ls)',
$identity_style); $this->emailAddresses);
} }
if ($this->identityNameLike != null) { if ($this->identityNameLike != null) {

View file

@ -13,6 +13,7 @@ final class PhabricatorRepositoryIdentity
protected $automaticGuessedUserPHID; protected $automaticGuessedUserPHID;
protected $manuallySetUserPHID; protected $manuallySetUserPHID;
protected $currentEffectiveUserPHID; protected $currentEffectiveUserPHID;
protected $emailAddress;
protected function getConfiguration() { protected function getConfiguration() {
return array( return array(
@ -26,12 +27,16 @@ final class PhabricatorRepositoryIdentity
'automaticGuessedUserPHID' => 'phid?', 'automaticGuessedUserPHID' => 'phid?',
'manuallySetUserPHID' => 'phid?', 'manuallySetUserPHID' => 'phid?',
'currentEffectiveUserPHID' => 'phid?', 'currentEffectiveUserPHID' => 'phid?',
'emailAddress' => 'sort255?',
), ),
self::CONFIG_KEY_SCHEMA => array( self::CONFIG_KEY_SCHEMA => array(
'key_identity' => array( 'key_identity' => array(
'columns' => array('identityNameHash'), 'columns' => array('identityNameHash'),
'unique' => true, 'unique' => true,
), ),
'key_email' => array(
'columns' => array('emailAddress(64)'),
),
), ),
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }
@ -69,6 +74,10 @@ final class PhabricatorRepositoryIdentity
return $this->getIdentityName(); return $this->getIdentityName();
} }
public function getObjectName() {
return pht('Identity %d', $this->getID());
}
public function getURI() { public function getURI() {
return '/diffusion/identity/view/'.$this->getID().'/'; return '/diffusion/identity/view/'.$this->getID().'/';
} }
@ -92,6 +101,25 @@ final class PhabricatorRepositoryIdentity
$this->currentEffectiveUserPHID = $this->automaticGuessedUserPHID; $this->currentEffectiveUserPHID = $this->automaticGuessedUserPHID;
} }
$email_address = $this->getIdentityEmailAddress();
// Raw identities are unrestricted binary data, and may consequently
// have arbitrarily long, binary email address information. We can't
// store this kind of information in the "emailAddress" column, which
// has column type "sort255".
// This kind of address almost certainly not legitimate and users can
// manually set the target of the identity, so just discard it rather
// than trying especially hard to make it work.
$byte_limit = $this->getColumnMaximumByteLength('emailAddress');
$email_address = phutil_utf8ize($email_address);
if (strlen($email_address) > $byte_limit) {
$email_address = null;
}
$this->setEmailAddress($email_address);
return parent::save(); return parent::save();
} }
@ -111,7 +139,8 @@ final class PhabricatorRepositoryIdentity
} }
public function hasAutomaticCapability( public function hasAutomaticCapability(
$capability, PhabricatorUser $viewer) { $capability,
PhabricatorUser $viewer) {
return false; return false;
} }

View file

@ -21,7 +21,7 @@ extends PhabricatorWorker {
foreach ($emails as $email) { foreach ($emails as $email) {
$identities = id(new PhabricatorRepositoryIdentityQuery()) $identities = id(new PhabricatorRepositoryIdentityQuery())
->setViewer($viewer) ->setViewer($viewer)
->withEmailAddress($email->getAddress()) ->withEmailAddresses($email->getAddress())
->execute(); ->execute();
foreach ($identities as $identity) { foreach ($identities as $identity) {