Refactor freeform field specification

Summary:
I want to attach the task also to revision, not only to commit by mentioning it in summary.
This is a preparation for it, useful by itself:

* One query instead of N.
* Lower CRAP score, yay!

Refs T945.

Test Plan:
My Test Plan is really //plan// this time.
I want to update Phabricator in the minute when I commit this.

Reviewers: 20after4, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T945

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

src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php

@@ -3,18 +3,7 @@
 abstract class DifferentialFreeformFieldSpecification
   extends DifferentialFieldSpecification {
 
-  public function didParseCommit(
+  private function findMentionedTasks($message) {
-    PhabricatorRepository $repository,
-    PhabricatorRepositoryCommit $commit,
-    PhabricatorRepositoryCommitData $data) {
-
-    $user = id(new PhabricatorUser())->loadOneWhere(
-      'phid = %s',
-      $data->getCommitDetail('authorPHID'));
-    if (!$user) {
-      return;
-    }
-
     $prefixes = array(
       'resolves'      => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
       'fixes'         => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
@@ -55,16 +44,13 @@ abstract class DifferentialFreeformFieldSpecification
     $suffix_regex = implode('|', $suffix_regex);
 
     $matches = null;
-    $ok = preg_match_all(
+    preg_match_all(
       "/({$prefix_regex})\s+T(\d+)\s*({$suffix_regex})/i",
-      $this->renderValueForCommitMessage($is_edit = false),
+      $message,
       $matches,
       PREG_SET_ORDER);
 
-    if (!$ok) {
+    $tasks_statuses = array();
-      return;
-    }
-
     foreach ($matches as $set) {
       $prefix = strtolower($set[1]);
       $task_id = (int)$set[2];
@@ -75,16 +61,35 @@ abstract class DifferentialFreeformFieldSpecification
         $status = idx($prefixes, $prefix);
       }
 
-      $tasks = id(new ManiphestTaskQuery())
+      $tasks_statuses[$task_id] = $status;
-        ->withTaskIDs(array($task_id))
+    }
-        ->execute();
-      $task = idx($tasks, $task_id);
 
-      if (!$task) {
+    return $tasks_statuses;
-        // Task doesn't exist, or the user can't see it.
+  }
-        continue;
-      }
 
+  public function didParseCommit(
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryCommit $commit,
+    PhabricatorRepositoryCommitData $data) {
+
+    $user = id(new PhabricatorUser())->loadOneWhere(
+      'phid = %s',
+      $data->getCommitDetail('authorPHID'));
+    if (!$user) {
+      return;
+    }
+
+    $message = $this->renderValueForCommitMessage($is_edit = false);
+    $tasks_statuses = $this->findMentionedTasks($message);
+    if (!$tasks_statuses) {
+      return;
+    }
+
+    $tasks = id(new ManiphestTaskQuery())
+      ->withTaskIDs(array_keys($tasks_statuses))
+      ->execute();
+
+    foreach ($tasks as $task_id => $task) {
       id(new PhabricatorEdgeEditor())
         ->setActor($user)
         ->addEdge(
@@ -93,6 +98,7 @@ abstract class DifferentialFreeformFieldSpecification
           $commit->getPHID())
         ->save();
 
+      $status = $tasks_statuses[$task_id];
       if (!$status) {
         // Text like "Ref T123", don't change the task status.
         continue;