From 2bf220fd5b6025db6aa653227acc6d96fbf7cab9 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Fri, 28 May 2021 20:31:12 -0400 Subject: [PATCH] APIv4 - Refactor field options getter into a swappable callback function This allows any fieldSpec provider to change the callback used to build field options, or to add an ad-hoc field not in the schema with its own option getter. --- Civi/Api4/Action/CustomValue/GetFields.php | 4 +- Civi/Api4/Generic/DAOGetFieldsAction.php | 22 +- Civi/Api4/Service/Spec/CustomFieldSpec.php | 6 +- Civi/Api4/Service/Spec/FieldSpec.php | 190 ++++++------------ Civi/Api4/Service/Spec/SpecFormatter.php | 142 +++++++++++-- tests/phpunit/api/v4/Action/GetFieldsTest.php | 16 ++ .../phpunit/api/v4/Spec/SpecFormatterTest.php | 12 -- 7 files changed, 225 insertions(+), 167 deletions(-) diff --git a/Civi/Api4/Action/CustomValue/GetFields.php b/Civi/Api4/Action/CustomValue/GetFields.php index 9ba2981dd8..f685a3b312 100644 --- a/Civi/Api4/Action/CustomValue/GetFields.php +++ b/Civi/Api4/Action/CustomValue/GetFields.php @@ -12,8 +12,6 @@ namespace Civi\Api4\Action\CustomValue; -use Civi\Api4\Service\Spec\SpecFormatter; - /** * Get fields for a custom group. */ @@ -25,7 +23,7 @@ class GetFields extends \Civi\Api4\Generic\DAOGetFieldsAction { /** @var \Civi\Api4\Service\Spec\SpecGatherer $gatherer */ $gatherer = \Civi::container()->get('spec_gatherer'); $spec = $gatherer->getSpec('Custom_' . $this->getCustomGroup(), $this->getAction(), $this->includeCustom, $this->values); - return SpecFormatter::specToArray($spec->getFields($fields), $this->loadOptions); + return $this->specToArray($spec->getFields($fields)); } /** diff --git a/Civi/Api4/Generic/DAOGetFieldsAction.php b/Civi/Api4/Generic/DAOGetFieldsAction.php index 1ad7c3d3fb..9672a99c86 100644 --- a/Civi/Api4/Generic/DAOGetFieldsAction.php +++ b/Civi/Api4/Generic/DAOGetFieldsAction.php @@ -19,8 +19,6 @@ namespace Civi\Api4\Generic; -use Civi\Api4\Service\Spec\SpecFormatter; - /** * @inheritDoc * @method bool getIncludeCustom() @@ -48,7 +46,7 @@ class DAOGetFieldsAction extends BasicGetFieldsAction { $this->includeCustom = strpos(implode('', $fieldsToGet), '.') !== FALSE; } $spec = $gatherer->getSpec($this->getEntityName(), $this->getAction(), $this->includeCustom, $this->values); - $fields = SpecFormatter::specToArray($spec->getFields($fieldsToGet), $this->loadOptions, $this->values); + $fields = $this->specToArray($spec->getFields($fieldsToGet)); foreach ($fieldsToGet ?? [] as $fieldName) { if (empty($fields[$fieldName]) && strpos($fieldName, '.') !== FALSE) { $fkField = $this->getFkFieldSpec($fieldName, $fields); @@ -61,6 +59,24 @@ class DAOGetFieldsAction extends BasicGetFieldsAction { return $fields; } + /** + * @param \Civi\Api4\Service\Spec\FieldSpec[] $fields + * + * @return array + */ + protected function specToArray($fields) { + $fieldArray = []; + + foreach ($fields as $field) { + if ($this->loadOptions) { + $field->getOptions($this->values, $this->loadOptions, $this->checkPermissions); + } + $fieldArray[$field->getName()] = $field->toArray(); + } + + return $fieldArray; + } + /** * @param string $fieldName * @param array $fields diff --git a/Civi/Api4/Service/Spec/CustomFieldSpec.php b/Civi/Api4/Service/Spec/CustomFieldSpec.php index 0222473ae5..d64dff2e0b 100644 --- a/Civi/Api4/Service/Spec/CustomFieldSpec.php +++ b/Civi/Api4/Service/Spec/CustomFieldSpec.php @@ -23,17 +23,17 @@ class CustomFieldSpec extends FieldSpec { /** * @var int */ - protected $customFieldId; + public $customFieldId; /** * @var int */ - protected $customGroup; + public $customGroup; /** * @var string */ - protected $tableName; + public $tableName; /** * @inheritDoc diff --git a/Civi/Api4/Service/Spec/FieldSpec.php b/Civi/Api4/Service/Spec/FieldSpec.php index 2b24df11af..5bba209081 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -19,108 +19,111 @@ namespace Civi\Api4\Service\Spec; -use Civi\Api4\Utils\CoreUtil; - class FieldSpec { /** * @var mixed */ - protected $defaultValue; + public $defaultValue; /** * @var string */ - protected $name; + public $name; /** * @var string */ - protected $label; + public $label; /** * @var string */ - protected $title; + public $title; /** * @var string */ - protected $entity; + public $entity; /** * @var string */ - protected $description; + public $description; /** * @var bool */ - protected $required = FALSE; + public $required = FALSE; /** * @var bool */ - protected $requiredIf; + public $requiredIf; /** * @var array|bool */ - protected $options; + public $options; + + /** + * @var callable + */ + private $optionsCallback; /** * @var string */ - protected $dataType; + public $dataType; /** * @var string */ - protected $inputType; + public $inputType; /** * @var array */ - protected $inputAttrs = []; + public $inputAttrs = []; /** * @var string */ - protected $fkEntity; + public $fkEntity; /** * @var int */ - protected $serialize; + public $serialize; /** * @var string */ - protected $helpPre; + public $helpPre; /** * @var string */ - protected $helpPost; + public $helpPost; /** * @var array */ - protected $permission; + public $permission; /** * @var string */ - protected $columnName; + public $columnName; /** * @var bool */ - protected $readonly = FALSE; + public $readonly = FALSE; /** * @var callable[] */ - protected $outputFormatters = []; + public $outputFormatters = []; /** * Aliases for the valid data types @@ -458,122 +461,48 @@ class FieldSpec { /** * @param array $values * @param array|bool $return + * @param bool $checkPermissions * @return array */ - public function getOptions($values = [], $return = TRUE) { - if (!isset($this->options) || $this->options === TRUE) { - $fieldName = $this->getName(); - - if ($this instanceof CustomFieldSpec) { - // buildOptions relies on the custom_* type of field names - $fieldName = sprintf('custom_%d', $this->getCustomFieldId()); - } - - // BAO::buildOptions returns a single-dimensional list, we call that first because of the hook contract, - // @see CRM_Utils_Hook::fieldOptions - // We then supplement the data with additional properties if requested. - $bao = CoreUtil::getBAOFromApiName($this->getEntity()); - $optionLabels = $bao::buildOptions($fieldName, NULL, $values); - - if (!is_array($optionLabels) || !$optionLabels) { - $this->options = FALSE; + public function getOptions($values = [], $return = TRUE, $checkPermissions = TRUE) { + if (!isset($this->options)) { + if ($this->optionsCallback) { + $this->options = ($this->optionsCallback)($this, $values, $return, $checkPermissions); } else { - $this->options = \CRM_Utils_Array::makeNonAssociative($optionLabels, 'id', 'label'); - if (is_array($return)) { - self::addOptionProps($bao, $fieldName, $values, $return); - } + $this->options = FALSE; } } return $this->options; } /** - * Augment the 2 values returned by BAO::buildOptions (id, label) with extra properties (name, description, color, icon, etc). - * - * We start with BAO::buildOptions in order to respect hooks which may be adding/removing items, then we add the extra data. + * @param array|bool $options * - * @param \CRM_Core_DAO $baoName - * @param string $fieldName - * @param array $values - * @param array $return - */ - private function addOptionProps($baoName, $fieldName, $values, $return) { - // FIXME: For now, call the buildOptions function again and then combine the arrays. Not an ideal approach. - // TODO: Teach CRM_Core_Pseudoconstant to always load multidimensional option lists so we can get more properties like 'color' and 'icon', - // however that might require a change to the hook_civicrm_fieldOptions signature so that's a bit tricky. - if (in_array('name', $return)) { - $props['name'] = $baoName::buildOptions($fieldName, 'validate', $values); - } - $return = array_diff($return, ['id', 'name', 'label']); - // CRM_Core_Pseudoconstant doesn't know how to fetch extra stuff like icon, description, color, etc., so we have to invent that wheel here... - if ($return) { - $optionIds = implode(',', array_column($this->options, 'id')); - $optionIndex = array_flip(array_column($this->options, 'id')); - if ($this instanceof CustomFieldSpec) { - $optionGroupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $this->getCustomFieldId(), 'option_group_id'); - } - else { - $dao = new $baoName(); - $fieldSpec = $dao->getFieldSpec($fieldName); - $pseudoconstant = $fieldSpec['pseudoconstant'] ?? NULL; - $optionGroupName = $pseudoconstant['optionGroupName'] ?? NULL; - $optionGroupId = $optionGroupName ? \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', $optionGroupName, 'id', 'name') : NULL; - } - if (!empty($optionGroupId)) { - $extraStuff = \CRM_Core_BAO_OptionValue::getOptionValuesArray($optionGroupId); - $keyColumn = $pseudoconstant['keyColumn'] ?? 'value'; - foreach ($extraStuff as $item) { - if (isset($optionIndex[$item[$keyColumn]])) { - foreach ($return as $ret) { - // Note: our schema is inconsistent about whether `description` fields allow html, - // but it's usually assumed to be plain text, so we strip_tags() to standardize it. - $this->options[$optionIndex[$item[$keyColumn]]][$ret] = ($ret === 'description' && isset($item[$ret])) ? strip_tags($item[$ret]) : $item[$ret] ?? NULL; - } - } - } - } - else { - // Fetch the abbr if requested using context: abbreviate - if (in_array('abbr', $return)) { - $props['abbr'] = $baoName::buildOptions($fieldName, 'abbreviate', $values); - $return = array_diff($return, ['abbr']); - } - // Fetch anything else (color, icon, description) - if ($return && !empty($pseudoconstant['table']) && \CRM_Utils_Rule::commaSeparatedIntegers($optionIds)) { - $sql = "SELECT * FROM {$pseudoconstant['table']} WHERE id IN (%1)"; - $query = \CRM_Core_DAO::executeQuery($sql, [1 => [$optionIds, 'CommaSeparatedIntegers']]); - while ($query->fetch()) { - foreach ($return as $ret) { - if (property_exists($query, $ret)) { - // Note: our schema is inconsistent about whether `description` fields allow html, - // but it's usually assumed to be plain text, so we strip_tags() to standardize it. - $this->options[$optionIndex[$query->id]][$ret] = $ret === 'description' ? strip_tags($query->$ret) : $query->$ret; - } - } - } - } - } - } - if (isset($props)) { - foreach ($this->options as &$option) { - foreach ($props as $name => $prop) { - $option[$name] = $prop[$option['id']] ?? NULL; - } - } - } + * @return $this + */ + public function setOptions($options) { + $this->options = $options; + return $this; } /** - * @param array|bool $options + * @param callable $callback * * @return $this */ - public function setOptions($options) { - $this->options = $options; + public function setOptionsCallback($callback) { + $this->optionsCallback = $callback; return $this; } + /** + * @return callable + */ + public function getOptionsCallback() { + return $this->optionsCallback; + } + /** * @return string */ @@ -610,16 +539,29 @@ class FieldSpec { } /** - * @param array $values + * Gets all public variables, converted to snake_case + * * @return array */ - public function toArray($values = []) { + public function toArray() { + // Anonymous class will only have access to public vars + $getter = new class { + + function getPublicVars($object) { + return get_object_vars($object); + } + + }; + + // If getOptions was never called, make options a boolean + if (!isset($this->options)) { + $this->options = isset($this->optionsCallback); + } + $ret = []; - foreach (get_object_vars($this) as $key => $val) { + foreach ($getter->getPublicVars($this) as $key => $val) { $key = strtolower(preg_replace('/(?=[A-Z])/', '_$0', $key)); - if (!$values || in_array($key, $values)) { - $ret[$key] = $val; - } + $ret[$key] = $val; } return $ret; } diff --git a/Civi/Api4/Service/Spec/SpecFormatter.php b/Civi/Api4/Service/Spec/SpecFormatter.php index d7d474b585..bc1451f6fb 100644 --- a/Civi/Api4/Service/Spec/SpecFormatter.php +++ b/Civi/Api4/Service/Spec/SpecFormatter.php @@ -19,30 +19,11 @@ namespace Civi\Api4\Service\Spec; +use Civi\Api4\Utils\CoreUtil; use CRM_Core_DAO_AllCoreTables as AllCoreTables; class SpecFormatter { - /** - * @param FieldSpec[] $fields - * @param bool $includeFieldOptions - * @param array $values - * - * @return array - */ - public static function specToArray($fields, $includeFieldOptions = FALSE, $values = []) { - $fieldArray = []; - - foreach ($fields as $field) { - if ($includeFieldOptions) { - $field->getOptions($values, $includeFieldOptions); - } - $fieldArray[$field->getName()] = $field->toArray(); - } - - return $fieldArray; - } - /** * @param array $data * @param string $entity @@ -67,7 +48,9 @@ class SpecFormatter { $field->setLabel($data['custom_group.title'] . ': ' . $data['label']); $field->setHelpPre($data['help_pre'] ?? NULL); $field->setHelpPost($data['help_post'] ?? NULL); - $field->setOptions(self::customFieldHasOptions($data)); + if (self::customFieldHasOptions($data)) { + $field->setOptionsCallback([__CLASS__, 'getOptions']); + } $field->setreadonly($data['is_view']); } else { @@ -76,7 +59,9 @@ class SpecFormatter { $field->setRequired(!empty($data['required'])); $field->setTitle($data['title'] ?? NULL); $field->setLabel($data['html']['label'] ?? NULL); - $field->setOptions(!empty($data['pseudoconstant'])); + if (!empty($data['pseudoconstant'])) { + $field->setOptionsCallback([__CLASS__, 'getOptions']); + } $field->setreadonly(!empty($data['readonly'])); } $field->setSerialize($data['serialize'] ?? NULL); @@ -135,6 +120,119 @@ class SpecFormatter { return $dataTypeName; } + /** + * Callback function to build option lists for all DAO & custom fields. + * + * @param FieldSpec $spec + * @param array $values + * @param bool|array $returnFormat + * @param bool $checkPermissions + * @return array|false + */ + public static function getOptions($spec, $values, $returnFormat, $checkPermissions) { + $fieldName = $spec->getName(); + + if ($spec instanceof CustomFieldSpec) { + // buildOptions relies on the custom_* type of field names + $fieldName = sprintf('custom_%d', $spec->getCustomFieldId()); + } + + // BAO::buildOptions returns a single-dimensional list, we call that first because of the hook contract, + // @see CRM_Utils_Hook::fieldOptions + // We then supplement the data with additional properties if requested. + $bao = CoreUtil::getBAOFromApiName($spec->getEntity()); + $optionLabels = $bao::buildOptions($fieldName, NULL, $values); + + if (!is_array($optionLabels) || !$optionLabels) { + $options = FALSE; + } + else { + $options = \CRM_Utils_Array::makeNonAssociative($optionLabels, 'id', 'label'); + if (is_array($returnFormat)) { + self::addOptionProps($options, $spec, $bao, $fieldName, $values, $returnFormat); + } + } + return $options; + } + + /** + * Augment the 2 values returned by BAO::buildOptions (id, label) with extra properties (name, description, color, icon, etc). + * + * We start with BAO::buildOptions in order to respect hooks which may be adding/removing items, then we add the extra data. + * + * @param array $options + * @param FieldSpec $spec + * @param \CRM_Core_DAO $baoName + * @param string $fieldName + * @param array $values + * @param array $returnFormat + */ + private static function addOptionProps(&$options, $spec, $baoName, $fieldName, $values, $returnFormat) { + // FIXME: For now, call the buildOptions function again and then combine the arrays. Not an ideal approach. + // TODO: Teach CRM_Core_Pseudoconstant to always load multidimensional option lists so we can get more properties like 'color' and 'icon', + // however that might require a change to the hook_civicrm_fieldOptions signature so that's a bit tricky. + if (in_array('name', $returnFormat)) { + $props['name'] = $baoName::buildOptions($fieldName, 'validate', $values); + } + $returnFormat = array_diff($returnFormat, ['id', 'name', 'label']); + // CRM_Core_Pseudoconstant doesn't know how to fetch extra stuff like icon, description, color, etc., so we have to invent that wheel here... + if ($returnFormat) { + $optionIds = implode(',', array_column($options, 'id')); + $optionIndex = array_flip(array_column($options, 'id')); + if ($spec instanceof CustomFieldSpec) { + $optionGroupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $spec->getCustomFieldId(), 'option_group_id'); + } + else { + $dao = new $baoName(); + $fieldSpec = $dao->getFieldSpec($fieldName); + $pseudoconstant = $fieldSpec['pseudoconstant'] ?? NULL; + $optionGroupName = $pseudoconstant['optionGroupName'] ?? NULL; + $optionGroupId = $optionGroupName ? \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', $optionGroupName, 'id', 'name') : NULL; + } + if (!empty($optionGroupId)) { + $extraStuff = \CRM_Core_BAO_OptionValue::getOptionValuesArray($optionGroupId); + $keyColumn = $pseudoconstant['keyColumn'] ?? 'value'; + foreach ($extraStuff as $item) { + if (isset($optionIndex[$item[$keyColumn]])) { + foreach ($returnFormat as $ret) { + // Note: our schema is inconsistent about whether `description` fields allow html, + // but it's usually assumed to be plain text, so we strip_tags() to standardize it. + $options[$optionIndex[$item[$keyColumn]]][$ret] = ($ret === 'description' && isset($item[$ret])) ? strip_tags($item[$ret]) : $item[$ret] ?? NULL; + } + } + } + } + else { + // Fetch the abbr if requested using context: abbreviate + if (in_array('abbr', $returnFormat)) { + $props['abbr'] = $baoName::buildOptions($fieldName, 'abbreviate', $values); + $returnFormat = array_diff($returnFormat, ['abbr']); + } + // Fetch anything else (color, icon, description) + if ($returnFormat && !empty($pseudoconstant['table']) && \CRM_Utils_Rule::commaSeparatedIntegers($optionIds)) { + $sql = "SELECT * FROM {$pseudoconstant['table']} WHERE id IN (%1)"; + $query = \CRM_Core_DAO::executeQuery($sql, [1 => [$optionIds, 'CommaSeparatedIntegers']]); + while ($query->fetch()) { + foreach ($returnFormat as $ret) { + if (property_exists($query, $ret)) { + // Note: our schema is inconsistent about whether `description` fields allow html, + // but it's usually assumed to be plain text, so we strip_tags() to standardize it. + $options[$optionIndex[$query->id]][$ret] = $ret === 'description' ? strip_tags($query->$ret) : $query->$ret; + } + } + } + } + } + } + if (isset($props)) { + foreach ($options as &$option) { + foreach ($props as $name => $prop) { + $option[$name] = $prop[$option['id']] ?? NULL; + } + } + } + } + /** * @param \Civi\Api4\Service\Spec\FieldSpec $fieldSpec * @param array $data diff --git a/tests/phpunit/api/v4/Action/GetFieldsTest.php b/tests/phpunit/api/v4/Action/GetFieldsTest.php index 734b78a754..5c442941b0 100644 --- a/tests/phpunit/api/v4/Action/GetFieldsTest.php +++ b/tests/phpunit/api/v4/Action/GetFieldsTest.php @@ -20,12 +20,28 @@ namespace api\v4\Action; use api\v4\UnitTestCase; +use Civi\Api4\Contact; /** * @group headless */ class GetFieldsTest extends UnitTestCase { + public function testOptionsAreReturned() { + $fields = Contact::getFields(FALSE) + ->execute() + ->indexBy('name'); + $this->assertTrue($fields['gender_id']['options']); + $this->assertFalse($fields['first_name']['options']); + + $fields = Contact::getFields(FALSE) + ->setLoadOptions(TRUE) + ->execute() + ->indexBy('name'); + $this->assertTrue(is_array($fields['gender_id']['options'])); + $this->assertFalse($fields['first_name']['options']); + } + public function testComponentFields() { \CRM_Core_BAO_ConfigSetting::disableComponent('CiviCampaign'); $fields = \Civi\Api4\Event::getFields() diff --git a/tests/phpunit/api/v4/Spec/SpecFormatterTest.php b/tests/phpunit/api/v4/Spec/SpecFormatterTest.php index b0d805ea31..30c5bde346 100644 --- a/tests/phpunit/api/v4/Spec/SpecFormatterTest.php +++ b/tests/phpunit/api/v4/Spec/SpecFormatterTest.php @@ -20,8 +20,6 @@ namespace api\v4\Spec; use Civi\Api4\Service\Spec\CustomFieldSpec; -use Civi\Api4\Service\Spec\FieldSpec; -use Civi\Api4\Service\Spec\RequestSpec; use Civi\Api4\Service\Spec\SpecFormatter; use api\v4\UnitTestCase; @@ -30,16 +28,6 @@ use api\v4\UnitTestCase; */ class SpecFormatterTest extends UnitTestCase { - public function testSpecToArray() { - $spec = new RequestSpec('Contact', 'get'); - $fieldName = 'last_name'; - $field = new FieldSpec($fieldName, 'Contact'); - $spec->addFieldSpec($field); - $arraySpec = SpecFormatter::specToArray($spec->getFields()); - - $this->assertEquals('String', $arraySpec[$fieldName]['data_type']); - } - /** * @dataProvider arrayFieldSpecProvider * -- 2.25.1