From ec1df21befae80f9f630246578fbc94aa1bca7e5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 3 Jan 2012 21:57:45 -0800 Subject: [PATCH] Add getStrList() to AphrontRequest Summary: - We have a few places where we do some kind of ad-hoc comma list tokenizing, and I'm adding another one in D1290. Add a helper to the request object. - Add some unit tests. Test Plan: - Ran unit tests. - Used PHID manager, Maniphest custom view, and Repository project editor. Reviewers: btrahan, fratrik, jungejason Reviewed By: btrahan CC: aran, btrahan, epriestley Differential Revision: https://secure.phabricator.com/D1302 --- src/__phutil_library_map__.php | 2 + src/aphront/request/AphrontRequest.php | 71 +++++++++++++--- .../__tests__/AphrontRequestTestCase.php | 81 +++++++++++++++++++ src/aphront/request/__tests__/__init__.php | 13 +++ src/applications/feed/story/base/__init__.php | 2 + .../tasklist/ManiphestTaskListController.php | 9 +-- .../PhabricatorPHIDLookupController.php | 5 +- ...epositoryArcanistProjectEditController.php | 8 +- 8 files changed, 165 insertions(+), 26 deletions(-) create mode 100644 src/aphront/request/__tests__/AphrontRequestTestCase.php create mode 100644 src/aphront/request/__tests__/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 17b1efed9b..6c4b66d962 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -71,6 +71,7 @@ phutil_register_library_map(array( 'AphrontReloadResponse' => 'aphront/response/reload', 'AphrontRequest' => 'aphront/request', 'AphrontRequestFailureView' => 'view/page/failure', + 'AphrontRequestTestCase' => 'aphront/request/__tests__', 'AphrontResponse' => 'aphront/response/base', 'AphrontScopedUnguardedWriteCapability' => 'aphront/writeguard/scopeguard', 'AphrontSideNavFilterView' => 'view/layout/sidenavfilter', @@ -834,6 +835,7 @@ phutil_register_library_map(array( 'AphrontRedirectResponse' => 'AphrontResponse', 'AphrontReloadResponse' => 'AphrontRedirectResponse', 'AphrontRequestFailureView' => 'AphrontView', + 'AphrontRequestTestCase' => 'PhabricatorTestCase', 'AphrontSideNavFilterView' => 'AphrontView', 'AphrontSideNavView' => 'AphrontView', 'AphrontTableView' => 'AphrontView', diff --git a/src/aphront/request/AphrontRequest.php b/src/aphront/request/AphrontRequest.php index 85ca8214b8..b8bb0536b8 100644 --- a/src/aphront/request/AphrontRequest.php +++ b/src/aphront/request/AphrontRequest.php @@ -1,7 +1,7 @@ applicationConfiguration; } - final public function setRequestData(array $request_data) { - $this->requestData = $request_data; - return $this; - } - - final public function getRequestData() { - return $this->requestData; - } - final public function getPath() { return $this->path; } @@ -64,6 +57,30 @@ class AphrontRequest { return $this->host; } + +/* -( Accessing Request Data )--------------------------------------------- */ + + + /** + * @task data + */ + final public function setRequestData(array $request_data) { + $this->requestData = $request_data; + return $this; + } + + + /** + * @task data + */ + final public function getRequestData() { + return $this->requestData; + } + + + /** + * @task data + */ final public function getInt($name, $default = null) { if (isset($this->requestData[$name])) { return (int)$this->requestData[$name]; @@ -72,6 +89,10 @@ class AphrontRequest { } } + + /** + * @task data + */ final public function getBool($name, $default = null) { if (isset($this->requestData[$name])) { if ($this->requestData[$name] === 'true') { @@ -86,6 +107,10 @@ class AphrontRequest { } } + + /** + * @task data + */ final public function getStr($name, $default = null) { if (isset($this->requestData[$name])) { $str = (string)$this->requestData[$name]; @@ -100,6 +125,10 @@ class AphrontRequest { } } + + /** + * @task data + */ final public function getArr($name, $default = array()) { if (isset($this->requestData[$name]) && is_array($this->requestData[$name])) { @@ -109,6 +138,26 @@ class AphrontRequest { } } + + /** + * @task data + */ + final public function getStrList($name, $default = array()) { + if (!isset($this->requestData[$name])) { + return $default; + } + $list = $this->getStr($name); + $list = preg_split('/[,\n]/', $list); + $list = array_map('trim', $list); + $list = array_filter($list, 'strlen'); + $list = array_values($list); + return $list; + } + + + /** + * @task data + */ final public function getExists($name) { return array_key_exists($name, $this->requestData); } diff --git a/src/aphront/request/__tests__/AphrontRequestTestCase.php b/src/aphront/request/__tests__/AphrontRequestTestCase.php new file mode 100644 index 0000000000..151b3ccc7c --- /dev/null +++ b/src/aphront/request/__tests__/AphrontRequestTestCase.php @@ -0,0 +1,81 @@ +setRequestData( + array( + 'str_empty' => '', + 'str' => 'derp', + 'str_true' => 'true', + 'str_false' => 'false', + + 'zero' => '0', + 'one' => '1', + + 'arr_empty' => array(), + 'arr_num' => array(1, 2, 3), + + 'comma' => ',', + 'comma_1' => 'a, b', + 'comma_2' => ' ,a ,, b ,,,, ,, ', + 'comma_3' => '0', + 'comma_4' => 'a, a, b, a', + 'comma_5' => "a\nb, c\n\nd\n\n\n,\n", + )); + + $this->assertEqual(1, $r->getInt('one')); + $this->assertEqual(0, $r->getInt('zero')); + $this->assertEqual(null, $r->getInt('does-not-exist')); + $this->assertEqual(0, $r->getInt('str_empty')); + + $this->assertEqual(true, $r->getBool('one')); + $this->assertEqual(false, $r->getBool('zero')); + $this->assertEqual(true, $r->getBool('str_true')); + $this->assertEqual(false, $r->getBool('str_false')); + $this->assertEqual(true, $r->getBool('str')); + $this->assertEqual(null, $r->getBool('does-not-exist')); + $this->assertEqual(false, $r->getBool('str_empty')); + + $this->assertEqual('derp', $r->getStr('str')); + $this->assertEqual('', $r->getStr('str_empty')); + $this->assertEqual(null, $r->getStr('does-not-exist')); + + $this->assertEqual(array(), $r->getArr('arr_empty')); + $this->assertEqual(array(1, 2, 3), $r->getArr('arr_num')); + $this->assertEqual(null, $r->getArr('str_empty', null)); + $this->assertEqual(null, $r->getArr('str_true', null)); + $this->assertEqual(null, $r->getArr('does-not-exist', null)); + $this->assertEqual(array(), $r->getArr('does-not-exist')); + + $this->assertEqual(array(), $r->getStrList('comma')); + $this->assertEqual(array('a', 'b'), $r->getStrList('comma_1')); + $this->assertEqual(array('a', 'b'), $r->getStrList('comma_2')); + $this->assertEqual(array('0'), $r->getStrList('comma_3')); + $this->assertEqual(array('a', 'a', 'b', 'a'), $r->getStrList('comma_4')); + $this->assertEqual(array('a', 'b', 'c', 'd'), $r->getStrList('comma_5')); + $this->assertEqual(array(), $r->getStrList('does-not-exist')); + $this->assertEqual(null, $r->getStrList('does-not-exist', null)); + + $this->assertEqual(true, $r->getExists('str')); + $this->assertEqual(false, $r->getExists('does-not-exist')); + } + +} diff --git a/src/aphront/request/__tests__/__init__.php b/src/aphront/request/__tests__/__init__.php new file mode 100644 index 0000000000..e8fe154944 --- /dev/null +++ b/src/aphront/request/__tests__/__init__.php @@ -0,0 +1,13 @@ +getStr('tasks'); - if (strlen($task_ids)) { - $task_ids = preg_split('/[\s,]+/', $task_ids); - } else { - $task_ids = array(); - } + $task_ids = $request->getStrList('tasks'); $page = $request->getInt('page'); $page_size = self::DEFAULT_PAGE_SIZE; diff --git a/src/applications/phid/controller/lookup/PhabricatorPHIDLookupController.php b/src/applications/phid/controller/lookup/PhabricatorPHIDLookupController.php index e95dd94b05..88aff7772c 100644 --- a/src/applications/phid/controller/lookup/PhabricatorPHIDLookupController.php +++ b/src/applications/phid/controller/lookup/PhabricatorPHIDLookupController.php @@ -1,7 +1,7 @@ getRequest(); if ($request->isFormPost()) { - $phids = preg_split('/[\s,]+/', $request->getStr('phids')); - $phids = array_filter($phids); + $phids = $request->getStrList('phids'); if ($phids) { $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); diff --git a/src/applications/repository/controller/arcansistprojectedit/PhabricatorRepositoryArcanistProjectEditController.php b/src/applications/repository/controller/arcansistprojectedit/PhabricatorRepositoryArcanistProjectEditController.php index 4d8e3827d8..920b358678 100644 --- a/src/applications/repository/controller/arcansistprojectedit/PhabricatorRepositoryArcanistProjectEditController.php +++ b/src/applications/repository/controller/arcansistprojectedit/PhabricatorRepositoryArcanistProjectEditController.php @@ -1,7 +1,7 @@ isFormPost()) { - $indexed = $request->getStr('symbolIndexLanguages'); - $indexed = strtolower($indexed); - $indexed = preg_split('/[ ,]+/', $indexed); - $indexed = array_filter($indexed); + $indexed = $request->getStrList('symbolIndexLanguages'); + $indexed = array_map('strtolower', $indexed); $project->setSymbolIndexLanguages($indexed); $project->setSymbolIndexProjects($request->getArr('symbolIndexProjects'));