From d1225e782b0e9a94406121fb0e793e3dbab302d7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 8 Sep 2013 09:43:27 -0700 Subject: [PATCH] Don't try to load user profile images in PhabricatorPeopleQuery if no users have any Summary: Fixes T3810. In PhabricatorPeopleQuery, we issue an unnecessary query like this: SELECT f.* FROM file f WHERE (f.phid IN ('')) ORDER BY f.id DESC ...if we're loading a user without a profile picture. Filter the file PHIDs before loading them to prevent this. This doesn't change anything, but saves us a spurious/silly query. Also makes `PhabricatorPeopleProfileController` use `needProfileImage()`, moving us closer to getting rid of `loadProfileImageURI()` eventually. Test Plan: Looked at profiles of users with and without profile pictures. Checked query log in DarkConsole. Reviewers: chad, btrahan Reviewed By: chad CC: aran Maniphest Tasks: T3810 Differential Revision: https://secure.phabricator.com/D6913 --- .../PhabricatorPeopleProfileController.php | 1 + .../people/query/PhabricatorPeopleQuery.php | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 97dd6bfb35..3a946996cd 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -19,6 +19,7 @@ final class PhabricatorPeopleProfileController $user = id(new PhabricatorPeopleQuery()) ->setViewer($viewer) ->withUsernames(array($this->username)) + ->needProfileImage(true) ->executeOne(); if (!$user) { return new Aphront404Response(); diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index a4e73d8b16..afb5a67542 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -133,11 +133,17 @@ final class PhabricatorPeopleQuery } if ($this->needProfileImage) { - $files = id(new PhabricatorFileQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs(mpull($users, 'getProfileImagePHID')) - ->execute(); - $files = mpull($files, null, 'getPHID'); + $user_profile_file_phids = mpull($users, 'getProfileImagePHID'); + $user_profile_file_phids = array_filter($user_profile_file_phids); + if ($user_profile_file_phids) { + $files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($user_profile_file_phids) + ->execute(); + $files = mpull($files, null, 'getPHID'); + } else { + $files = array(); + } foreach ($users as $user) { $image_phid = $user->getProfileImagePHID(); if (isset($files[$image_phid])) {