Use loadRelatives() in loadPrimaryEmail()

Summary: This is an example of code simplification with D2557.

Test Plan: Display user list, verify the SQL queries.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Koolvin

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

src/__phutil_library_map__.php

@@ -471,6 +471,7 @@ phutil_register_library_map(array(
     'JavelinViewExample' => 'applications/uiexample/examples/client',
     'JavelinViewExampleServerView' => 'applications/uiexample/examples/client',
     'LiskDAO' => 'storage/lisk/dao',
+    'LiskDAOSet' => 'storage/lisk/dao',
     'LiskEphemeralObjectException' => 'storage/lisk/dao',
     'LiskFixtureTestCase' => 'storage/lisk/dao/__tests__',
     'LiskIsolationTestCase' => 'storage/lisk/dao/__tests__',

src/applications/people/controller/list/PhabricatorPeopleListController.php

@@ -43,7 +43,8 @@ final class PhabricatorPeopleListController
 
     $rows = array();
     foreach ($users as $user) {
-      if ($user->getPrimaryEmail()->getIsVerified()) {
+      $primary_email = $user->loadPrimaryEmail();
+      if ($primary_email && $primary_email->getIsVerified()) {
         $email = 'Verified';
       } else {
         $email = 'Unverified';

src/applications/people/query/PhabricatorPeopleQuery.php

@@ -67,23 +67,11 @@ final class PhabricatorPeopleQuery extends PhabricatorOffsetPagedQuery {
       $where_clause,
       $limit_clause);
 
+    if ($this->needPrimaryEmail) {
+      $table->putInSet(new LiskDAOSet());
+    }
+
     $users = $table->loadAllFromArray($data);
-
-    if ($users && $this->needPrimaryEmail) {
-      $emails = id(new PhabricatorUserEmail())->loadAllWhere(
-        'userPHID IN (%Ls)',
-        mpull($users, 'getPHID'));
-      $emails = mpull($emails, null, 'getUserPHID');
-      foreach ($users as $user) {
-        // Fail gracefully if we have data integrity problems.
-        if (empty($emails[$user->getPHID()])) {
-          $user->attachPrimaryEmail(new PhabricatorUserEmail());
-        } else {
-          $user->attachPrimaryEmail($emails[$user->getPHID()]);
-        }
-      }
-    }
-
     return $users;
   }
 

src/applications/people/query/__init__.php

@@ -9,10 +9,9 @@
 phutil_require_module('phabricator', 'applications/people/storage/email');
 phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/query/offsetpaged');
+phutil_require_module('phabricator', 'storage/lisk/dao');
 phutil_require_module('phabricator', 'storage/qsprintf');
 phutil_require_module('phabricator', 'storage/queryfx');
 
-phutil_require_module('phutil', 'utils');
-
 
 phutil_require_source('PhabricatorPeopleQuery.php');

src/applications/people/storage/user/PhabricatorUser.php

@@ -41,7 +41,6 @@ final class PhabricatorUser extends PhabricatorUserDAO {
   protected $isDisabled = 0;
 
   private $preferences = null;
-  private $primaryEmail;
 
   protected function readField($field) {
     switch ($field) {
@@ -413,23 +412,11 @@ final class PhabricatorUser extends PhabricatorUserDAO {
   }
 
   public function loadPrimaryEmail() {
-    return id(new PhabricatorUserEmail())->loadOneWhere(
+    return $this->loadOneRelative(
-      'userPHID = %s AND isPrimary = %d',
+      new PhabricatorUserEmail(),
-      $this->getPHID(),
+      'userPHID',
-      1);
+      'getPHID',
-  }
+      '(isPrimary = 1)');
-
-  public function attachPrimaryEmail(PhabricatorUserEmail $email) {
-    $this->primaryEmail = $email;
-    return $this;
-  }
-
-  public function getPrimaryEmail() {
-    if ($this->primaryEmail === null) {
-      throw new Exception(
-        "Call attachPrimaryEmail() before getPrimaryEmail()!");
-    }
-    return $this->primaryEmail;
   }
 
   public function loadPreferences() {

src/storage/lisk/dao/LiskDAO.php

@@ -772,13 +772,13 @@ abstract class LiskDAO {
    * a homework.
    *
    * The method also supports retrieving referenced objects, for example authors
-   * of all diffs:
+   * of all diffs (using shortcut @{method:loadOneRelative}):
    *
    *   foreach ($diffs as $diff) {
-   *     $author = head($diff->loadRelatives(
+   *     $author = $diff->loadOneRelative(
    *       new PhabricatorUser(),
    *       'phid',
-   *       'getAuthorPHID'));
+   *       'getAuthorPHID');
    *     // Do something with author.
    *   }
    *
@@ -789,11 +789,11 @@ abstract class LiskDAO {
    * meaning). This is intentional to avoid mistakes with using data from one
    * row in retrieving other rows. Example of a correct usage:
    *
-   *   $status = head($author->loadRelatives(
+   *   $status = $author->loadOneRelative(
    *     new PhabricatorUserStatus(),
    *     'userPHID',
    *     'getPHID',
-   *     '(UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo)'));
+   *     '(UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo)');
    *
    * @param  LiskDAO  Type of objects to load.
    * @param  string   Name of the column in target table.
@@ -822,6 +822,43 @@ abstract class LiskDAO {
     return idx($relatives, $this->$key_method(), array());
   }
 
+  /**
+   * Load referenced row. See @{method:loadRelatives} for details.
+   *
+   * @param  LiskDAO  Type of objects to load.
+   * @param  string   Name of the column in target table.
+   * @param  string   Method name in this table.
+   * @param  string   Additional constraints on returned rows. It supports no
+   *                  placeholders and requires putting the WHERE part into
+   *                  parentheses. It's not possible to use LIMIT.
+   * @return LiskDAO  Object of type $object or null if there's no such object.
+   *
+   * @task   load
+   */
+  final public function loadOneRelative(
+    LiskDAO $object,
+    $foreign_column,
+    $key_method = 'getID',
+    $where = '') {
+
+    $relatives = $this->loadRelatives(
+      $object,
+      $foreign_column,
+      $key_method,
+      $where);
+
+    if (!$relatives) {
+      return null;
+    }
+
+    if (count($relatives) > 1) {
+      throw new AphrontQueryCountException(
+        "More than 1 result from loadOneRelative()!");
+    }
+
+    return reset($relatives);
+  }
+
   final public function putInSet(LiskDAOSet $set) {
     $this->inSet = $set;
     return $this;