Allow filtering of "date" custom fields

Summary: Ref T4663. Ref T4659. Allows "date" fields to be filtered with range parameters.

Test Plan:
  - Added a custom "date" field with "search".
  - Populated some values.
  - Searched for dates using new range filters.
  - Combined date search with other searches.
  - Ran other searches independently.
  - Inspected the generated queries.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: shadowhand, epriestley

Maniphest Tasks: T4659, T4663

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

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php

@@ -81,9 +81,73 @@ final class PhabricatorStandardCustomFieldDate
     return $control;
   }
 
-  // TODO: Support ApplicationSearch for these fields. We build indexes above,
+  public function readApplicationSearchValueFromRequest(
-  // but don't provide a UI for searching. To do so, we need a reasonable date
+    PhabricatorApplicationSearchEngine $engine,
-  // range control and the ability to add a range constraint.
+    AphrontRequest $request) {
+
+    $key = $this->getFieldKey();
+
+    return array(
+      'min' => $request->getStr($key.'.min'),
+      'max' => $request->getStr($key.'.max'),
+    );
+  }
+
+  public function applyApplicationSearchConstraintToQuery(
+    PhabricatorApplicationSearchEngine $engine,
+    PhabricatorCursorPagedPolicyAwareQuery $query,
+    $value) {
+
+    $viewer = $this->getViewer();
+
+    if (!is_array($value)) {
+      $value = array();
+    }
+
+    $min_str = idx($value, 'min', '');
+    if (strlen($min_str)) {
+      $min = PhabricatorTime::parseLocalTime($min_str, $viewer);
+    } else {
+      $min = null;
+    }
+
+    $max_str = idx($value, 'max', '');
+    if (strlen($max_str)) {
+      $max = PhabricatorTime::parseLocalTime($max_str, $viewer);
+    } else {
+      $max = null;
+    }
+
+    if (($min !== null) || ($max !== null)) {
+      $query->withApplicationSearchRangeConstraint(
+        $this->newNumericIndex(null),
+        $min,
+        $max);
+    }
+  }
+
+  public function appendToApplicationSearchForm(
+    PhabricatorApplicationSearchEngine $engine,
+    AphrontFormView $form,
+    $value,
+    array $handles) {
+
+    if (!is_array($value)) {
+      $value = array();
+    }
+
+    $form
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel(pht('%s After', $this->getFieldName()))
+          ->setName($this->getFieldKey().'.min')
+          ->setValue(idx($value, 'min', '')))
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel(pht('%s Before', $this->getFieldName()))
+          ->setName($this->getFieldKey().'.max')
+          ->setValue(idx($value, 'max', '')));
+  }
 
   public function getApplicationTransactionTitle(
     PhabricatorApplicationTransaction $xaction) {

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -285,16 +285,17 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
 
   /**
-   * Constrain the query with an ApplicationSearch index. This adds a constraint
+   * Constrain the query with an ApplicationSearch index, requiring field values
-   * which requires objects to have one or more corresponding rows in the index
+   * contain at least one of the values in a set.
-   * with one of the given values. Combined with appropriate indexes, it can
+   *
-   * build the most common types of queries, like:
+   * This constraint can build the most common types of queries, like:
    *
    *   - Find users with shirt sizes "X" or "XL".
    *   - Find shoes with size "13".
    *
    * @param PhabricatorCustomFieldIndexStorage Table where the index is stored.
    * @param string|list<string> One or more values to filter by.
+   * @return this
    * @task appsearch
    */
   public function withApplicationSearchContainsConstraint(
@@ -313,6 +314,51 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   }
 
 
+  /**
+   * Constrain the query with an ApplicationSearch index, requiring values
+   * exist in a given range.
+   *
+   * This constraint is useful for expressing date ranges:
+   *
+   *   - Find events between July 1st and July 7th.
+   *
+   * The ends of the range are inclusive, so a `$min` of `3` and a `$max` of
+   * `5` will match fields with values `3`, `4`, or `5`. Providing `null` for
+   * either end of the range will leave that end of the constraint open.
+   *
+   * @param PhabricatorCustomFieldIndexStorage Table where the index is stored.
+   * @param int|null Minimum permissible value, inclusive.
+   * @param int|null Maximum permissible value, inclusive.
+   * @return this
+   * @task appsearch
+   */
+  public function withApplicationSearchRangeConstraint(
+    PhabricatorCustomFieldIndexStorage $index,
+    $min,
+    $max) {
+
+    $index_type = $index->getIndexValueType();
+    if ($index_type != 'int') {
+      throw new Exception(
+        pht(
+          'Attempting to apply a range constraint to a field with index type '.
+          '"%s", expected type "%s".',
+          $index_type,
+          'int'));
+    }
+
+    $this->applicationSearchConstraints[] = array(
+      'type' => $index->getIndexValueType(),
+      'cond' => 'range',
+      'table' => $index->getTableName(),
+      'index' => $index->getIndexKey(),
+      'value' => array($min, $max),
+    );
+
+    return $this;
+  }
+
+
   /**
    * Get the name of the query's primary object PHID column, for constructing
    * JOIN clauses. Normally (and by default) this is just `"phid"`, but if the
@@ -339,7 +385,10 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
     foreach ($this->applicationSearchConstraints as $constraint) {
       $type = $constraint['type'];
       $value = $constraint['value'];
+      $cond = $constraint['cond'];
 
+      switch ($cond) {
+        case '=':
           switch ($type) {
             case 'string':
             case 'int':
@@ -348,7 +397,17 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
               }
               break;
             default:
-          throw new Exception("Unknown constraint type '{$type}!");
+              throw new Exception(pht('Unknown index type "%s"!', $type));
+          }
+          break;
+        case 'range':
+          // NOTE: It's possible to write a custom field where multiple rows
+          // match a range constraint, but we don't currently ship any in the
+          // upstream and I can't immediately come up with cases where this
+          // would make sense.
+          break;
+        default:
+          throw new Exception(pht('Unknown constraint condition "%s"!', $cond));
       }
     }
 
@@ -395,44 +454,84 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
       $index = $constraint['index'];
       $cond = $constraint['cond'];
       $phid_column = $this->getApplicationSearchObjectPHIDColumn();
-      if ($cond !== '=') {
+      switch ($cond) {
-        throw new Exception("Unknown constraint condition '{$cond}'!");
+        case '=':
-      }
-
           $type = $constraint['type'];
           switch ($type) {
             case 'string':
-          $joins[] = qsprintf(
+              $constraint_clause = qsprintf(
                 $conn_r,
-            'JOIN %T %T ON %T.objectPHID = %Q
+                '%T.indexValue IN (%Ls)',
-              AND %T.indexKey = %s
-              AND %T.indexValue IN (%Ls)',
-            $table,
-            $alias,
-            $alias,
-            $phid_column,
-            $alias,
-            $index,
                 $alias,
                 (array)$constraint['value']);
               break;
             case 'int':
+              $constraint_clause = qsprintf(
+                $conn_r,
+                '%T.indexValue IN (%Ld)',
+                $alias,
+                (array)$constraint['value']);
+              break;
+            default:
+              throw new Exception(pht('Unknown index type "%s"!', $type));
+          }
+
           $joins[] = qsprintf(
             $conn_r,
             'JOIN %T %T ON %T.objectPHID = %Q
               AND %T.indexKey = %s
-              AND %T.indexValue IN (%Ld)',
+              AND (%Q)',
             $table,
             $alias,
             $alias,
             $phid_column,
             $alias,
             $index,
+            $constraint_clause);
+          break;
+        case 'range':
+          list($min, $max) = $constraint['value'];
+          if (($min === null) && ($max === null)) {
+            // If there's no actual range constraint, just move on.
+            break;
+          }
+
+          if ($min === null) {
+            $constraint_clause = qsprintf(
+              $conn_r,
+              '%T.indexValue <= %d',
               $alias,
-            (array)$constraint['value']);
+              $max);
+          } else if ($max === null) {
+            $constraint_clause = qsprintf(
+              $conn_r,
+              '%T.indexValue >= %d',
+              $alias,
+              $min);
+          } else {
+            $constraint_clause = qsprintf(
+              $conn_r,
+              '%T.indexValue BETWEEN %d AND %d',
+              $alias,
+              $min,
+              $max);
+          }
+
+          $joins[] = qsprintf(
+            $conn_r,
+            'JOIN %T %T ON %T.objectPHID = %Q
+              AND %T.indexKey = %s
+              AND (%Q)',
+            $table,
+            $alias,
+            $alias,
+            $phid_column,
+            $alias,
+            $index,
+            $constraint_clause);
           break;
         default:
-          throw new Exception("Unknown constraint type '{$type}'!");
+          throw new Exception(pht('Unknown constraint condition "%s"!', $cond));
       }
     }