APIv4 - Default select clause to exclude "Extra" fields

Recently a "type" property was added to getFieldSpec to allow "Extra"
calculated fields to be declared. This updates api Get to *not*
select those fields by default, as their calculation can be expensive.

Civi/Api4/Generic/AbstractGetAction.php

@@ -66,18 +66,32 @@ protected function setDefaultWhereClause() {
   }
 
   /**
-   * Adds all fields matched by the * wildcard
+   * Adds all standard fields matched by the * wildcard
+   *
+   * Note: this function only deals with simple wildcard expressions.
+   * It ignores those containing special characters like dots or parentheses,
+   * they are handled separately in Api4SelectQuery.
    *
    * @throws \API_Exception
    */
   protected function expandSelectClauseWildcards() {
+    if (!$this->select) {
+      $this->select = ['*'];
+    }
+    // Get expressions containing wildcards but no dots or parentheses
     $wildFields = array_filter($this->select, function($item) {
       return strpos($item, '*') !== FALSE && strpos($item, '.') === FALSE && strpos($item, '(') === FALSE && strpos($item, ' ') === FALSE;
     });
-    foreach ($wildFields as $item) {
-      $pos = array_search($item, array_values($this->select));
-      $matches = SelectUtil::getMatchingFields($item, array_column($this->entityFields(), 'name'));
-      array_splice($this->select, $pos, 1, $matches);
+    if ($wildFields) {
+      // Wildcards should not match "Extra" fields
+      $standardFields = array_filter(array_map(function($field) {
+        return $field['type'] === 'Extra' ? NULL : $field['name'];
+      }, $this->entityFields()));
+      foreach ($wildFields as $item) {
+        $pos = array_search($item, array_values($this->select));
+        $matches = SelectUtil::getMatchingFields($item, $standardFields);
+        array_splice($this->select, $pos, 1, $matches);
+      }
     }
     $this->select = array_unique($this->select);
   }

Civi/Api4/Generic/DAOGetAction.php

@@ -29,9 +29,9 @@ class DAOGetAction extends AbstractGetAction {
   use Traits\DAOActionTrait;
 
   /**
-   * Fields to return. Defaults to all non-custom fields `['*']`.
+   * Fields to return. Defaults to all standard (non-custom, non-extra) fields `['*']`.
    *
-   * The keyword `"custom.*"` selects all custom fields. So to select all core + custom fields, select `['*', 'custom.*']`.
+   * The keyword `"custom.*"` selects all custom fields. So to select all standard + custom fields, select `['*', 'custom.*']`.
    *
    * Use the dot notation to perform joins in the select clause, e.g. selecting `['*', 'contact.*']` from `Email::get()`
    * will select all fields for the email + all fields for the related contact.

ext/afform/core/Civi/Api4/Action/Afform/Get.php

@@ -17,7 +17,7 @@ class Get extends \Civi\Api4\Generic\BasicGetAction {
   public function getRecords() {
     /** @var \CRM_Afform_AfformScanner $scanner */
     $scanner = \Civi::service('afform_scanner');
-    $getComputed = $this->_isFieldSelected('has_local') || $this->_isFieldSelected('has_base');
+    $getComputed = $this->_isFieldSelected('has_local', 'has_base');
     $getLayout = $this->_isFieldSelected('layout');
     $values = [];
 

ext/afform/core/Civi/Api4/Afform.php

@@ -201,19 +201,23 @@ public static function getFields($checkPermissions = TRUE) {
       if ($self->getAction() === 'get') {
         $fields[] = [
           'name' => 'module_name',
+          'type' => 'Extra',
           'readonly' => TRUE,
         ];
         $fields[] = [
           'name' => 'directive_name',
+          'type' => 'Extra',
           'readonly' => TRUE,
         ];
         $fields[] = [
           'name' => 'has_local',
+          'type' => 'Extra',
           'data_type' => 'Boolean',
           'readonly' => TRUE,
         ];
         $fields[] = [
           'name' => 'has_base',
+          'type' => 'Extra',
           'data_type' => 'Boolean',
           'readonly' => TRUE,
         ];

ext/afform/core/tests/phpunit/Civi/Afform/AfformGetTest.php

@@ -0,0 +1,57 @@
+<?php
+namespace Civi\Afform;
+
+use Civi\Api4\Afform;
+use Civi\Test\HeadlessInterface;
+use Civi\Test\TransactionalInterface;
+
+/**
+ * @group headless
+ */
+class AfformGetTest extends \PHPUnit\Framework\TestCase implements HeadlessInterface, TransactionalInterface {
+
+  private $formName = 'abc_123_test';
+
+  public function setUpHeadless() {
+    return \Civi\Test::headless()->installMe(__DIR__)->apply();
+  }
+
+  public function tearDown(): void {
+    Afform::revert(FALSE)->addWhere('name', '=', $this->formName)->execute();
+    parent::tearDown();
+  }
+
+  public function testGetReturnFields() {
+    Afform::create()
+      ->addValue('name', $this->formName)
+      ->addValue('title', 'Test Form')
+      ->execute();
+
+    // Omitting select should return regular fields but not extra fields
+    $result = Afform::get()
+      ->addWhere('name', '=', $this->formName)
+      ->execute()->single();
+    $this->assertEquals($this->formName, $result['name']);
+    $this->assertArrayNotHasKey('directive_name', $result);
+    $this->assertArrayNotHasKey('has_base', $result);
+
+    // Select * should also return regular fields only
+    $result = Afform::get()
+      ->addSelect('*')
+      ->addWhere('name', '=', $this->formName)
+      ->execute()->single();
+    $this->assertEquals($this->formName, $result['name']);
+    $this->assertArrayNotHasKey('module_name', $result);
+    $this->assertArrayNotHasKey('has_local', $result);
+
+    // Selecting * and has_base should return core and that one extra field
+    $result = Afform::get()
+      ->addSelect('*', 'has_base')
+      ->addWhere('name', '=', $this->formName)
+      ->execute()->single();
+    $this->assertEquals($this->formName, $result['name']);
+    $this->assertFalse($result['has_base']);
+    $this->assertArrayNotHasKey('has_local', $result);
+  }
+
+}

ext/afform/mock/tests/phpunit/api/v4/AfformCustomFieldUsageTest.php

@@ -50,6 +50,7 @@ public function testMultiRecordCustomBlock(): void {
     // Creating a custom group should automatically create an afform block
     $block = \Civi\Api4\Afform::get()
       ->addWhere('name', '=', 'afblockCustom_MyThings')
+      ->addSelect('layout', 'directive_name')
       ->setLayoutFormat('shallow')
       ->setFormatWhitespace(TRUE)
       ->execute()->single();

ext/afform/mock/tests/phpunit/api/v4/AfformTest.php

@@ -53,7 +53,9 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
     Civi\Api4\Afform::revert()->addWhere('name', '=', $formName)->execute();
 
     $message = 'The initial Afform.get should return default data';
-    $result = Civi\Api4\Afform::get()->addWhere('name', '=', $formName)->execute();
+    $result = Civi\Api4\Afform::get()
+      ->addSelect('*', 'has_base', 'has_local')
+      ->addWhere('name', '=', $formName)->execute();
     $this->assertEquals($formName, $result[0]['name'], $message);
     $this->assertEquals($get($originalMetadata, 'title'), $get($result[0], 'title'), $message);
     $this->assertEquals($get($originalMetadata, 'description'), $get($result[0], 'description'), $message);
@@ -75,7 +77,9 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
     $this->assertEquals('The temporary description', $result[0]['description'], $message);
 
     $message = 'After updating, the Afform.get API should return blended data';
-    $result = Civi\Api4\Afform::get()->addWhere('name', '=', $formName)->execute();
+    $result = Civi\Api4\Afform::get()
+      ->addSelect('*', 'has_base', 'has_local')
+      ->addWhere('name', '=', $formName)->execute();
     $this->assertEquals($formName, $result[0]['name'], $message);
     $this->assertEquals($get($originalMetadata, 'title'), $get($result[0], 'title'), $message);
     $this->assertEquals('The temporary description', $get($result[0], 'description'), $message);
@@ -88,7 +92,9 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
 
     Civi\Api4\Afform::revert()->addWhere('name', '=', $formName)->execute();
     $message = 'After reverting, the final Afform.get should return default data';
-    $result = Civi\Api4\Afform::get()->addWhere('name', '=', $formName)->execute();
+    $result = Civi\Api4\Afform::get()
+      ->addSelect('*', 'has_base', 'has_local')
+      ->addWhere('name', '=', $formName)->execute();
     $this->assertEquals($formName, $result[0]['name'], $message);
     $this->assertEquals($get($originalMetadata, 'title'), $get($result[0], 'title'), $message);
     $this->assertEquals($get($originalMetadata, 'description'), $get($result[0], 'description'), $message);

ext/oauth-client/Civi/Api4/OAuthProvider.php

@@ -59,6 +59,9 @@ public static function getFields($checkPermissions = TRUE) {
         [
           'name' => 'options',
         ],
+        [
+          'name' => 'contactTemplate',
+        ],
       ];
     });
     return $action->setCheckPermissions($checkPermissions);

tests/phpunit/api/v4/Action/BasicActionsTest.php

@@ -158,7 +158,7 @@ public function testBatchFrobnicate() {
   public function testGetFields() {
     $getFields = MockBasicEntity::getFields()->execute()->indexBy('name');
 
-    $this->assertCount(7, $getFields);
+    $this->assertCount(8, $getFields);
     $this->assertEquals('Identifier', $getFields['identifier']['title']);
     // Ensure default data type is "String" when not specified
     $this->assertEquals('String', $getFields['color']['data_type']);

tests/phpunit/api/v4/Action/CurrentFilterTest.php

@@ -83,6 +83,12 @@ public function testCurrentRelationship() {
     $this->assertArrayNotHasKey($expiring['id'], $notCurrent);
     $this->assertArrayHasKey($past['id'], $notCurrent);
     $this->assertArrayHasKey($inactive['id'], $notCurrent);
+
+    // Assert that "Extra" fields like is_current are not returned with select *
+    $defaultGet = Relationship::get()->setLimit(1)->execute()->single();
+    $this->assertArrayNotHasKey('is_current', $defaultGet);
+    $starGet = Relationship::get()->addSelect('*')->setLimit(1)->execute()->single();
+    $this->assertArrayNotHasKey('is_current', $starGet);
   }
 
 }

tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php

@@ -60,6 +60,9 @@ public static function getFields($checkPermissions = TRUE) {
         [
           'name' => 'size',
         ],
+        [
+          'name' => 'foo',
+        ],
         [
           'name' => 'weight',
           'data_type' => 'Integer',