APIv4 - Filer out unknown fields in BasicGetAction

A 'known' field is one that has been declared in the `getFields` action. Before: BasicGetAction would default to returning only known fields, but others could be added to the `select` clause and would be returned. After: BasicGetAction only ever returns known fields. Unknown fields are filtered out even if they are returned by the getter function.
civicrm · Feb 15, 2022 · 3c8783b · 3c8783b
1 parent 6d0b4f6
commit 3c8783b
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 32 deletions.
diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php
@@ -81,7 +81,7 @@ public static function getFields($checkPermissions = TRUE) {
         ],
         [
           'name' => 'primary_key',
-          'type' => 'Array',
+          'data_type' => 'Array',
           'description' => 'Name of unique identifier field(s) (e.g. [id])',
         ],
         [

diff --git a/Civi/Api4/Generic/BasicGetAction.php b/Civi/Api4/Generic/BasicGetAction.php
@@ -50,7 +50,7 @@ public function _run(Result $result) {
     $this->setDefaultWhereClause();
     $this->expandSelectClauseWildcards();
     $values = $this->getRecords();
-    $this->formatRawValues($values);
+    $values = $this->formatRawValues($values);
     $this->queryArray($values, $result);
   }
 
@@ -101,25 +101,34 @@ protected function getRecords() {
    * @param $records
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
+   * @return array
    */
-  protected function formatRawValues(&$records) {
+  private function formatRawValues($records): array {
     // Pad $records and $fields with pseudofields
     $fields = $this->entityFields();
-    foreach ($records as &$values) {
-      foreach ($this->entityFields() as $field) {
-        $values += [$field['name'] => NULL];
+    $values = [];
+    foreach ($records as $idx => $row) {
+      $values[$idx] = [];
+      foreach ($fields as $name => $field) {
+        $values[$idx][$name] = $row[$name] ?? NULL;
         if (!empty($field['options'])) {
           foreach (FormattingUtil::$pseudoConstantSuffixes as $suffix) {
-            $pseudofield = $field['name'] . ':' . $suffix;
-            if (!isset($values[$pseudofield]) && isset($values[$field['name']]) && $this->_isFieldSelected($pseudofield)) {
-              $values[$pseudofield] = $values[$field['name']];
+            $pseudofield = $name . ':' . $suffix;
+            // In some (nonstandard) cases the getter may supply already matched pseudoconstant values
+            if (isset($row[$pseudofield])) {
+              $values[$idx][$pseudofield] = $row[$pseudofield];
+            }
+            // Standard case - replace
+            elseif (isset($row[$name]) && $this->_isFieldSelected($pseudofield)) {
+              $values[$idx][$pseudofield] = $row[$name];
             }
           }
         }
       }
     }
     // Swap raw values with pseudoconstants
-    FormattingUtil::formatOutputValues($records, $fields, $this->getActionName());
+    FormattingUtil::formatOutputValues($values, $fields, $this->getActionName());
+    return $values;
   }
 
 }
diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php
@@ -209,25 +209,27 @@ public static function formatOutputValues(&$results, $fields, $action = 'get', $
           self::applyFormatters($result, $fieldName, $field, $value);
           $dataType = NULL;
         }
-        // Evaluate pseudoconstant suffixes
-        $suffix = strrpos($fieldName, ':');
-        if ($suffix) {
-          $fieldOptions[$fieldName] = $fieldOptions[$fieldName] ?? self::getPseudoconstantList($field, $fieldName, $result, $action);
-          $dataType = NULL;
-        }
-        if ($fieldExpr->supportsExpansion) {
-          if (!empty($field['serialize']) && is_string($value)) {
-            $value = \CRM_Core_DAO::unSerializeField($value, $field['serialize']);
+        if ($value !== NULL) {
+          // Evaluate pseudoconstant suffixes
+          $suffix = strrpos($fieldName, ':');
+          if ($suffix) {
+            $fieldOptions[$fieldName] = $fieldOptions[$fieldName] ?? self::getPseudoconstantList($field, $fieldName, $result, $action);
+            $dataType = NULL;
+          }
+          if ($fieldExpr->supportsExpansion) {
+            if (!empty($field['serialize']) && is_string($value)) {
+              $value = \CRM_Core_DAO::unSerializeField($value, $field['serialize']);
+            }
+            if (isset($fieldOptions[$fieldName])) {
+              $value = self::replacePseudoconstant($fieldOptions[$fieldName], $value);
+            }
           }
-          if (isset($fieldOptions[$fieldName])) {
-            $value = self::replacePseudoconstant($fieldOptions[$fieldName], $value);
+          // Keep track of contact types for self::contactFieldsToRemove
+          if ($value && isset($field['entity']) && $field['entity'] === 'Contact' && $field['name'] === 'contact_type') {
+            $prefix = strrpos($fieldName, '.');
+            $contactTypePaths[$prefix ? substr($fieldName, 0, $prefix + 1) : ''] = $value;
           }
         }
-        // Keep track of contact types for self::contactFieldsToRemove
-        if ($value && isset($field['entity']) && $field['entity'] === 'Contact' && $field['name'] === 'contact_type') {
-          $prefix = strrpos($fieldName, '.');
-          $contactTypePaths[$prefix ? substr($fieldName, 0, $prefix + 1) : ''] = $value;
-        }
         $result[$key] = self::convertDataType($value, $dataType);
       }
       // Remove inapplicable contact fields

diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php
@@ -162,10 +162,10 @@ public function testReplace() {
 
   public function testBatchFrobnicate() {
     $objects = [
-      ['group' => 'one', 'color' => 'red', 'number' => 10],
-      ['group' => 'one', 'color' => 'blue', 'number' => 20],
-      ['group' => 'one', 'color' => 'green', 'number' => 30],
-      ['group' => 'two', 'color' => 'blue', 'number' => 40],
+      ['group' => 'one', 'color' => 'red', 'size' => 10],
+      ['group' => 'one', 'color' => 'blue', 'size' => 20],
+      ['group' => 'one', 'color' => 'green', 'size' => 30],
+      ['group' => 'two', 'color' => 'blue', 'size' => 40],
     ];
     $this->replaceRecords($objects);
 

diff --git a/tests/phpunit/api/v4/Mock/Api4/Action/MockBasicEntity/BatchFrobnicate.php b/tests/phpunit/api/v4/Mock/Api4/Action/MockBasicEntity/BatchFrobnicate.php
@@ -27,12 +27,12 @@ class BatchFrobnicate extends \Civi\Api4\Generic\BasicBatchAction {
   protected function doTask($item) {
     return [
       'identifier' => $item['identifier'],
-      'frobnication' => $item['number'] * $item['number'],
+      'frobnication' => $item['size'] * $item['size'],
     ];
   }
 
   protected function getSelect() {
-    return ['identifier', 'number'];
+    return ['identifier', 'size'];
   }
 
 }