Page MenuHomePhabricator
Files F14047569

D8445.id20040.diff
No OneTemporary
Actions

D8445.id20040.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1770,6 +1770,8 @@
'PhabricatorObjectHandle' => 'applications/phid/PhabricatorObjectHandle.php',
'PhabricatorObjectHandleConstants' => 'applications/phid/handle/const/PhabricatorObjectHandleConstants.php',
'PhabricatorObjectHandleStatus' => 'applications/phid/handle/const/PhabricatorObjectHandleStatus.php',
+ 'PhabricatorObjectListQuery' => 'applications/phid/query/PhabricatorObjectListQuery.php',
+ 'PhabricatorObjectListQueryTestCase' => 'applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php',
'PhabricatorObjectListView' => 'view/control/PhabricatorObjectListView.php',
'PhabricatorObjectMailReceiver' => 'applications/metamta/receiver/PhabricatorObjectMailReceiver.php',
'PhabricatorObjectMailReceiverTestCase' => 'applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php',
@@ -4543,6 +4545,7 @@
'PhabricatorOAuthServerTokenController' => 'PhabricatorAuthController',
'PhabricatorObjectHandle' => 'PhabricatorPolicyInterface',
'PhabricatorObjectHandleStatus' => 'PhabricatorObjectHandleConstants',
+ 'PhabricatorObjectListQueryTestCase' => 'PhabricatorTestCase',
'PhabricatorObjectListView' => 'AphrontView',
'PhabricatorObjectMailReceiver' => 'PhabricatorMailReceiver',
'PhabricatorObjectMailReceiverTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/differential/field/specification/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFieldSpecification.php
--- a/src/applications/differential/field/specification/DifferentialFieldSpecification.php
+++ b/src/applications/differential/field/specification/DifferentialFieldSpecification.php
@@ -783,8 +783,7 @@
return $this->parseCommitMessageObjectList(
$value,
$mailables = false,
- $allow_partial = false,
- $projects = true);
+ $allow_partial = false);
}
/**
@@ -814,89 +813,21 @@
$include_mailables,
$allow_partial = false) {
- $value = array_unique(array_filter(preg_split('/[\s,]+/', $value)));
- if (!$value) {
- return array();
- }
-
- $object_map = array();
-
- $project_names = array();
- $other_names = array();
- foreach ($value as $item) {
- if (preg_match('/^#/', $item)) {
- $project_names[$item] = ltrim(phutil_utf8_strtolower($item), '#').'/';
- } else {
- $other_names[] = $item;
- }
- }
-
- if ($project_names) {
- // TODO: (T603) This should probably be policy-aware, although maybe not,
- // since we generally don't want to destroy data and it doesn't leak
- // anything?
- $projects = id(new PhabricatorProjectQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPhrictionSlugs($project_names)
- ->execute();
-
- $reverse_map = array_flip($project_names);
- foreach ($projects as $project) {
- $reverse_key = $project->getPhrictionSlug();
- if (isset($reverse_map[$reverse_key])) {
- $object_map[$reverse_map[$reverse_key]] = $project->getPHID();
- }
- }
- }
-
- if ($other_names) {
- $users = id(new PhabricatorUser())->loadAllWhere(
- '(username IN (%Ls))',
- $other_names);
-
- $user_map = mpull($users, 'getPHID', 'getUsername');
- foreach ($user_map as $username => $phid) {
- // Usernames may have uppercase letters in them. Put both names in the
- // map so we can try the original case first, so that username *always*
- // works in weird edge cases where some other mailable object collides.
- $object_map[$username] = $phid;
- $object_map[strtolower($username)] = $phid;
- }
-
- if ($include_mailables) {
- $mailables = id(new PhabricatorMetaMTAMailingList())->loadAllWhere(
- '(email IN (%Ls)) OR (name IN (%Ls))',
- $other_names,
- $other_names);
- $object_map += mpull($mailables, 'getPHID', 'getName');
- $object_map += mpull($mailables, 'getPHID', 'getEmail');
- }
- }
-
- $invalid = array();
- $results = array();
- foreach ($value as $name) {
- if (empty($object_map[$name])) {
- if (empty($object_map[phutil_utf8_strtolower($name)])) {
- $invalid[] = $name;
- } else {
- $results[] = $object_map[phutil_utf8_strtolower($name)];
- }
- } else {
- $results[] = $object_map[$name];
- }
- }
+ $types = array(
+ PhabricatorPeoplePHIDTypeUser::TYPECONST,
+ PhabricatorProjectPHIDTypeProject::TYPECONST,
+ );
- if ($invalid && !$allow_partial) {
- $invalid = implode(', ', $invalid);
- $what = $include_mailables
- ? "users and mailing lists"
- : "users";
- throw new DifferentialFieldParseException(
- "Commit message references nonexistent {$what}: {$invalid}.");
+ if ($include_mailables) {
+ $types[] = PhabricatorMailingListPHIDTypeList::TYPECONST;
}
- return array_unique($results);
+ return id(new PhabricatorObjectListQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->setAllowPartialResults($allow_partial)
+ ->setAllowedTypes($types)
+ ->setObjectList($value)
+ ->execute();
}
diff --git a/src/applications/mailinglists/phid/PhabricatorMailingListPHIDTypeList.php b/src/applications/mailinglists/phid/PhabricatorMailingListPHIDTypeList.php
--- a/src/applications/mailinglists/phid/PhabricatorMailingListPHIDTypeList.php
+++ b/src/applications/mailinglists/phid/PhabricatorMailingListPHIDTypeList.php
@@ -37,4 +37,35 @@
}
}
+ public function canLoadNamedObject($name) {
+ return preg_match('/^.+@.+/', $name);
+ }
+
+ public function loadNamedObjects(
+ PhabricatorObjectQuery $query,
+ array $names) {
+
+ $id_map = array();
+ foreach ($names as $name) {
+ // Maybe normalize these some day?
+ $id = $name;
+ $id_map[$id][] = $name;
+ }
+
+ $objects = id(new PhabricatorMailingListQuery())
+ ->setViewer($query->getViewer())
+ ->withEmails(array_keys($id_map))
+ ->execute();
+
+ $results = array();
+ foreach ($objects as $id => $object) {
+ $email = $object->getEmail();
+ foreach (idx($id_map, $email, array()) as $name) {
+ $results[$name] = $object;
+ }
+ }
+
+ return $results;
+ }
+
}
diff --git a/src/applications/mailinglists/query/PhabricatorMailingListQuery.php b/src/applications/mailinglists/query/PhabricatorMailingListQuery.php
--- a/src/applications/mailinglists/query/PhabricatorMailingListQuery.php
+++ b/src/applications/mailinglists/query/PhabricatorMailingListQuery.php
@@ -5,6 +5,8 @@
private $phids;
private $ids;
+ private $emails;
+ private $names;
public function withIDs($ids) {
$this->ids = $ids;
@@ -16,6 +18,16 @@
return $this;
}
+ public function withEmails(array $emails) {
+ $this->emails = $emails;
+ return $this;
+ }
+
+ public function withNames(array $names) {
+ $this->names = $names;
+ return $this;
+ }
+
public function loadPage() {
$table = new PhabricatorMetaMTAMailingList();
$conn_r = $table->establishConnection('r');
@@ -48,6 +60,20 @@
$this->phids);
}
+ if ($this->names) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'name IN (%Ls)',
+ $this->names);
+ }
+
+ if ($this->emails) {
+ $where[] = qsprintf(
+ $conn_r,
+ 'email IN (%Ls)',
+ $this->emails);
+ }
+
$where[] = $this->buildPagingClause($conn_r);
return $this->formatWhereClause($where);
diff --git a/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php b/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php
--- a/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php
+++ b/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php
@@ -52,4 +52,34 @@
}
+ public function canLoadNamedObject($name) {
+ return preg_match('/^@.+/', $name);
+ }
+
+ public function loadNamedObjects(
+ PhabricatorObjectQuery $query,
+ array $names) {
+
+ $id_map = array();
+ foreach ($names as $name) {
+ $id = substr($name, 1);
+ $id_map[$id][] = $name;
+ }
+
+ $objects = id(new PhabricatorPeopleQuery())
+ ->setViewer($query->getViewer())
+ ->withUsernames(array_keys($id_map))
+ ->execute();
+
+ $results = array();
+ foreach ($objects as $id => $object) {
+ $username = $object->getUsername();
+ foreach (idx($id_map, $username, array()) as $name) {
+ $results[$name] = $object;
+ }
+ }
+
+ return $results;
+ }
+
}
diff --git a/src/applications/phid/query/PhabricatorObjectListQuery.php b/src/applications/phid/query/PhabricatorObjectListQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/phid/query/PhabricatorObjectListQuery.php
@@ -0,0 +1,171 @@
+<?php
+
+final class PhabricatorObjectListQuery {
+
+ private $viewer;
+ private $objectList;
+ private $allowedTypes = array();
+ private $allowPartialResults;
+
+ public function setAllowPartialResults($allow_partial_results) {
+ $this->allowPartialResults = $allow_partial_results;
+ return $this;
+ }
+
+ public function getAllowPartialResults() {
+ return $this->allowPartialResults;
+ }
+
+ public function setAllowedTypes(array $allowed_types) {
+ $this->allowedTypes = $allowed_types;
+ return $this;
+ }
+
+ public function getAllowedTypes() {
+ return $this->allowedTypes;
+ }
+
+ public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
+ return $this;
+ }
+
+ public function getViewer() {
+ return $this->viewer;
+ }
+
+ public function setObjectList($object_list) {
+ $this->objectList = $object_list;
+ return $this;
+ }
+
+ public function getObjectList() {
+ return $this->objectList;
+ }
+
+ public function execute() {
+ $names = $this->getObjectList();
+ $names = array_unique(array_filter(preg_split('/[\s,]+/', $names)));
+
+ $objects = $this->loadObjects($names);
+
+ $types = array();
+ foreach ($objects as $name => $object) {
+ $types[phid_get_type($object->getPHID())][] = $name;
+ }
+
+ $invalid = array();
+ if ($this->getAllowedTypes()) {
+ $allowed = array_fuse($this->getAllowedTypes());
+ foreach ($types as $type => $names_of_type) {
+ if (empty($allowed[$type])) {
+ $invalid[] = $names_of_type;
+ }
+ }
+ }
+ $invalid = array_mergev($invalid);
+
+ $missing = array();
+ foreach ($names as $name) {
+ if (empty($objects[$name])) {
+ $missing[] = $name;
+ }
+ }
+
+ // NOTE: We could couple this less tightly with Differential, but it is
+ // currently the only thing that uses it, and we'd have to add a lot of
+ // extra API to loosen this. It's not clear that this will be useful
+ // elsewhere any time soon, so let's cross that bridge when we come to it.
+
+ if (!$this->getAllowPartialResults()) {
+ if ($invalid && $missing) {
+ throw new DifferentialFieldParseException(
+ pht(
+ 'The objects you have listed include objects of the wrong '.
+ 'type (%s) and objects which do not exist (%s).',
+ implode(', ', $invalid),
+ implode(', ', $missing)));
+ } else if ($invalid) {
+ throw new DifferentialFieldParseException(
+ pht(
+ 'The objects you have listed include objects of the wrong '.
+ 'type (%s).',
+ implode(', ', $invalid)));
+ } else if ($missing) {
+ throw new DifferentialFieldParseException(
+ pht(
+ 'The objects you have listed include objects which do not '.
+ 'exist (%s).',
+ implode(', ', $missing)));
+ }
+ }
+
+ return array_values(array_unique(mpull($objects, 'getPHID')));
+ }
+
+ private function loadObjects($names) {
+ // First, try to load visible objects using monograms. This covers most
+ // object types, but does not cover users or user email addresses.
+ $query = id(new PhabricatorObjectQuery())
+ ->setViewer($this->getViewer())
+ ->withNames($names);
+
+ $query->execute();
+ $objects = $query->getNamedResults();
+
+ $results = array();
+ foreach ($names as $key => $name) {
+ if (isset($objects[$name])) {
+ $results[$name] = $objects[$name];
+ unset($names[$key]);
+ }
+ }
+
+ if ($names) {
+ // We still have some symbols we haven't been able to resolve, so try to
+ // load users. Try by username first...
+ $users = id(new PhabricatorPeopleQuery())
+ ->setViewer($this->getViewer())
+ ->withUsernames($names)
+ ->execute();
+
+ $user_map = array();
+ foreach ($users as $user) {
+ $user_map[phutil_utf8_strtolower($user->getUsername())] = $user;
+ }
+
+ foreach ($names as $key => $name) {
+ $normal_name = phutil_utf8_strtolower($name);
+ if (isset($user_map[$normal_name])) {
+ $results[$name] = $user_map[$normal_name];
+ unset($names[$key]);
+ }
+ }
+ }
+
+ $mailing_list_app = PhabricatorApplication::getByClass(
+ 'PhabricatorApplicationMailingLists');
+ if ($mailing_list_app->isInstalled()) {
+ if ($names) {
+ // We still haven't been able to resolve everything; try mailing lists
+ // by name as a last resort.
+ $lists = id(new PhabricatorMailingListQuery())
+ ->setViewer($this->getViewer())
+ ->withNames($names)
+ ->execute();
+
+ $lists = mpull($lists, null, 'getName');
+ foreach ($names as $key => $name) {
+ if (isset($lists[$name])) {
+ $results[$name] = $lists[$name];
+ unset($names[$key]);
+ }
+ }
+ }
+ }
+
+ return $results;
+ }
+
+
+}
diff --git a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php
@@ -0,0 +1,87 @@
+<?php
+
+final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase {
+
+ protected function getPhabricatorTestCaseConfiguration() {
+ return array(
+ self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
+ );
+ }
+
+ public function testObjectListQuery() {
+ $user = $this->generateNewTestUser();
+ $name = $user->getUsername();
+ $phid = $user->getPHID();
+
+
+ $result = $this->parseObjectList("@{$name}");
+ $this->assertEqual(array($phid), $result);
+
+ $result = $this->parseObjectList("{$name}");
+ $this->assertEqual(array($phid), $result);
+
+ $result = $this->parseObjectList("{$name}, {$name}");
+ $this->assertEqual(array($phid), $result);
+
+ $result = $this->parseObjectList("@{$name}, {$name}");
+ $this->assertEqual(array($phid), $result);
+
+ $result = $this->parseObjectList("");
+ $this->assertEqual(array(), $result);
+
+ // Expect failure when loading a user if objects must be of type "DUCK".
+ $caught = null;
+ try {
+ $result = $this->parseObjectList("{$name}", array("DUCK"));
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+ $this->assertEqual(true, ($caught instanceof Exception));
+
+
+ // Expect failure when loading an invalid object.
+ $caught = null;
+ try {
+ $result = $this->parseObjectList("invalid");
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+ $this->assertEqual(true, ($caught instanceof Exception));
+
+
+ // Expect failure when loading ANY invalid object, by default.
+ $caught = null;
+ try {
+ $result = $this->parseObjectList("{$name}, invalid");
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+ $this->assertEqual(true, ($caught instanceof Exception));
+
+
+ // With partial results, this should load the valid user.
+ $result = $this->parseObjectList("{$name}, invalid", array(), true);
+ $this->assertEqual(array($phid), $result);
+ }
+
+ private function parseObjectList(
+ $list,
+ array $types = array(),
+ $allow_partial = false) {
+
+ $query = id(new PhabricatorObjectListQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->setObjectList($list);
+
+ if ($types) {
+ $query->setAllowedTypes($types);
+ }
+
+ if ($allow_partial) {
+ $query->setAllowPartialResults(true);
+ }
+
+ return $query->execute();
+ }
+
+}
diff --git a/src/applications/search/engine/PhabricatorJumpNavHandler.php b/src/applications/search/engine/PhabricatorJumpNavHandler.php
--- a/src/applications/search/engine/PhabricatorJumpNavHandler.php
+++ b/src/applications/search/engine/PhabricatorJumpNavHandler.php
@@ -16,9 +16,7 @@
'/^p$/i' => 'uri:/project/',
'/^u$/i' => 'uri:/people/',
'/^p\s+(.+)$/i' => 'project',
- '/^#(.+)$/i' => 'project',
'/^u\s+(\S+)$/i' => 'user',
- '/^@(.+)$/i' => 'user',
'/^task:\s*(.+)/i' => 'create-task',
'/^(?:s|symbol)\s+(\S+)/i' => 'find-symbol',
'/^r\s+(.+)$/i' => 'find-repository',

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 15, 4:29 AM (4 d, 10 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6737814
Default Alt Text
D8445.id20040.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility