Don't allow empty list constraints in Conduit calls

Summary:
Ref T11473. If you write a method like `get_stuff(ids)` and then call it with an empty list of IDs, you can end up passing an empty constraint to Conduit.

If you run a `*.search` method with such a constraint, like this one:

```
{
  "ids": []
}
```

...we have three possible beahviors:

  # Treat it like the user passed no constraint (basically, ignore the constraint).
  # Respect the constraint (return no results).
  # Error.

Currently, we do (1). However, this is pretty confusing and I think clearly the worst option, since it means `get_stuff(array())` in client code will often tend to return a ton of results.

We could do (2) instead, but this is also sort of confusing (it may not be obvious why nothing matched, even though it's an application bug) and I think most reasonable client code should be doing an `if ($ids)` test: this test makes clients a little more complicated, but they can save a network call, and I think they often need to do this test anyway (for example, to show the user a different message).

This implements (3), and just considers these to be errors: this is the least tricky behavior, it's consistent with what we do in PHP, makes fairly good sense, and the only cost for this is that client code may need to be slightly more complex, but this slightly more complex code is usually better code.

Test Plan: Ran Conduit `*.search` queries with `"ids":[]` and `"phids":[]`, got sensible errors instead of runaway result sets.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11473

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

src/applications/conduit/parametertype/ConduitListParameterType.php

@@ -3,6 +3,17 @@
 abstract class ConduitListParameterType
   extends ConduitParameterType {
 
+  private $allowEmptyList = true;
+
+  public function setAllowEmptyList($allow_empty_list) {
+    $this->allowEmptyList = $allow_empty_list;
+    return $this;
+  }
+
+  public function getAllowEmptyList() {
+    return $this->allowEmptyList;
+  }
+
   protected function getParameterValue(array $request, $key) {
     $value = parent::getParameterValue($request, $key);
 
@@ -27,6 +38,13 @@ abstract class ConduitListParameterType
         pht('Expected a list, but value is an object.'));
     }
 
+    if (!$value && !$this->getAllowEmptyList()) {
+      $this->raiseValidationException(
+        $request,
+        $key,
+        pht('Expected a nonempty list, but value is an empty list.'));
+    }
+
     return $value;
   }
 

src/applications/conduit/parametertype/ConduitParameterType.php

@@ -61,7 +61,11 @@ abstract class ConduitParameterType extends Phobject {
   protected function raiseValidationException(array $request, $key, $message) {
     // TODO: Specialize this so we can give users more tailored messages from
     // Conduit.
-    throw new Exception($message);
+    throw new Exception(
+      pht(
+        'Error while reading "%s": %s',
+        $key,
+        $message));
   }
 
 

src/applications/search/field/PhabricatorIDsSearchField.php

@@ -24,7 +24,8 @@ final class PhabricatorIDsSearchField
   }
 
   protected function newConduitParameterType() {
-    return new ConduitIntListParameterType();
+    return id(new ConduitIntListParameterType())
+      ->setAllowEmptyList(false);
   }
 
 }

src/applications/search/field/PhabricatorPHIDsSearchField.php

@@ -24,7 +24,8 @@ final class PhabricatorPHIDsSearchField
   }
 
   protected function newConduitParameterType() {
-    return new ConduitPHIDListParameterType();
+    return id(new ConduitPHIDListParameterType())
+      ->setAllowEmptyList(false);
   }
 
 }

src/applications/search/field/PhabricatorSearchDatasourceField.php

@@ -22,7 +22,8 @@ final class PhabricatorSearchDatasourceField
 
   protected function newConduitParameterType() {
     if (!$this->conduitParameterType) {
-      return new ConduitStringListParameterType();
+      return id(new ConduitStringListParameterType())
+        ->setAllowEmptyList(false);
     }
 
     return $this->conduitParameterType;