From 026137f92fe660cf470c6ebc2673bd1567882f13 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Aug 2013 09:53:59 -0700 Subject: [PATCH] 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 | 1 + .../people/storage/PhabricatorUser.php | 13 ++++----- .../field/PhabricatorCustomField.php | 27 +++++++++--------- .../PhabricatorCustomFieldAttachment.php | 28 +++++++++++++++++++ .../PhabricatorCustomFieldInterface.php | 17 +++++------ 5 files changed, 54 insertions(+), 32 deletions(-) create mode 100644 src/infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 23924b44c6..46ab9b1318 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 7bea2f0441..f7fce6f047 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/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; } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 6902d70ed4..f882619c88 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/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); } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php new file mode 100644 index 0000000000..36a8a5bd8b --- /dev/null +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldAttachment.php @@ -0,0 +1,28 @@ +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]; + } + +} diff --git a/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php b/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php index 6bec3b345a..d8569d0a3c 100644 --- a/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php +++ b/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; }