(stable) Promote 2019 Week 47

resources/sql/autopatches/20191113.identity.01.email.sql

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

resources/sql/autopatches/20191113.identity.02.populate.php

@@ -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;
+  }
+}

resources/sql/autopatches/20191113.identity.03.unassigned.sql

@@ -0,0 +1,3 @@
+UPDATE {$NAMESPACE}_repository.repository_identity
+  SET currentEffectiveUserPHID = NULL
+  WHERE currentEffectiveUserPHID = 'unassigned()';

resources/sql/autopatches/20191114.email.01.phid.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_user.user_email
+  ADD phid VARBINARY(64) NOT NULL;

resources/sql/autopatches/20191114.email.02.populate.php

@@ -0,0 +1,18 @@
+<?php
+
+$table = new PhabricatorUserEmail();
+$conn = $table->establishConnection('w');
+
+$iterator = new LiskRawMigrationIterator($conn, $table->getTableName());
+foreach ($iterator as $row) {
+  $phid = $row['phid'];
+
+  if (!strlen($phid)) {
+    queryfx(
+      $conn,
+      'UPDATE %R SET phid = %s WHERE id = %d',
+      $table,
+      $table->generatePHID(),
+      $row['id']);
+  }
+}

src/__phutil_library_map__.php

@@ -984,7 +984,9 @@ phutil_register_library_map(array(
     'DiffusionRepositoryEditUpdateController' => 'applications/diffusion/controller/DiffusionRepositoryEditUpdateController.php',
     'DiffusionRepositoryFunctionDatasource' => 'applications/diffusion/typeahead/DiffusionRepositoryFunctionDatasource.php',
     'DiffusionRepositoryHistoryManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryHistoryManagementPanel.php',
+    'DiffusionRepositoryIdentityDestructionEngineExtension' => 'applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php',
     'DiffusionRepositoryIdentityEditor' => 'applications/diffusion/editor/DiffusionRepositoryIdentityEditor.php',
+    'DiffusionRepositoryIdentityEngine' => 'applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php',
     'DiffusionRepositoryIdentitySearchEngine' => 'applications/diffusion/query/DiffusionRepositoryIdentitySearchEngine.php',
     'DiffusionRepositoryLimitsManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php',
     'DiffusionRepositoryListController' => 'applications/diffusion/controller/DiffusionRepositoryListController.php',
@@ -4120,6 +4122,8 @@ phutil_register_library_map(array(
     'PhabricatorPeopleTasksProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleTasksProfileMenuItem.php',
     'PhabricatorPeopleTestDataGenerator' => 'applications/people/lipsum/PhabricatorPeopleTestDataGenerator.php',
     'PhabricatorPeopleTransactionQuery' => 'applications/people/query/PhabricatorPeopleTransactionQuery.php',
+    'PhabricatorPeopleUserEmailPHIDType' => 'applications/people/phid/PhabricatorPeopleUserEmailPHIDType.php',
+    'PhabricatorPeopleUserEmailQuery' => 'applications/people/query/PhabricatorPeopleUserEmailQuery.php',
     'PhabricatorPeopleUserFunctionDatasource' => 'applications/people/typeahead/PhabricatorPeopleUserFunctionDatasource.php',
     'PhabricatorPeopleUserPHIDType' => 'applications/people/phid/PhabricatorPeopleUserPHIDType.php',
     'PhabricatorPeopleUsernameMailEngine' => 'applications/people/mail/PhabricatorPeopleUsernameMailEngine.php',
@@ -6965,7 +6969,9 @@ phutil_register_library_map(array(
     'DiffusionRepositoryEditUpdateController' => 'DiffusionRepositoryManageController',
     'DiffusionRepositoryFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DiffusionRepositoryHistoryManagementPanel' => 'DiffusionRepositoryManagementPanel',
+    'DiffusionRepositoryIdentityDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
     'DiffusionRepositoryIdentityEditor' => 'PhabricatorApplicationTransactionEditor',
+    'DiffusionRepositoryIdentityEngine' => 'Phobject',
     'DiffusionRepositoryIdentitySearchEngine' => 'PhabricatorApplicationSearchEngine',
     'DiffusionRepositoryLimitsManagementPanel' => 'DiffusionRepositoryManagementPanel',
     'DiffusionRepositoryListController' => 'DiffusionController',
@@ -10615,6 +10621,8 @@ phutil_register_library_map(array(
     'PhabricatorPeopleTasksProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorPeopleTestDataGenerator' => 'PhabricatorTestDataGenerator',
     'PhabricatorPeopleTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorPeopleUserEmailPHIDType' => 'PhabricatorPHIDType',
+    'PhabricatorPeopleUserEmailQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorPeopleUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorPeopleUserPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorPeopleUsernameMailEngine' => 'PhabricatorPeopleMailEngine',
@@ -11679,7 +11687,10 @@ phutil_register_library_map(array(
     'PhabricatorUserEditEngine' => 'PhabricatorEditEngine',
     'PhabricatorUserEditor' => 'PhabricatorEditor',
     'PhabricatorUserEditorTestCase' => 'PhabricatorTestCase',
-    'PhabricatorUserEmail' => 'PhabricatorUserDAO',
+    'PhabricatorUserEmail' => array(
+      'PhabricatorUserDAO',
+      'PhabricatorDestructibleInterface',
+    ),
     'PhabricatorUserEmailTestCase' => 'PhabricatorTestCase',
     'PhabricatorUserEmpowerTransaction' => 'PhabricatorUserTransactionType',
     'PhabricatorUserFerretEngine' => 'PhabricatorFerretEngine',

src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php

@@ -93,6 +93,12 @@ final class PhabricatorAuditManagementDeleteWorkflow
 
     if ($repos) {
       $query->withRepositoryIDs(mpull($repos, 'getID'));
+
+      // See T13457. If we're iterating over commits in a single large
+      // repository, the lack of a "<repositoryID, [id]>" key can slow things
+      // down. Iterate in a specific order to use a key which is present
+      // on the table ("<repositoryID, epoch, [id]>").
+      $query->setOrderVector(array('-epoch', '-id'));
     }
 
     $auditor_map = array();
@@ -105,7 +111,11 @@ final class PhabricatorAuditManagementDeleteWorkflow
       $query->withPHIDs(mpull($commits, 'getPHID'));
     }
 
-    $commit_iterator = new PhabricatorQueryIterator($query);
+    $commit_iterator = id(new PhabricatorQueryIterator($query));
+
+    // See T13457. We may be examining many commits; each commit is small so
+    // we can safely increase the page size to improve performance a bit.
+    $commit_iterator->setPageSize(1000);
 
     $audits = array();
     foreach ($commit_iterator as $commit) {

src/applications/conduit/controller/PhabricatorConduitAPIController.php

@@ -99,9 +99,6 @@ final class PhabricatorConduitAPIController
         list($error_code, $error_info) = $auth_error;
       }
     } catch (Exception $ex) {
-      if (!($ex instanceof ConduitMethodNotFoundException)) {
-        phlog($ex);
-      }
       $result = null;
       $error_code = ($ex instanceof ConduitException
         ? 'ERR-CONDUIT-CALL'

src/applications/config/check/PhabricatorManualActivitySetupCheck.php

@@ -113,7 +113,8 @@ final class PhabricatorManualActivitySetupCheck
       'pre',
       array(),
       (string)csprintf(
-        'phabricator/ $ ./bin/repository rebuild-identities --all'));
+        'phabricator/ $ '.
+        './bin/repository rebuild-identities --all-repositories'));
 
     $message[] = pht(
       'You can find more information about this new identity mapping '.

src/applications/diffusion/controller/DiffusionIdentityViewController.php

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

src/applications/diffusion/controller/DiffusionRepositoryListController.php

@@ -17,6 +17,14 @@ final class DiffusionRepositoryListController extends DiffusionController {
       ->setName(pht('Browse Commits'))
       ->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())
       ->setController($this)
       ->setNavigationItems($items)

src/applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php

@@ -0,0 +1,40 @@
+<?php
+
+final class DiffusionRepositoryIdentityDestructionEngineExtension
+  extends PhabricatorDestructionEngineExtension {
+
+  const EXTENSIONKEY = 'repository-identities';
+
+  public function getExtensionName() {
+    return pht('Repository Identities');
+  }
+
+  public function didDestroyObject(
+    PhabricatorDestructionEngine $engine,
+    $object) {
+
+    // When users or email addresses are destroyed, queue a task to update
+    // any repository identities that are associated with them. See T13444.
+
+    $related_phids = array();
+    $email_addresses = array();
+
+    if ($object instanceof PhabricatorUser) {
+      $related_phids[] = $object->getPHID();
+    }
+
+    if ($object instanceof PhabricatorUserEmail) {
+      $email_addresses[] = $object->getAddress();
+    }
+
+    if ($related_phids || $email_addresses) {
+      PhabricatorWorker::scheduleTask(
+        'PhabricatorRepositoryIdentityChangeWorker',
+        array(
+          'relatedPHIDs' => $related_phids,
+          'emailAddresses' => $email_addresses,
+        ));
+    }
+  }
+
+}

src/applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php

@@ -0,0 +1,120 @@
+<?php
+
+final class DiffusionRepositoryIdentityEngine
+  extends Phobject {
+
+  private $viewer;
+  private $sourcePHID;
+  private $dryRun;
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+  public function setSourcePHID($source_phid) {
+    $this->sourcePHID = $source_phid;
+    return $this;
+  }
+
+  public function getSourcePHID() {
+    if (!$this->sourcePHID) {
+      throw new PhutilInvalidStateException('setSourcePHID');
+    }
+
+    return $this->sourcePHID;
+  }
+
+  public function setDryRun($dry_run) {
+    $this->dryRun = $dry_run;
+    return $this;
+  }
+
+  public function getDryRun() {
+    return $this->dryRun;
+  }
+
+  public function newResolvedIdentity($raw_identity) {
+    $identity = $this->loadRawIdentity($raw_identity);
+
+    if (!$identity) {
+      $identity = $this->newIdentity($raw_identity);
+    }
+
+    return $this->updateIdentity($identity);
+  }
+
+  public function newUpdatedIdentity(PhabricatorRepositoryIdentity $identity) {
+    return $this->updateIdentity($identity);
+  }
+
+  private function loadRawIdentity($raw_identity) {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorRepositoryIdentityQuery())
+      ->setViewer($viewer)
+      ->withIdentityNames(array($raw_identity))
+      ->executeOne();
+  }
+
+  private function newIdentity($raw_identity) {
+    $source_phid = $this->getSourcePHID();
+
+    return id(new PhabricatorRepositoryIdentity())
+      ->setAuthorPHID($source_phid)
+      ->setIdentityName($raw_identity);
+  }
+
+  private function resolveIdentity(PhabricatorRepositoryIdentity $identity) {
+    $raw_identity = $identity->getIdentityName();
+
+    return id(new DiffusionResolveUserQuery())
+      ->withName($raw_identity)
+      ->execute();
+  }
+
+  private function updateIdentity(PhabricatorRepositoryIdentity $identity) {
+
+    // If we're updating an identity and it has a manual user PHID associated
+    // with it but the user is no longer valid, remove the value. This likely
+    // corresponds to a user that was destroyed.
+
+    $assigned_phid = $identity->getManuallySetUserPHID();
+    $unassigned = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
+    if ($assigned_phid && ($assigned_phid !== $unassigned)) {
+      $viewer = $this->getViewer();
+      $user = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($assigned_phid))
+        ->executeOne();
+      if (!$user) {
+        $identity->setManuallySetUserPHID(null);
+      }
+    }
+
+    $resolved_phid = $this->resolveIdentity($identity);
+
+    $identity->setAutomaticGuessedUserPHID($resolved_phid);
+
+    if ($this->getDryRun()) {
+      $identity->makeEphemeral();
+    } else {
+      $identity->save();
+    }
+
+    return $identity;
+  }
+
+  public function didUpdateEmailAddress($raw_address) {
+    PhabricatorWorker::scheduleTask(
+      'PhabricatorRepositoryIdentityChangeWorker',
+      array(
+        'emailAddresses' => array($raw_address),
+      ));
+  }
+
+}

src/applications/diffusion/query/DiffusionRepositoryIdentitySearchEngine.php

@@ -17,21 +17,35 @@ final class DiffusionRepositoryIdentitySearchEngine
 
   protected function buildCustomSearchFields() {
     return array(
+      id(new PhabricatorUsersSearchField())
+        ->setLabel(pht('Matching Users'))
+        ->setKey('effectivePHIDs')
+        ->setAliases(
+          array(
+            'effective',
+            'effectivePHID',
+          ))
+        ->setDescription(pht('Search for identities by effective user.')),
       id(new DiffusionIdentityAssigneeSearchField())
         ->setLabel(pht('Assigned To'))
-        ->setKey('assignee')
+        ->setKey('assignedPHIDs')
-        ->setDescription(pht('Search for identities by assignee.')),
+        ->setAliases(
+          array(
+            'assigned',
+            'assignedPHID',
+          ))
+        ->setDescription(pht('Search for identities by explicit assignee.')),
       id(new PhabricatorSearchTextField())
         ->setLabel(pht('Identity Contains'))
         ->setKey('match')
         ->setDescription(pht('Search for identities by substring.')),
       id(new PhabricatorSearchThreeStateField())
-        ->setLabel(pht('Is Assigned'))
+        ->setLabel(pht('Has Matching User'))
         ->setKey('hasEffectivePHID')
         ->setOptions(
           pht('(Show All)'),
-          pht('Show Only Assigned Identities'),
+          pht('Show Identities With Matching Users'),
-          pht('Show Only Unassigned Identities')),
+          pht('Show Identities Without Matching Users')),
     );
   }
 
@@ -46,8 +60,12 @@ final class DiffusionRepositoryIdentitySearchEngine
       $query->withIdentityNameLike($map['match']);
     }
 
-    if ($map['assignee']) {
+    if ($map['assignedPHIDs']) {
-      $query->withAssigneePHIDs($map['assignee']);
+      $query->withAssignedPHIDs($map['assignedPHIDs']);
+    }
+
+    if ($map['effectivePHIDs']) {
+      $query->withEffectivePHIDs($map['effectivePHIDs']);
     }
 
     return $query;
@@ -86,15 +104,54 @@ final class DiffusionRepositoryIdentitySearchEngine
 
     $viewer = $this->requireViewer();
 
-    $list = new PHUIObjectItemListView();
+    $list = id(new PHUIObjectItemListView())
-    $list->setUser($viewer);
+      ->setViewer($viewer);
+
+    $phids = array();
+    foreach ($identities as $identity) {
+      $phids[] = $identity->getCurrentEffectiveUserPHID();
+      $phids[] = $identity->getManuallySetUserPHID();
+    }
+
+    $handles = $viewer->loadHandles($phids);
+
+    $unassigned = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
+
     foreach ($identities as $identity) {
       $item = id(new PHUIObjectItemView())
-        ->setObjectName(pht('Identity %d', $identity->getID()))
+        ->setObjectName($identity->getObjectName())
         ->setHeader($identity->getIdentityShortName())
         ->setHref($identity->getURI())
         ->setObject($identity);
 
+      $status_icon = 'fa-circle-o grey';
+
+      $effective_phid = $identity->getCurrentEffectiveUserPHID();
+      if ($effective_phid) {
+        $item->addIcon(
+          'fa-id-badge',
+          pht('Matches User: %s', $handles[$effective_phid]->getName()));
+
+        $status_icon = 'fa-id-badge';
+      }
+
+      $assigned_phid = $identity->getManuallySetUserPHID();
+      if ($assigned_phid) {
+        if ($assigned_phid === $unassigned) {
+          $item->addIcon(
+            'fa-ban',
+            pht('Explicitly Unassigned'));
+          $status_icon = 'fa-ban';
+        } else {
+          $item->addIcon(
+            'fa-user',
+            pht('Assigned To: %s', $handles[$assigned_phid]->getName()));
+          $status_icon = 'fa-user';
+        }
+      }
+
+      $item->setStatusIcon($status_icon);
+
       $list->addItem($item);
     }
 

src/applications/diffusion/query/DiffusionResolveUserQuery.php

@@ -8,25 +8,14 @@
 final class DiffusionResolveUserQuery extends Phobject {
 
   private $name;
-  private $commit;
 
   public function withName($name) {
     $this->name = $name;
     return $this;
   }
 
-  public function withCommit($commit) {
-    $this->commit = $commit;
-    return $this;
-  }
-
   public function execute() {
-    $user_name = $this->name;
+    return $this->findUserPHID($this->name);
-
-    $phid = $this->findUserPHID($this->name);
-    $phid = $this->fireLookupEvent($phid);
-
-    return $phid;
   }
 
   private function findUserPHID($user_name) {
@@ -75,33 +64,15 @@ final class DiffusionResolveUserQuery extends Phobject {
   }
 
 
-  /**
-   * Emit an event so installs can do custom lookup of commit authors who may
-   * not be naturally resolvable.
-   */
-  private function fireLookupEvent($guess) {
-
-    $type = PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER;
-    $data = array(
-      'commit'  => $this->commit,
-      'query'   => $this->name,
-      'result'  => $guess,
-    );
-
-    $event = new PhabricatorEvent($type, $data);
-    PhutilEventEngine::dispatchEvent($event);
-
-    return $event->getValue('result');
-  }
-
-
   private function findUserByUserName($user_name) {
     $by_username = id(new PhabricatorUser())->loadOneWhere(
       'userName = %s',
       $user_name);
+
     if ($by_username) {
       return $by_username->getPHID();
     }
+
     return null;
   }
 
@@ -112,18 +83,22 @@ final class DiffusionResolveUserQuery extends Phobject {
     $by_realname = id(new PhabricatorUser())->loadAllWhere(
       'realName = %s',
       $real_name);
+
     if (count($by_realname) == 1) {
-      return reset($by_realname)->getPHID();
+      return head($by_realname)->getPHID();
     }
+
     return null;
   }
 
 
   private function findUserByEmailAddress($email_address) {
     $by_email = PhabricatorUser::loadOneWithEmailAddress($email_address);
+
     if ($by_email) {
       return $by_email->getPHID();
     }
+
     return null;
   }
 

src/applications/owners/storage/PhabricatorOwnersPath.php

@@ -91,11 +91,22 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
   }
 
   private static function getScalarKeyForRef(array $ref) {
+    // See T13464. When building refs from raw transactions, the path has
+    // not been normalized yet and doesn't have a separate "display" path.
+    // If the "display" path isn't populated, just use the actual path to
+    // build the ref key.
+
+    if (isset($ref['display'])) {
+      $display = $ref['display'];
+    } else {
+      $display = $ref['path'];
+    }
+
     return sprintf(
       'repository=%s path=%s display=%s excluded=%d',
       $ref['repositoryPHID'],
       $ref['path'],
-      $ref['display'],
+      $display,
       $ref['excluded']);
   }
 

src/applications/people/editor/PhabricatorUserEditor.php

@@ -89,6 +89,9 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
       $this->didVerifyEmail($user, $email);
     }
 
+    id(new DiffusionRepositoryIdentityEngine())
+      ->didUpdateEmailAddress($email->getAddress());
+
     return $this;
   }
 
@@ -202,11 +205,8 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
       $user->endWriteLocking();
     $user->saveTransaction();
 
-    // Try and match this new address against unclaimed `RepositoryIdentity`s
+    id(new DiffusionRepositoryIdentityEngine())
-    PhabricatorWorker::scheduleTask(
+      ->didUpdateEmailAddress($email->getAddress());
-      'PhabricatorRepositoryIdentityChangeWorker',
-      array('userPHID' => $user->getPHID()),
-      array('objectPHID' => $user->getPHID()));
 
     return $this;
   }
@@ -241,7 +241,9 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
           throw new Exception(pht('Email not owned by user!'));
         }
 
-        $email->delete();
+        $destruction_engine = id(new PhabricatorDestructionEngine())
+          ->setWaitToFinalizeDestruction(true)
+          ->destroyObject($email);
 
         $log = PhabricatorUserLog::initializeNewLog(
           $actor,
@@ -254,6 +256,7 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
     $user->saveTransaction();
 
     $this->revokePasswordResetLinks($user);
+    $destruction_engine->finalizeDestruction();
 
     return $this;
   }
@@ -326,7 +329,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
     }
     $email->sendNewPrimaryEmail($user);
 
-
     $this->revokePasswordResetLinks($user);
 
     return $this;
@@ -440,6 +442,9 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
 
       $user->endWriteLocking();
     $user->saveTransaction();
+
+    id(new DiffusionRepositoryIdentityEngine())
+      ->didUpdateEmailAddress($email->getAddress());
   }
 
 

src/applications/people/phid/PhabricatorPeopleUserEmailPHIDType.php

@@ -0,0 +1,35 @@
+<?php
+
+final class PhabricatorPeopleUserEmailPHIDType
+  extends PhabricatorPHIDType {
+
+  const TYPECONST = 'EADR';
+
+  public function getTypeName() {
+    return pht('User Email');
+  }
+
+  public function newObject() {
+    return new PhabricatorUserEmail();
+  }
+
+  public function getPHIDTypeApplicationClass() {
+    return 'PhabricatorPeopleApplication';
+  }
+
+  protected function buildQueryForObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new PhabricatorPeopleUserEmailQuery())
+      ->withPHIDs($phids);
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+    return null;
+  }
+
+}

src/applications/people/query/PhabricatorPeopleUserEmailQuery.php

@@ -0,0 +1,55 @@
+<?php
+
+final class PhabricatorPeopleUserEmailQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function newResultObject() {
+    return new PhabricatorUserEmail();
+  }
+
+  protected function loadPage() {
+    return $this->loadStandardPage($this->newResultObject());
+  }
+
+  protected function getPrimaryTableAlias() {
+    return 'email';
+  }
+
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
+
+    if ($this->ids !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'email.id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'email.phid IN (%Ls)',
+        $this->phids);
+    }
+
+    return $where;
+  }
+
+  public function getQueryApplicationClass() {
+    return 'PhabricatorPeopleApplication';
+  }
+
+}

src/applications/people/storage/PhabricatorUser.php

@@ -1148,7 +1148,7 @@ final class PhabricatorUser
         'userPHID = %s',
         $this->getPHID());
       foreach ($emails as $email) {
-        $email->delete();
+        $engine->destroyObject($email);
       }
 
       $sessions = id(new PhabricatorAuthSession())->loadAllWhere(

src/applications/people/storage/PhabricatorUserEmail.php

@@ -4,7 +4,9 @@
  * @task restrictions   Domain Restrictions
  * @task email          Email About Email
  */
-final class PhabricatorUserEmail extends PhabricatorUserDAO {
+final class PhabricatorUserEmail
+  extends PhabricatorUserDAO
+  implements PhabricatorDestructibleInterface {
 
   protected $userPHID;
   protected $address;
@@ -16,6 +18,7 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO {
 
   protected function getConfiguration() {
     return array(
+      self::CONFIG_AUX_PHID => true,
       self::CONFIG_COLUMN_SCHEMA => array(
         'address' => 'sort128',
         'isVerified' => 'bool',
@@ -34,6 +37,10 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO {
     ) + parent::getConfiguration();
   }
 
+  public function getPHIDType() {
+    return PhabricatorPeopleUserEmailPHIDType::TYPECONST;
+  }
+
   public function getVerificationURI() {
     return '/emailverify/'.$this->getVerificationCode().'/';
   }
@@ -271,4 +278,13 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO {
     return $this;
   }
 
+
+/* -(  PhabricatorDestructibleInterface  )----------------------------------- */
+
+
+  public function destroyObjectPermanently(
+    PhabricatorDestructionEngine $engine) {
+    $this->delete();
+  }
+
 }

src/applications/repository/management/PhabricatorRepositoryManagementLookupUsersWorkflow.php

@@ -99,7 +99,6 @@ final class PhabricatorRepositoryManagementLookupUsersWorkflow
 
   private function resolveUser(PhabricatorRepositoryCommit $commit, $name) {
     $phid = id(new DiffusionResolveUserQuery())
-      ->withCommit($commit)
       ->withName($name)
       ->execute();
 

src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php

@@ -3,6 +3,10 @@
 final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
   extends PhabricatorRepositoryManagementWorkflow {
 
+  private $identityCache = array();
+  private $phidCache = array();
+  private $dryRun;
+
   protected function didConstruct() {
     $this
       ->setName('rebuild-identities')
@@ -12,38 +16,189 @@ final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
       ->setArguments(
         array(
           array(
-            'name'    => 'repositories',
+            'name' => 'all-repositories',
-            'wildcard' => true,
+            'help' => pht('Rebuild identities across all repositories.'),
           ),
           array(
-            'name'     => 'all',
+            'name' => 'all-identities',
-            'help'     => pht('Rebuild identities across all repositories.'),
+            'help' => pht('Rebuild all currently-known identities.'),
+          ),
+          array(
+            'name' => 'repository',
+            'param' => 'repository',
+            'repeat' => true,
+            'help' => pht('Rebuild identities in a repository.'),
+          ),
+          array(
+            'name' => 'commit',
+            'param' => 'commit',
+            'repeat' => true,
+            'help' => pht('Rebuild identities for a commit.'),
+          ),
+          array(
+            'name' => 'user',
+            'param' => 'user',
+            'repeat' => true,
+            'help' => pht('Rebuild identities for a user.'),
+          ),
+          array(
+            'name' => 'email',
+            'param' => 'email',
+            'repeat' => true,
+            'help' => pht('Rebuild identities for an email address.'),
+          ),
+          array(
+            'name' => 'raw',
+            'param' => 'raw',
+            'repeat' => true,
+            'help' => pht('Rebuild identities for a raw commit string.'),
+          ),
+          array(
+            'name' => 'dry-run',
+            'help' => pht('Show changes, but do not make any changes.'),
           ),
         ));
   }
 
   public function execute(PhutilArgumentParser $args) {
-    $console = PhutilConsole::getConsole();
+    $viewer = $this->getViewer();
 
-    $all = $args->getArg('all');
+    $rebuilt_anything = false;
-    $repositories = $args->getArg('repositories');
 
-    if ($all xor empty($repositories)) {
+    $all_repositories = $args->getArg('all-repositories');
+    $repositories = $args->getArg('repository');
+
+    if ($all_repositories && $repositories) {
       throw new PhutilArgumentUsageException(
-        pht('Specify --all or a list of repositories, but not both.'));
+        pht(
+          'Flags "--all-repositories" and "--repository" are not '.
+          'compatible.'));
     }
 
-    $query = id(new DiffusionCommitQuery())
+
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+    $all_identities = $args->getArg('all-identities');
-      ->needCommitData(true);
+    $raw = $args->getArg('raw');
+
+    if ($all_identities && $raw) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Flags "--all-identities" and "--raw" are not '.
+          'compatible.'));
+    }
+
+    $dry_run = $args->getArg('dry-run');
+    $this->dryRun = $dry_run;
+
+    if ($this->dryRun) {
+      $this->logWarn(
+        pht('DRY RUN'),
+        pht('This is a dry run, so no changes will be written.'));
+    }
+
+    if ($all_repositories || $repositories) {
+      $rebuilt_anything = true;
 
       if ($repositories) {
-      $repos = $this->loadRepositories($args, 'repositories');
+        $repository_list = $this->loadRepositories($args, 'repository');
-      $query->withRepositoryIDs(mpull($repos, 'getID'));
+      } else {
+        $repository_query = id(new PhabricatorRepositoryQuery())
+          ->setViewer($viewer);
+        $repository_list = new PhabricatorQueryIterator($repository_query);
       }
 
-    $iterator = new PhabricatorQueryIterator($query);
+      foreach ($repository_list as $repository) {
-    foreach ($iterator as $commit) {
+        $commit_query = id(new DiffusionCommitQuery())
+          ->setViewer($viewer)
+          ->needCommitData(true)
+          ->withRepositoryIDs(array($repository->getID()));
+
+        // See T13457. Adjust ordering to hit keys better and tweak page size
+        // to improve performance slightly, since these records are small.
+        $commit_query->setOrderVector(array('-epoch', '-id'));
+
+        $commit_iterator = id(new PhabricatorQueryIterator($commit_query))
+          ->setPageSize(1000);
+
+        $this->rebuildCommits($commit_iterator);
+      }
+    }
+
+    $commits = $args->getArg('commit');
+    if ($commits) {
+      $rebuilt_anything = true;
+      $commit_list = $this->loadCommits($args, 'commit');
+
+      // Reload commits to get commit data.
+      $commit_list = id(new DiffusionCommitQuery())
+        ->setViewer($viewer)
+        ->needCommitData(true)
+        ->withIDs(mpull($commit_list, 'getID'))
+        ->execute();
+
+      $this->rebuildCommits($commit_list);
+    }
+
+    $users = $args->getArg('user');
+    if ($users) {
+      $rebuilt_anything = true;
+
+      $user_list = $this->loadUsersFromArguments($users);
+      $this->rebuildUsers($user_list);
+    }
+
+    $emails = $args->getArg('email');
+    if ($emails) {
+      $rebuilt_anything = true;
+      $this->rebuildEmails($emails);
+    }
+
+    if ($all_identities || $raw) {
+      $rebuilt_anything = true;
+
+      if ($raw) {
+        $identities = id(new PhabricatorRepositoryIdentityQuery())
+          ->setViewer($viewer)
+          ->withIdentityNames($raw)
+          ->execute();
+
+        $identities = mpull($identities, null, 'getIdentityNameRaw');
+        foreach ($raw as $raw_identity) {
+          if (!isset($identities[$raw_identity])) {
+            throw new PhutilArgumentUsageException(
+              pht(
+                'No identity "%s" exists. When selecting identities with '.
+                '"--raw", the entire identity must match exactly.',
+                $raw_identity));
+          }
+        }
+
+        $identity_list = $identities;
+      } else {
+        $identity_query = id(new PhabricatorRepositoryIdentityQuery())
+          ->setViewer($viewer);
+
+        $identity_list = new PhabricatorQueryIterator($identity_query);
+
+        $this->logInfo(
+          pht('REBUILD'),
+          pht('Rebuilding all existing identities.'));
+      }
+
+      $this->rebuildIdentities($identity_list);
+    }
+
+    if (!$rebuilt_anything) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Nothing specified to rebuild. Use flags to choose which '.
+          'identities to rebuild, or "--help" for help.'));
+    }
+
+    return 0;
+  }
+
+  private function rebuildCommits($commits) {
+    foreach ($commits as $commit) {
       $needs_update = false;
 
       $data = $commit->getCommitData();
@@ -55,6 +210,8 @@ final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
 
       $author_phid = $commit->getAuthorIdentityPHID();
       $identity_phid = $author_identity->getPHID();
+
+      $aidentity_phid = $identity_phid;
       if ($author_phid !== $identity_phid) {
         $commit->setAuthorIdentityPHID($identity_phid);
         $data->setCommitDetail('authorIdentityPHID', $identity_phid);
@@ -81,45 +238,196 @@ final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
       if ($needs_update) {
         $commit->save();
         $data->save();
-        echo tsprintf(
-          "Rebuilt identities for %s.\n",
-          $commit->getDisplayName());
-      } else {
-        echo tsprintf(
-          "No changes for %s.\n",
-          $commit->getDisplayName());
-      }
-    }
 
+        $this->logInfo(
+          pht('COMMIT'),
+          pht(
+            'Rebuilt identities for "%s".',
+            $commit->getDisplayName()));
+      } else {
+        $this->logInfo(
+          pht('SKIP'),
+          pht(
+            'No changes for commit "%s".',
+            $commit->getDisplayName()));
+      }
+    }
   }
 
   private function getIdentityForCommit(
-    PhabricatorRepositoryCommit $commit, $identity_name) {
+    PhabricatorRepositoryCommit $commit,
+    $raw_identity) {
 
-    static $seen = array();
+    if (!isset($this->identityCache[$raw_identity])) {
-    $identity_key = PhabricatorHash::digestForIndex($identity_name);
+      $identity = $this->newIdentityEngine()
-    if (empty($seen[$identity_key])) {
+        ->setSourcePHID($commit->getPHID())
-      try {
+        ->newResolvedIdentity($raw_identity);
-        $user_phid = id(new DiffusionResolveUserQuery())
+
-          ->withCommit($commit)
+      $this->identityCache[$raw_identity] = $identity;
-          ->withName($identity_name)
+    }
+
+    return $this->identityCache[$raw_identity];
+  }
+
+
+  private function rebuildUsers($users) {
+    $viewer = $this->getViewer();
+
+    foreach ($users as $user) {
+      $this->logInfo(
+        pht('USER'),
+        pht(
+          'Rebuilding identities for user "%s".',
+          $user->getMonogram()));
+
+      $emails = id(new PhabricatorUserEmail())->loadAllWhere(
+        'userPHID = %s',
+        $user->getPHID());
+      if ($emails) {
+        $this->rebuildEmails(mpull($emails, 'getAddress'));
+      }
+
+      $identities = id(new PhabricatorRepositoryIdentityQuery())
+        ->setViewer($viewer)
+        ->withRelatedPHIDs(array($user->getPHID()))
         ->execute();
 
-        $identity = id(new PhabricatorRepositoryIdentity())
+      if (!$identities) {
-          ->setAuthorPHID($commit->getPHID())
+        $this->logWarn(
-          ->setIdentityName($identity_name)
+          pht('NO IDENTITIES'),
-          ->setAutomaticGuessedUserPHID($user_phid)
+          pht('Found no identities directly related to user.'));
-          ->save();
+        continue;
-      } catch (AphrontDuplicateKeyQueryException $ex) {
-          // Somehow this identity already exists?
-        $identity = id(new PhabricatorRepositoryIdentityQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withIdentityNames(array($identity_name))
-          ->executeOne();
-      }
-      $seen[$identity_key] = $identity;
       }
 
-    return $seen[$identity_key];
+      $this->rebuildIdentities($identities);
     }
+  }
+
+  private function rebuildEmails($emails) {
+    $viewer = $this->getViewer();
+
+    foreach ($emails as $email) {
+      $this->logInfo(
+        pht('EMAIL'),
+        pht('Rebuilding identities for email address "%s".', $email));
+
+      $identities = id(new PhabricatorRepositoryIdentityQuery())
+        ->setViewer($viewer)
+        ->withEmailAddresses(array($email))
+        ->execute();
+
+      if (!$identities) {
+        $this->logWarn(
+          pht('NO IDENTITIES'),
+          pht('Found no identities for email address "%s".', $email));
+        continue;
+      }
+
+      $this->rebuildIdentities($identities);
+    }
+  }
+
+  private function rebuildIdentities($identities) {
+    $dry_run = $this->dryRun;
+
+    foreach ($identities as $identity) {
+      $raw_identity = $identity->getIdentityName();
+
+      if (isset($this->identityCache[$raw_identity])) {
+        $this->logInfo(
+          pht('SKIP'),
+          pht(
+            'Identity "%s" has already been rebuilt.',
+            $raw_identity));
+        continue;
+      }
+
+      $this->logInfo(
+        pht('IDENTITY'),
+        pht(
+          'Rebuilding identity "%s".',
+          $raw_identity));
+
+      $old_auto = $identity->getAutomaticGuessedUserPHID();
+      $old_assign = $identity->getManuallySetUserPHID();
+
+      $identity = $this->newIdentityEngine()
+        ->newUpdatedIdentity($identity);
+
+      $this->identityCache[$raw_identity] = $identity;
+
+      $new_auto = $identity->getAutomaticGuessedUserPHID();
+      $new_assign = $identity->getManuallySetUserPHID();
+
+      $same_auto = ($old_auto === $new_auto);
+      $same_assign = ($old_assign === $new_assign);
+
+      if ($same_auto && $same_assign) {
+        $this->logInfo(
+          pht('UNCHANGED'),
+          pht('No changes to identity.'));
+      } else {
+        if (!$same_auto) {
+          if ($dry_run) {
+            $this->logWarn(
+              pht('DETECTED PHID'),
+              pht(
+                '(Dry Run) Would update detected user from "%s" to "%s".',
+                $this->renderPHID($old_auto),
+                $this->renderPHID($new_auto)));
+          } else {
+            $this->logWarn(
+              pht('DETECTED PHID'),
+              pht(
+                'Detected user updated from "%s" to "%s".',
+                $this->renderPHID($old_auto),
+                $this->renderPHID($new_auto)));
+          }
+        }
+        if (!$same_assign) {
+          if ($dry_run) {
+            $this->logWarn(
+              pht('ASSIGNED PHID'),
+              pht(
+                '(Dry Run) Would update assigned user from "%s" to "%s".',
+                $this->renderPHID($old_assign),
+                $this->renderPHID($new_assign)));
+          } else {
+            $this->logWarn(
+              pht('ASSIGNED PHID'),
+              pht(
+                'Assigned user updated from "%s" to "%s".',
+                $this->renderPHID($old_assign),
+                $this->renderPHID($new_assign)));
+          }
+        }
+      }
+    }
+  }
+
+  private function renderPHID($phid) {
+    if ($phid == null) {
+      return pht('NULL');
+    }
+
+    if (!isset($this->phidCache[$phid])) {
+      $viewer = $this->getViewer();
+      $handles = $viewer->loadHandles(array($phid));
+      $this->phidCache[$phid] = pht(
+        '%s <%s>',
+        $handles[$phid]->getFullName(),
+        $phid);
+    }
+
+    return $this->phidCache[$phid];
+  }
+
+  private function newIdentityEngine() {
+    $viewer = $this->getViewer();
+
+    return id(new DiffusionRepositoryIdentityEngine())
+      ->setViewer($viewer)
+      ->setDryRun($this->dryRun);
+  }
+
 }

src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php

@@ -6,10 +6,12 @@ final class PhabricatorRepositoryIdentityQuery
   private $ids;
   private $phids;
   private $identityNames;
-  private $emailAddress;
+  private $emailAddresses;
-  private $assigneePHIDs;
+  private $assignedPHIDs;
+  private $effectivePHIDs;
   private $identityNameLike;
   private $hasEffectivePHID;
+  private $relatedPHIDs;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -31,13 +33,23 @@ final class PhabricatorRepositoryIdentityQuery
     return $this;
   }
 
-  public function withEmailAddress($address) {
+  public function withEmailAddresses(array $addresses) {
-    $this->emailAddress = $address;
+    $this->emailAddresses = $addresses;
     return $this;
   }
 
-  public function withAssigneePHIDs(array $assignees) {
+  public function withAssignedPHIDs(array $assigned) {
-    $this->assigneePHIDs = $assignees;
+    $this->assignedPHIDs = $assigned;
+    return $this;
+  }
+
+  public function withEffectivePHIDs(array $effective) {
+    $this->effectivePHIDs = $effective;
+    return $this;
+  }
+
+  public function withRelatedPHIDs(array $related) {
+    $this->relatedPHIDs = $related;
     return $this;
   }
 
@@ -51,7 +63,7 @@ final class PhabricatorRepositoryIdentityQuery
   }
 
   protected function getPrimaryTableAlias() {
-     return 'repository_identity';
+     return 'identity';
    }
 
   protected function loadPage() {
@@ -64,33 +76,40 @@ final class PhabricatorRepositoryIdentityQuery
     if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
-        'repository_identity.id IN (%Ld)',
+        'identity.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
-        'repository_identity.phid IN (%Ls)',
+        'identity.phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->assigneePHIDs !== null) {
+    if ($this->assignedPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'repository_identity.currentEffectiveUserPHID IN (%Ls)',
+        'identity.manuallySetUserPHID IN (%Ls)',
-        $this->assigneePHIDs);
+        $this->assignedPHIDs);
+    }
+
+    if ($this->effectivePHIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'identity.currentEffectiveUserPHID IN (%Ls)',
+        $this->effectivePHIDs);
     }
 
     if ($this->hasEffectivePHID !== null) {
       if ($this->hasEffectivePHID) {
         $where[] = qsprintf(
           $conn,
-          'repository_identity.currentEffectiveUserPHID IS NOT NULL');
+          'identity.currentEffectiveUserPHID IS NOT NULL');
       } else {
         $where[] = qsprintf(
           $conn,
-          'repository_identity.currentEffectiveUserPHID IS NULL');
+          'identity.currentEffectiveUserPHID IS NULL');
       }
     }
 
@@ -102,25 +121,35 @@ final class PhabricatorRepositoryIdentityQuery
 
       $where[] = qsprintf(
         $conn,
-        'repository_identity.identityNameHash IN (%Ls)',
+        'identity.identityNameHash IN (%Ls)',
         $name_hashes);
     }
 
-    if ($this->emailAddress !== null) {
+    if ($this->emailAddresses !== null) {
-      $identity_style = "<{$this->emailAddress}>";
       $where[] = qsprintf(
         $conn,
-        'repository_identity.identityNameRaw LIKE %<',
+        'identity.emailAddress IN (%Ls)',
-        $identity_style);
+        $this->emailAddresses);
     }
 
     if ($this->identityNameLike != null) {
       $where[] = qsprintf(
         $conn,
-        'repository_identity.identityNameRaw LIKE %~',
+        'identity.identityNameRaw LIKE %~',
         $this->identityNameLike);
     }
 
+    if ($this->relatedPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        '(identity.manuallySetUserPHID IN (%Ls) OR
+          identity.currentEffectiveUserPHID IN (%Ls) OR
+          identity.automaticGuessedUserPHID IN (%Ls))',
+        $this->relatedPHIDs,
+        $this->relatedPHIDs,
+        $this->relatedPHIDs);
+    }
+
     return $where;
   }
 

src/applications/repository/storage/PhabricatorRepositoryIdentity.php

@@ -13,6 +13,7 @@ final class PhabricatorRepositoryIdentity
   protected $automaticGuessedUserPHID;
   protected $manuallySetUserPHID;
   protected $currentEffectiveUserPHID;
+  protected $emailAddress;
 
   protected function getConfiguration() {
     return array(
@@ -26,12 +27,16 @@ final class PhabricatorRepositoryIdentity
         'automaticGuessedUserPHID' => 'phid?',
         'manuallySetUserPHID' => 'phid?',
         'currentEffectiveUserPHID' => 'phid?',
+        'emailAddress' => 'sort255?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_identity' => array(
           'columns' => array('identityNameHash'),
           'unique' => true,
         ),
+        'key_email' => array(
+          'columns' => array('emailAddress(64)'),
+        ),
       ),
     ) + parent::getConfiguration();
   }
@@ -69,6 +74,10 @@ final class PhabricatorRepositoryIdentity
     return $this->getIdentityName();
   }
 
+  public function getObjectName() {
+    return pht('Identity %d', $this->getID());
+  }
+
   public function getURI() {
     return '/diffusion/identity/view/'.$this->getID().'/';
   }
@@ -87,10 +96,36 @@ final class PhabricatorRepositoryIdentity
 
   public function save() {
     if ($this->manuallySetUserPHID) {
-      $this->currentEffectiveUserPHID = $this->manuallySetUserPHID;
+      $unassigned = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
+      if ($this->manuallySetUserPHID === $unassigned) {
+        $effective_phid = null;
       } else {
-      $this->currentEffectiveUserPHID = $this->automaticGuessedUserPHID;
+        $effective_phid = $this->manuallySetUserPHID;
       }
+    } else {
+      $effective_phid = $this->automaticGuessedUserPHID;
+    }
+
+    $this->setCurrentEffectiveUserPHID($effective_phid);
+
+    $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();
   }
@@ -111,7 +146,8 @@ final class PhabricatorRepositoryIdentity
   }
 
   public function hasAutomaticCapability(
-    $capability, PhabricatorUser $viewer) {
+    $capability,
+    PhabricatorUser $viewer) {
     return false;
   }
 

src/applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php

@@ -1,33 +1,59 @@
 <?php
 
 final class PhabricatorRepositoryIdentityChangeWorker
-extends PhabricatorWorker {
+  extends PhabricatorWorker {
 
   protected function doWork() {
     $viewer = PhabricatorUser::getOmnipotentUser();
 
-    $task_data = $this->getTaskData();
+    $related_phids = $this->getTaskDataValue('relatedPHIDs');
-    $user_phid = idx($task_data, 'userPHID');
+    $email_addresses = $this->getTaskDataValue('emailAddresses');
 
-    $user = id(new PhabricatorPeopleQuery())
+    // Retain backward compatibility with older tasks which may still be in
-      ->withPHIDs(array($user_phid))
+    // queue. Previously, this worker accepted a single "userPHID". See
-      ->setViewer($viewer)
+    // T13444. This can be removed in some future version of Phabricator once
-      ->executeOne();
+    // these tasks have likely flushed out of queue.
+    $legacy_phid = $this->getTaskDataValue('userPHID');
+    if ($legacy_phid) {
+      if (!is_array($related_phids)) {
+        $related_phids = array();
+      }
+      $related_phids[] = $legacy_phid;
+    }
 
-    $emails = id(new PhabricatorUserEmail())->loadAllWhere(
+    // Note that we may arrive in this worker after the associated objects
-      'userPHID = %s ORDER BY address',
+    // have already been destroyed, so we can't (and shouldn't) verify that
-      $user->getPHID());
+    // PHIDs correspond to real objects. If you "bin/remove destroy" a user,
+    // we'll end up here with a now-bogus user PHID that we should
+    // disassociate from identities.
 
-    foreach ($emails as $email) {
+    $identity_map = array();
+
+    if ($related_phids) {
       $identities = id(new PhabricatorRepositoryIdentityQuery())
         ->setViewer($viewer)
-        ->withEmailAddress($email->getAddress())
+        ->withRelatedPHIDs($related_phids)
         ->execute();
-
+      $identity_map += mpull($identities, null, 'getPHID');
-      foreach ($identities as $identity) {
-        $identity->setAutomaticGuessedUserPHID($user->getPHID())
-          ->save();
     }
+
+    if ($email_addresses) {
+      $identities = id(new PhabricatorRepositoryIdentityQuery())
+        ->setViewer($viewer)
+        ->withEmailAddresses($email_addresses)
+        ->execute();
+      $identity_map += mpull($identities, null, 'getPHID');
+    }
+
+    // If we didn't find any related identities, we're all set.
+    if (!$identity_map) {
+      return;
+    }
+
+    $identity_engine = id(new DiffusionRepositoryIdentityEngine())
+      ->setViewer($viewer);
+    foreach ($identity_map as $identity) {
+      $identity_engine->newUpdatedIdentity($identity);
     }
   }
 

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -65,37 +65,20 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     $message = $ref->getMessage();
     $committer = $ref->getCommitter();
     $hashes = $ref->getHashes();
+    $has_committer = (bool)strlen($committer);
 
-    $author_identity = id(new PhabricatorRepositoryIdentityQuery())
+    $viewer = PhabricatorUser::getOmnipotentUser();
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withIdentityNames(array($author))
-      ->executeOne();
 
-    if (!$author_identity) {
+    $identity_engine = id(new DiffusionRepositoryIdentityEngine())
-      $author_identity = id(new PhabricatorRepositoryIdentity())
+      ->setViewer($viewer)
-        ->setAuthorPHID($commit->getPHID())
+      ->setSourcePHID($commit->getPHID());
-        ->setIdentityName($author)
-        ->setAutomaticGuessedUserPHID(
-          $this->resolveUserPHID($commit, $author))
-        ->save();
-    }
 
+    $author_identity = $identity_engine->newResolvedIdentity($author);
+
+    if ($has_committer) {
+      $committer_identity = $identity_engine->newResolvedIdentity($committer);
+    } else {
       $committer_identity = null;
-
-    if ($committer) {
-      $committer_identity = id(new PhabricatorRepositoryIdentityQuery())
-        ->setViewer(PhabricatorUser::getOmnipotentUser())
-        ->withIdentityNames(array($committer))
-        ->executeOne();
-
-      if (!$committer_identity) {
-        $committer_identity = id(new PhabricatorRepositoryIdentity())
-          ->setAuthorPHID($commit->getPHID())
-          ->setIdentityName($committer)
-          ->setAutomaticGuessedUserPHID(
-            $this->resolveUserPHID($commit, $committer))
-          ->save();
-      }
     }
 
     $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
@@ -117,11 +100,11 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       'authorIdentityPHID', $author_identity->getPHID());
     $data->setCommitDetail(
       'authorPHID',
-      $this->resolveUserPHID($commit, $author));
+      $author_identity->getCurrentEffectiveUserPHID());
 
     $data->setCommitMessage($message);
 
-    if (strlen($committer)) {
+    if ($has_committer) {
       $data->setCommitDetail('committer', $committer);
 
       $data->setCommitDetail('committerName', $ref->getCommitterName());
@@ -129,7 +112,8 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
 
       $data->setCommitDetail(
         'committerPHID',
-        $this->resolveUserPHID($commit, $committer));
+        $committer_identity->getCurrentEffectiveUserPHID());
+
       $data->setCommitDetail(
         'committerIdentityPHID', $committer_identity->getPHID());
 
@@ -177,16 +161,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       PhabricatorRepositoryCommit::IMPORTED_MESSAGE);
   }
 
-  private function resolveUserPHID(
-    PhabricatorRepositoryCommit $commit,
-    $user_name) {
-
-    return id(new DiffusionResolveUserQuery())
-      ->withCommit($commit)
-      ->withName($user_name)
-      ->execute();
-  }
-
   private function closeRevisions(
     PhabricatorUser $actor,
     DiffusionCommitRef $ref,

src/applications/system/engine/PhabricatorDestructionEngine.php

@@ -5,6 +5,9 @@ final class PhabricatorDestructionEngine extends Phobject {
   private $rootLogID;
   private $collectNotes;
   private $notes = array();
+  private $depth = 0;
+  private $destroyedObjects = array();
+  private $waitToFinalizeDestruction = false;
 
   public function setCollectNotes($collect_notes) {
     $this->collectNotes = $collect_notes;
@@ -19,9 +22,20 @@ final class PhabricatorDestructionEngine extends Phobject {
     return PhabricatorUser::getOmnipotentUser();
   }
 
+  public function setWaitToFinalizeDestruction($wait) {
+    $this->waitToFinalizeDestruction = $wait;
+    return $this;
+  }
+
+  public function getWaitToFinalizeDestruction() {
+    return $this->waitToFinalizeDestruction;
+  }
+
   public function destroyObject(PhabricatorDestructibleInterface $object) {
+    $this->depth++;
+
     $log = id(new PhabricatorSystemDestructionLog())
-      ->setEpoch(time())
+      ->setEpoch(PhabricatorTime::getNow())
       ->setObjectClass(get_class($object));
 
     if ($this->rootLogID) {
@@ -73,7 +87,42 @@ final class PhabricatorDestructionEngine extends Phobject {
       foreach ($extensions as $key => $extension) {
         $extension->destroyObject($this, $object);
       }
+
+      $this->destroyedObjects[] = $object;
     }
+
+    $this->depth--;
+
+    // If this is a root-level invocation of "destroyObject()", flush the
+    // queue of destroyed objects and fire "didDestroyObject()" hooks. This
+    // hook allows extensions to do things like queue cache updates which
+    // might race if we fire them during object destruction.
+
+    if (!$this->depth) {
+      if (!$this->getWaitToFinalizeDestruction()) {
+        $this->finalizeDestruction();
+      }
+    }
+
+    return $this;
+  }
+
+  public function finalizeDestruction() {
+    $extensions = PhabricatorDestructionEngineExtension::getAllExtensions();
+
+    foreach ($this->destroyedObjects as $object) {
+      foreach ($extensions as $extension) {
+        if (!$extension->canDestroyObject($this, $object)) {
+          continue;
+        }
+
+        $extension->didDestroyObject($this, $object);
+      }
+    }
+
+    $this->destroyedObjects = array();
+
+    return $this;
   }
 
   private function getObjectPHID($object) {

src/applications/system/engine/PhabricatorDestructionEngineExtension.php

@@ -14,9 +14,17 @@ abstract class PhabricatorDestructionEngineExtension extends Phobject {
     return true;
   }
 
-  abstract public function destroyObject(
+  public function destroyObject(
     PhabricatorDestructionEngine $engine,
-    $object);
+    $object) {
+    return null;
+  }
+
+  public function didDestroyObject(
+    PhabricatorDestructionEngine $engine,
+    $object) {
+    return null;
+  }
 
   final public static function getAllExtensions() {
     return id(new PhutilClassMapQuery())

src/docs/user/userguide/events.diviner

@@ -159,35 +159,6 @@ will be available yet. Data available on this event:
   - `repository` The @{class:PhabricatorRepository} the commit was discovered
     in.
 
-== Diffusion: Lookup User ==
-
-The constant for this event is
-`PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER`.
-
-This event is dispatched when the daemons are trying to link a commit to a
-Phabricator user account. You can listen for it to improve the accuracy of
-associating users with their commits.
-
-By default, Phabricator will try to find matches based on usernames, real names,
-or email addresses, but this can result in incorrect matches (e.g., if you have
-several employees with the same name) or failures to match (e.g., if someone
-changed their email address). Listening for this event allows you to intercept
-the lookup and supplement the results from another datasource.
-
-Data available on this event:
-
-  - `commit` The @{class:PhabricatorRepositoryCommit} that data is being looked
-    up for.
-  - `query` The author or committer string being looked up. This will usually
-    be something like "Abraham Lincoln <alincoln@logcabin.example.com>", but
-    comes from the commit metadata so it may not be well-formatted.
-  - `result` The current result from the lookup (Phabricator's best guess at
-    the user PHID of the user named in the "query"). To substitute the result
-    with a different result, replace this with the correct PHID in your event
-    listener.
-
-Using @{class@libphutil:PhutilEmailAddress} may be helpful in parsing the query.
-
 == Test: Did Run Test ==
 
 The constant for this event is

src/infrastructure/events/constant/PhabricatorEventType.php

@@ -9,7 +9,6 @@ final class PhabricatorEventType extends PhutilEventType {
   const TYPE_DIFFERENTIAL_WILLMARKGENERATED = 'differential.willMarkGenerated';
 
   const TYPE_DIFFUSION_DIDDISCOVERCOMMIT    = 'diffusion.didDiscoverCommit';
-  const TYPE_DIFFUSION_LOOKUPUSER           = 'diffusion.lookupUser';
 
   const TYPE_TEST_DIDRUNTEST                = 'test.didRunTest';
 

src/infrastructure/management/PhabricatorManagementWorkflow.php

@@ -67,4 +67,125 @@ abstract class PhabricatorManagementWorkflow extends PhutilArgumentWorkflow {
     fprintf(STDERR, '%s', $message);
   }
 
+  final protected function loadUsersFromArguments(array $identifiers) {
+    if (!$identifiers) {
+      return array();
+    }
+
+    $ids = array();
+    $phids = array();
+    $usernames = array();
+
+    $user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
+
+    foreach ($identifiers as $identifier) {
+      // If the value is a user PHID, treat as a PHID.
+      if (phid_get_type($identifier) === $user_type) {
+        $phids[$identifier] = $identifier;
+        continue;
+      }
+
+      // If the value is "@..." and then some text, treat it as a username.
+      if ((strlen($identifier) > 1) && ($identifier[0] == '@')) {
+        $usernames[$identifier] = substr($identifier, 1);
+        continue;
+      }
+
+      // If the value is digits, treat it as both an ID and a username.
+      // Entirely numeric usernames, like "1234", are valid.
+      if (ctype_digit($identifier)) {
+        $ids[$identifier] = $identifier;
+        $usernames[$identifier] = $identifier;
+        continue;
+      }
+
+      // Otherwise, treat it as an unescaped username.
+      $usernames[$identifier] = $identifier;
+    }
+
+    $viewer = $this->getViewer();
+    $results = array();
+
+    if ($phids) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withPHIDs($phids)
+        ->execute();
+      foreach ($users as $user) {
+        $phid = $user->getPHID();
+        $results[$phid][] = $user;
+      }
+    }
+
+    if ($usernames) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withUsernames($usernames)
+        ->execute();
+
+      $reverse_map = array();
+      foreach ($usernames as $identifier => $username) {
+        $username = phutil_utf8_strtolower($username);
+        $reverse_map[$username][] = $identifier;
+      }
+
+      foreach ($users as $user) {
+        $username = $user->getUsername();
+        $username = phutil_utf8_strtolower($username);
+
+        $reverse_identifiers = idx($reverse_map, $username, array());
+
+        if (count($reverse_identifiers) > 1) {
+          throw new PhutilArgumentUsageException(
+            pht(
+              'Multiple user identifiers (%s) correspond to the same user. '.
+              'Identify each user exactly once.',
+              implode(', ', $reverse_identifiers)));
+        }
+
+        foreach ($reverse_identifiers as $reverse_identifier) {
+          $results[$reverse_identifier][] = $user;
+        }
+      }
+    }
+
+    if ($ids) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withIDs($ids)
+        ->execute();
+
+      foreach ($users as $user) {
+        $id = $user->getID();
+        $results[$id][] = $user;
+      }
+    }
+
+    $list = array();
+    foreach ($identifiers as $identifier) {
+      $users = idx($results, $identifier, array());
+      if (!$users) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'No user "%s" exists. Specify users by username, ID, or PHID.',
+            $identifier));
+      }
+
+      if (count($users) > 1) {
+        // This can happen if you have a user "@25", a user with ID 25, and
+        // specify "--user 25". You can disambiguate this by specifying
+        // "--user @25".
+        throw new PhutilArgumentUsageException(
+          pht(
+            'Identifier "%s" matches multiple users. Specify each user '.
+            'unambiguously with "@username" or by using user PHIDs.',
+            $identifier));
+      }
+
+      $list[] = head($users);
+    }
+
+    return $list;
+  }
+
 }

src/infrastructure/storage/lisk/PhabricatorQueryIterator.php

@@ -10,7 +10,12 @@ final class PhabricatorQueryIterator extends PhutilBufferedIterator {
   }
 
   protected function didRewind() {
-    $this->pager = new AphrontCursorPagerView();
+    $pager = new AphrontCursorPagerView();
+
+    $page_size = $this->getPageSize();
+    $pager->setPageSize($page_size);
+
+    $this->pager = $pager;
   }
 
   public function key() {