Further simplify PhabricatorCustomFieldInterface

Summary:
Ref T1703. Ref T3718. This introduces `PhabricatorCustomFieldAttachment`, which is just a fancy `array()`. The goal here is to simplify `PhabricatorCustomFieldInterface` as much as possible.

In particular, it can now use common infrastructure (`assertAttached()`) and is more difficult to get wrong.

Test Plan: Edited custom fields on profile.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1703, T3718

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

src/__phutil_library_map__.php

@@ -1025,6 +1025,7 @@ phutil_register_library_map(array(
     'PhabricatorCrumbsView' => 'view/layout/PhabricatorCrumbsView.php',
     'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php',
     'PhabricatorCustomField' => 'infrastructure/customfield/field/PhabricatorCustomField.php',
+    'PhabricatorCustomFieldAttachment' => 'infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php',
     'PhabricatorCustomFieldConfigOptionType' => 'infrastructure/customfield/config/PhabricatorCustomFieldConfigOptionType.php',
     'PhabricatorCustomFieldDataNotAvailableException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldDataNotAvailableException.php',
     'PhabricatorCustomFieldImplementationIncompleteException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldImplementationIncompleteException.php',

src/applications/people/storage/PhabricatorUser.php

@@ -36,7 +36,7 @@ final class PhabricatorUser
   private $status = self::ATTACHABLE;
   private $preferences = null;
   private $omnipotent = false;
-  private $customFields = array();
+  private $customFields = self::ATTACHABLE;
 
   protected function readField($field) {
     switch ($field) {
@@ -843,15 +843,12 @@ EOBODY;
     return 'PhabricatorUserCustomField';
   }
 
-  public function getCustomFields($role) {
-    if (idx($this->customFields, $role) === null) {
-      PhabricatorCustomField::raiseUnattachedException($this, $role);
-    }
-    return $this->customFields[$role];
+  public function getCustomFields() {
+    return $this->assertAttached($this->customFields);
   }
 
-  public function attachCustomFields($role, PhabricatorCustomFieldList $list) {
-    $this->customFields[$role] = $list;
+  public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) {
+    $this->customFields = $fields;
     return $this;
   }
 

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -30,17 +30,6 @@ abstract class PhabricatorCustomField {
 /* -(  Building Applications with Custom Fields  )--------------------------- */
 
 
-  /**
-   * @task apps
-   */
-  public static function raiseUnattachedException(
-    PhabricatorCustomFieldInterface $object,
-    $role) {
-    throw new PhabricatorCustomFieldNotAttachedException(
-      "Call attachCustomFields() before getCustomFields()!");
-  }
-
-
   /**
    * @task apps
    */
@@ -49,7 +38,14 @@ abstract class PhabricatorCustomField {
     $role) {
 
     try {
-      $field_list = $object->getCustomFields($role);
+      $attachment = $object->getCustomFields();
+    } catch (PhabricatorDataNotAttachedException $ex) {
+      $attachment = new PhabricatorCustomFieldAttachment();
+      $object->attachCustomFields($attachment);
+    }
+
+    try {
+      $field_list = $attachment->getCustomFieldList($role);
     } catch (PhabricatorCustomFieldNotAttachedException $ex) {
       $base_class = $object->getCustomFieldBaseClass();
 
@@ -74,7 +70,7 @@ abstract class PhabricatorCustomField {
       }
 
       $field_list = new PhabricatorCustomFieldList($fields);
-      $object->attachCustomFields($role, $field_list);
+      $attachment->addCustomFieldList($role, $field_list);
     }
 
     return $field_list;
@@ -88,7 +84,10 @@ abstract class PhabricatorCustomField {
     PhabricatorCustomFieldInterface $object,
     $role,
     $field_key) {
-    return idx(self::getObjectFields($object, $role)->getFields(), $field_key);
+
+    $fields = self::getObjectFields($object, $role)->getFields();
+
+    return idx($fields, $field_key);
   }
 
 

src/infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php

@@ -0,0 +1,28 @@
+<?php
+
+/**
+ * Convenience class which simplifies the implementation of
+ * @{interface:PhabricatorCustomFieldInterface} by obscuring the details of how
+ * custom fields are stored.
+ *
+ * Generally, you should not use this class directly. It is used by
+ * @{class:PhabricatorCustomField} to manage field storage on objects.
+ */
+final class PhabricatorCustomFieldAttachment {
+
+  private $lists = array();
+
+  public function addCustomFieldList($role, PhabricatorCustomFieldList $list) {
+    $this->lists[$role] = $list;
+    return $this;
+  }
+
+  public function getCustomFieldList($role) {
+    if (empty($this->lists[$role])) {
+      throw new PhabricatorCustomFieldNotAttachedException(
+        "Role list '{$role}' is not available!");
+    }
+    return $this->lists[$role];
+  }
+
+}

src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php

@@ -4,8 +4,8 @@ interface PhabricatorCustomFieldInterface {
 
   public function getCustomFieldBaseClass();
   public function getCustomFieldSpecificationForRole($role);
-  public function getCustomFields($role);
-  public function attachCustomFields($role, PhabricatorCustomFieldList $list);
+  public function getCustomFields();
+  public function attachCustomFields(PhabricatorCustomFieldAttachment $fields);
 
 }
 
@@ -16,7 +16,7 @@ interface PhabricatorCustomFieldInterface {
 /* -(  PhabricatorCustomFieldInterface  )------------------------------------ */
 /*
 
-  private $customFields = array();
+  private $customFields = self::ATTACHABLE;
 
   public function getCustomFieldSpecificationForRole($role) {
     return PhabricatorEnv::getEnvConfig(<<<'application.fields'>>>);
@@ -26,15 +26,12 @@ interface PhabricatorCustomFieldInterface {
     return <<<<'YourApplicationHereCustomField'>>>>;
   }
 
-  public function getCustomFields($role) {
-    if (idx($this->customFields, $role) === null) {
-      PhabricatorCustomField::raiseUnattachedException($this, $role);
-    }
-    return $this->customFields[$role];
+  public function getCustomFields() {
+    return $this->assertAttached($this->customFields);
   }
 
-  public function attachCustomFields($role, PhabricatorCustomFieldList $list) {
-    $this->customFields[$role] = $list;
+  public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) {
+    $this->customFields = $fields;
     return $this;
   }