From 524c2acb3dabf4c42b16723c472a108f151980a3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 6 Jun 2013 14:53:07 -0700 Subject: [PATCH] Flesh out ApplicationTransactions/CustomField integration Summary: None of this code is reachable yet. See discussion in D6147. Ref T1703. Provide tighter integration between ApplicationTransactions and CustomField. Basically, I'm just trying to get all the shared stuff into the base implementation. Test Plan: Code not reachable. Reviewers: chad, seporaitis Reviewed By: chad CC: aran Maniphest Tasks: T1703 Differential Revision: https://secure.phabricator.com/D6149 --- src/__phutil_library_map__.php | 7 +- .../{ => editor}/PhabricatorUserEditor.php | 0 .../storage/PhabricatorUserTransaction.php | 23 ++ .../constants/PhabricatorTransactions.php | 1 + ...habricatorApplicationTransactionEditor.php | 56 +++ ...ricatorCustomFieldNotAttachedException.php | 6 + .../field/PhabricatorCustomField.php | 344 +++++++++++++++++- .../PhabricatorCustomFieldInterface.php | 41 +++ 8 files changed, 463 insertions(+), 15 deletions(-) rename src/applications/people/{ => editor}/PhabricatorUserEditor.php (100%) create mode 100644 src/applications/people/storage/PhabricatorUserTransaction.php create mode 100644 src/infrastructure/customfield/exception/PhabricatorCustomFieldNotAttachedException.php create mode 100644 src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e0f0209ec5..07436b6d72 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -921,6 +921,8 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldDataNotAvailableException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldDataNotAvailableException.php', 'PhabricatorCustomFieldImplementationIncompleteException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldImplementationIncompleteException.php', 'PhabricatorCustomFieldIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldIndexStorage.php', + 'PhabricatorCustomFieldInterface' => 'infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php', + 'PhabricatorCustomFieldNotAttachedException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldNotAttachedException.php', 'PhabricatorCustomFieldNumericIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldNumericIndexStorage.php', 'PhabricatorCustomFieldStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php', 'PhabricatorCustomFieldStringIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStringIndexStorage.php', @@ -1541,7 +1543,7 @@ phutil_register_library_map(array( 'PhabricatorUnitsTestCase' => 'view/__tests__/PhabricatorUnitsTestCase.php', 'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php', 'PhabricatorUserDAO' => 'applications/people/storage/PhabricatorUserDAO.php', - 'PhabricatorUserEditor' => 'applications/people/PhabricatorUserEditor.php', + 'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php', 'PhabricatorUserEmail' => 'applications/people/storage/PhabricatorUserEmail.php', 'PhabricatorUserLDAPInfo' => 'applications/people/storage/PhabricatorUserLDAPInfo.php', 'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php', @@ -1554,6 +1556,7 @@ phutil_register_library_map(array( 'PhabricatorUserStatusInvalidEpochException' => 'applications/people/exception/PhabricatorUserStatusInvalidEpochException.php', 'PhabricatorUserStatusOverlapException' => 'applications/people/exception/PhabricatorUserStatusOverlapException.php', 'PhabricatorUserTestCase' => 'applications/people/storage/__tests__/PhabricatorUserTestCase.php', + 'PhabricatorUserTransaction' => 'applications/people/storage/PhabricatorUserTransaction.php', 'PhabricatorWorkboardExample' => 'applications/uiexample/examples/PhabricatorWorkboardExample.php', 'PhabricatorWorkboardView' => 'view/layout/PhabricatorWorkboardView.php', 'PhabricatorWorker' => 'infrastructure/daemon/workers/PhabricatorWorker.php', @@ -2764,6 +2767,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldDataNotAvailableException' => 'Exception', 'PhabricatorCustomFieldImplementationIncompleteException' => 'Exception', 'PhabricatorCustomFieldIndexStorage' => 'PhabricatorLiskDAO', + 'PhabricatorCustomFieldNotAttachedException' => 'Exception', 'PhabricatorCustomFieldNumericIndexStorage' => 'PhabricatorCustomFieldIndexStorage', 'PhabricatorCustomFieldStorage' => 'PhabricatorLiskDAO', 'PhabricatorCustomFieldStringIndexStorage' => 'PhabricatorCustomFieldIndexStorage', @@ -3385,6 +3389,7 @@ phutil_register_library_map(array( 'PhabricatorUserStatusInvalidEpochException' => 'Exception', 'PhabricatorUserStatusOverlapException' => 'Exception', 'PhabricatorUserTestCase' => 'PhabricatorTestCase', + 'PhabricatorUserTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorWorkboardExample' => 'PhabricatorUIExample', 'PhabricatorWorkboardView' => 'AphrontView', 'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask', diff --git a/src/applications/people/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php similarity index 100% rename from src/applications/people/PhabricatorUserEditor.php rename to src/applications/people/editor/PhabricatorUserEditor.php diff --git a/src/applications/people/storage/PhabricatorUserTransaction.php b/src/applications/people/storage/PhabricatorUserTransaction.php new file mode 100644 index 0000000000..67095c216d --- /dev/null +++ b/src/applications/people/storage/PhabricatorUserTransaction.php @@ -0,0 +1,23 @@ +object instanceof PhabricatorCustomFieldInterface) { + $types[] = PhabricatorTransactions::TYPE_CUSTOMFIELD; + } + return $types; } @@ -121,6 +125,9 @@ abstract class PhabricatorApplicationTransactionEditor $old_edges = $old_edges[$edge_src][$edge_type]; } return $old_edges; + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $field = $this->getCustomFieldForTransaction($object, $xaction); + return $field->getOldValueForApplicationTransactions(); default: return $this->getCustomTransactionOldValue($object, $xaction); } @@ -137,6 +144,9 @@ abstract class PhabricatorApplicationTransactionEditor return $xaction->getNewValue(); case PhabricatorTransactions::TYPE_EDGE: return $this->getEdgeTransactionNewValue($xaction); + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $field = $this->getCustomFieldForTransaction($object, $xaction); + return $field->getNewValueForApplicationTransactions(); default: return $this->getCustomTransactionNewValue($object, $xaction); } @@ -161,6 +171,9 @@ abstract class PhabricatorApplicationTransactionEditor switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: return $xaction->hasComment(); + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $field = $this->getCustomFieldForTransaction($object, $xaction); + return $field->getApplicationTransactionHasEffect($xaction); } return ($xaction->getOldValue() !== $xaction->getNewValue()); @@ -176,6 +189,9 @@ abstract class PhabricatorApplicationTransactionEditor case PhabricatorTransactions::TYPE_EDIT_POLICY: $object->setEditPolicy($xaction->getNewValue()); break; + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $field = $this->getCustomFieldForTransaction($object, $xaction); + return $field->applyApplicationTransactionInternalEffects($xaction); } return $this->applyCustomInternalTransaction($object, $xaction); } @@ -240,6 +256,9 @@ abstract class PhabricatorApplicationTransactionEditor $editor->save(); break; + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + $field = $this->getCustomFieldForTransaction($object, $xaction); + return $field->applyApplicationTransactionExternalEffects($xaction); } return $this->applyCustomExternalTransaction($object, $xaction); @@ -1244,4 +1263,41 @@ abstract class PhabricatorApplicationTransactionEditor return false; } + +/* -( Custom Fields )------------------------------------------------------- */ + + + /** + * @task customfield + */ + private function getCustomFieldForTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + $field_key = $xaction->getMetadataValue('customfield:key'); + if (!$field_key) { + throw new Exception( + "Custom field transaction has no 'customfield:key'!"); + } + + $field = PhabricatorCustomField::getObjectField( + $object, + PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS, + $field_key); + + if (!$field) { + throw new Exception( + "Custom field transaction has invalid 'customfield:key'; field ". + "'{$field_key}' is disabled or does not exist."); + } + + if (!$field->shouldAppearInApplicationTransactions()) { + throw new Exception( + "Custom field transaction '{$field_key}' does not implement ". + "integration for ApplicationTransactions."); + } + + return $field; + } + } diff --git a/src/infrastructure/customfield/exception/PhabricatorCustomFieldNotAttachedException.php b/src/infrastructure/customfield/exception/PhabricatorCustomFieldNotAttachedException.php new file mode 100644 index 0000000000..a633b2c4de --- /dev/null +++ b/src/infrastructure/customfield/exception/PhabricatorCustomFieldNotAttachedException.php @@ -0,0 +1,6 @@ +getFieldKey()); + +/* -( Building Applications with Custom Fields )--------------------------- */ + + + /** + * @task apps + */ + public static function raiseUnattachedException( + PhabricatorCustomFieldInterface $object, + $role) { + throw new PhabricatorCustomFieldNotAttachedException( + "Call attachCustomFields() before getCustomFields()!"); } - public function getFieldName() { - return $this->getFieldKey(); + + /** + * @task apps + */ + public static function getObjectFields( + PhabricatorCustomFieldInterface $object, + $role) { + + try { + $fields = $object->getCustomFields($role); + } catch (PhabricatorCustomFieldNotAttachedException $ex) { + $base_class = $object->getCustomFieldBaseClass(); + + if (!($base_class instanceof PhabricatorCustomField)) { + $obj_class = get_class($object); + throw new Exception( + "Object (of class '{$obj_class}') returned '{$base_class}' as its ". + "getCustomFieldBaseClass(), but this is not a recognized subclass ". + "of PhabricatorCustomField."); + } + + $spec = $object->getCustomFieldSpecificationForRole($role); + if (!is_array($spec)) { + $obj_class = get_class($object); + throw new Exception( + "Expected an array from getCustomFieldSpecificationForRole() for ". + "object of class '{$obj_class}'."); + } + + $fields = PhabricatorCustomField::buildFieldList($base_class, $spec); + + foreach ($fields as $key => $field) { + if (!$field->shouldEnableForRole($role)) { + unset($fields[$key]); + } + } + + foreach ($fields as $field) { + $field->setObject($object); + } + + $object->attachCustomFields($role, $fields); + } + + return $fields; } - public function createFields() { - return array($this); + + /** + * @task apps + */ + public static function getObjectField( + PhabricatorCustomFieldInterface $object, + $role, + $field_key) { + + return idx(self::getObjectFields($object, $role), $field_key); } - public function isFieldEnabled() { - return true; - } - - public function canDisableField() { - return true; - } + /** + * @task apps + */ public static function buildFieldList($base_class, array $spec) { $this_class = __CLASS__; if (!($base_class instanceof $this_class)) { @@ -81,9 +146,153 @@ abstract class PhabricatorCustomField { } +/* -( Core Properties and Field Identity )--------------------------------- */ + + + /** + * Return a key which uniquely identifies this field, like + * "mycompany.dinosaur.count". Normally you should provide some level of + * namespacing to prevent collisions. + * + * @return string String which uniquely identifies this field. + * @task core + */ + abstract public function getFieldKey(); + + + /** + * Return a human-readable field name. + * + * @return string Human readable field name. + * @task core + */ + public function getFieldName() { + return $this->getFieldKey(); + } + + + /** + * Return a short, human-readable description of the field's behavior. This + * provides more context to administrators when they are customizing fields. + * + * @return string|null Optional human-readable description. + * @task core + */ + public function getFieldDescription() { + return null; + } + + + /** + * Most field implementations are unique, in that one class corresponds to + * one field. However, some field implementations are general and a single + * implementation may drive several fields. + * + * For general implementations, the general field implementation can return + * multiple field instances here. + * + * @return list List of fields. + * @task core + */ + public function createFields() { + return array($this); + } + + + /** + * You can return `false` here if the field should not be enabled for any + * role. For example, it might depend on something (like an application or + * library) which isn't installed, or might have some global configuration + * which allows it to be disabled. + * + * @return bool False to completely disable this field for all roles. + * @task core + */ + public function isFieldEnabled() { + return true; + } + + + /** + * Low level selector for field availability. Fields can appear in different + * roles (like an edit view, a list view, etc.), but not every field needs + * to appear everywhere. Fields that are disabled in a role won't appear in + * that context within applications. + * + * Normally, you do not need to override this method. Instead, override the + * methods specific to roles you want to enable. For example, implement + * @{method:getStorageKey()} to activate the `'storage'` role. + * + * @return bool True to enable the field for the given role. + * @task core + */ + public function shouldEnableForRole($role) { + switch ($role) { + case self::ROLE_APPLICATIONTRANSACTIONS: + return $this->shouldAppearInApplicationTransactions(); + case self::ROLE_APPLICATIONSEARCH: + return $this->shouldAppearInApplicationSearch(); + case self::ROLE_STORAGE: + return ($this->getStorageKey() !== null); + case self::ROLE_DEFAULT: + return true; + default: + throw new Exception("Unknown field role '{$role}'!"); + } + } + + + /** + * Allow administrators to disable this field. Most fields should allow this, + * but some are fundamental to the behavior of the application and can be + * locked down to avoid chaos, disorder, and the decline of civilization. + * + * @return bool False to prevent this field from being disabled through + * configuration. + * @task core + */ + public function canDisableField() { + return true; + } + + + /** + * Return an index string which uniquely identifies this field. + * + * @return string Index string which uniquely identifies this field. + * @task core + */ + final public function getFieldIndex() { + return PhabricatorHash::digestForIndex($this->getFieldKey()); + } + + /* -( Contextual Data )---------------------------------------------------- */ + /** + * Sets the object this field belongs to. + * + * @param PhabricatorCustomFieldInterface The object this field belongs to. + * @task context + */ + final public function setObject(PhabricatorCustomFieldInterface $object) { + $this->object = $object; + return $this; + } + + + /** + * Get the object this field belongs to. + * + * @return PhabricatorCustomFieldInterface The object this field belongs to. + * @task context + */ + final public function getObject() { + return $this->object; + } + + /** * @task context */ @@ -143,6 +352,19 @@ abstract class PhabricatorCustomField { } + /** + * Return a new, empty storage object. This should be a subclass of + * @{class:PhabricatorCustomFieldStorage} which is bound to the application's + * database. + * + * @return PhabricatorCustomFieldStorage New empty storage object. + * @task storage + */ + public function getStorageObject() { + throw new PhabricatorCustomFieldImplementationIncompleteException($this); + } + + /** * Return a serialized representation of the field value, appropriate for * storing in auxiliary field storage. You must implement this method if @@ -277,4 +499,98 @@ abstract class PhabricatorCustomField { ->setIndexValue($value); } + +/* -( ApplicationTransactions )-------------------------------------------- */ + + + /** + * Appearing in ApplicationTrasactions allows a field to be edited using + * standard workflows. + * + * @return bool True to appear in ApplicationTransactions. + * @task appxaction + */ + public function shouldAppearInApplicationTransactions() { + return false; + } + + + /** + * @task appxaction + */ + public function getOldValueForApplicationTransactions() { + return $this->getValueForStorage(); + } + + + /** + * @task appxaction + */ + public function setValueFromApplicationTransactions($value) { + return $this->setValueFromStorage($value); + } + + + /** + * @task appxaction + */ + public function getNewValueForApplicationTransactions( + PhabricatorApplicationTransaction $xaction) { + return $xaction->getNewValue(); + } + + + /** + * @task appxaction + */ + public function getApplicationTransactionHasEffect( + PhabricatorApplicationTransaction $xaction) { + return ($xaction->getOldValue() !== $xaction->getNewValue()); + } + + + /** + * @task appxaction + */ + public function applyApplicationTransactionInternalEffects( + PhabricatorApplicationTransaction $xaction) { + return; + } + + + /** + * @task appxaction + */ + public function applyApplicationTransactionExternalEffects( + PhabricatorApplicationTransaction $xaction) { + if (!$this->shouldEnableForRole(self::ROLE_STORAGE)) { + return; + } + + $this->setValueFromApplicationTransaction($xaction->getNewValue()); + $value = $this->getValueForStorage(); + + $table = $this->newStorageObject(); + $conn_w = $table->establishConnection('w'); + + if ($value === null) { + queryfx( + $conn_w, + 'DELETE FROM %T WHERE objectPHID = %s AND fieldIndex = %s', + $this->getObject()->getPHID(), + $this->getFieldIndex()); + } else { + queryfx( + $conn_w, + 'INSERT INTO %T (objectPHID, fieldIndex, fieldValue) + VALUES (%s, %s, %s) + ON DUPLICATE KEY UPDATE fieldValue = VALUES(fieldValue)', + $this->getObject()->getPHID(), + $this->getFieldIndex(), + $value); + } + + return; + } + } diff --git a/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php b/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php new file mode 100644 index 0000000000..1695f6468b --- /dev/null +++ b/src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php @@ -0,0 +1,41 @@ +>>); + } + + public function getCustomFieldBaseClass() { + return <<<<'YourApplicationHereCustomField'>>>>; + } + + public function getCustomFields($role) { + if (idx($this->customFields, $role) === null) { + PhabricatorCustomField::raiseUnattachedException($this, $role); + } + return $this->customFields; + } + + public function attachCustomFields($role, array $fields) { + $this->customFields[$role] = $fields; + return $this; + } + +*/