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

@@ -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',

src/aphront/request/AphrontRequest.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,8 +16,10 @@
  * limitations under the License.
  */
 
-
 /**
+ *
+ * @task data Accessing Request Data
+ *
  * @group aphront
  */
 class AphrontRequest {
@@ -47,15 +49,6 @@ class AphrontRequest {
     return $this->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);
   }

src/aphront/request/__tests__/AphrontRequestTestCase.php

@@ -0,0 +1,81 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class AphrontRequestTestCase extends PhabricatorTestCase {
+
+  public function testRequestDataAccess() {
+    $r = new AphrontRequest('http://example.com/', '/');
+    $r->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'));
+  }
+
+}

src/aphront/request/__tests__/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/request');
+phutil_require_module('phabricator', 'infrastructure/testing/testcase');
+
+
+phutil_require_source('AphrontRequestTestCase.php');

src/applications/feed/story/base/__init__.php

@@ -6,5 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/phid/handle');
+
 
 phutil_require_source('PhabricatorFeedStory.php');

src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -95,12 +95,7 @@ class ManiphestTaskListController extends ManiphestController {
       $project_phids = array();
     }
 
-    $task_ids = $request->getStr('tasks');
+    $task_ids = $request->getStrList('tasks');
-    if (strlen($task_ids)) {
-      $task_ids = preg_split('/[\s,]+/', $task_ids);
-    } else {
-      $task_ids = array();
-    }
 
     $page = $request->getInt('page');
     $page_size = self::DEFAULT_PAGE_SIZE;

src/applications/phid/controller/lookup/PhabricatorPHIDLookupController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,8 +23,7 @@ class PhabricatorPHIDLookupController
 
     $request = $this->getRequest();
     if ($request->isFormPost()) {
-      $phids = preg_split('/[\s,]+/', $request->getStr('phids'));
+      $phids = $request->getStrList('phids');
-      $phids = array_filter($phids);
       if ($phids) {
         $handles = id(new PhabricatorObjectHandleData($phids))
           ->loadHandles();

src/applications/repository/controller/arcansistprojectedit/PhabricatorRepositoryArcanistProjectEditController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -47,10 +47,8 @@ class PhabricatorRepositoryArcanistProjectEditController
 
     if ($request->isFormPost()) {
 
-      $indexed = $request->getStr('symbolIndexLanguages');
+      $indexed = $request->getStrList('symbolIndexLanguages');
-      $indexed = strtolower($indexed);
+      $indexed = array_map('strtolower', $indexed);
-      $indexed = preg_split('/[ ,]+/', $indexed);
-      $indexed = array_filter($indexed);
       $project->setSymbolIndexLanguages($indexed);
 
       $project->setSymbolIndexProjects($request->getArr('symbolIndexProjects'));