Api4 - Use explicit adder functions rather than magicMethod

Civi/API/Kernel.php

@@ -132,12 +132,12 @@ public function runAuthorize($entity, $action, $params, $extra = NULL) {
   }
 
   /**
-   * Execute an API request.
+   * Execute an API v3 or v4 request.
    *
    * The request must be in canonical format. Exceptions will be propagated out.
    *
-   * @param array $apiRequest
-   * @return array
+   * @param array|\Civi\Api4\Generic\AbstractAction $apiRequest
+   * @return array|\Civi\Api4\Generic\Result
    * @throws \API_Exception
    * @throws \Civi\API\Exception\NotImplementedException
    * @throws \Civi\API\Exception\UnauthorizedException

Civi/Api4/Action/Setting/Get.php

@@ -26,8 +26,7 @@
  * Get the value of one or more CiviCRM settings.
  *
  * @method array getSelect
- * @method $this addSelect(string $name)
- * @method $this setSelect(array $select)
+ * @method $this setSelect(array $settingNames)
  */
 class Get extends AbstractSettingAction {
 
@@ -71,4 +70,14 @@ protected function processSettings(Result $result, $settingsBag, $meta, $domain)
     }
   }
 
+  /**
+   * Add one or more settings to be selected
+   * @param string ...$settingNames
+   * @return $this
+   */
+  public function addSelect(string ...$settingNames) {
+    $this->select = array_merge($this->select, $settingNames);
+    return $this;
+  }
+
 }

Civi/Api4/Action/Setting/Revert.php

@@ -26,8 +26,7 @@
  * Revert one or more CiviCRM settings to their default value.
  *
  * @method array getSelect
- * @method $this addSelect(string $name)
- * @method $this setSelect(array $select)
+ * @method $this setSelect(array $settingNames) Set settings to be reverted
  */
 class Revert extends AbstractSettingAction {
 
@@ -62,4 +61,14 @@ protected function processSettings(Result $result, $settingsBag, $meta, $domain)
     }
   }
 
+  /**
+   * Add one or more settings to be reverted
+   * @param string ...$settingNames
+   * @return $this
+   */
+  public function addSelect(string ...$settingNames) {
+    $this->select = array_merge($this->select, $settingNames);
+    return $this;
+  }
+
 }

Civi/Api4/Action/Setting/Set.php

@@ -27,7 +27,6 @@
  *
  * @method array getValues
  * @method $this setValues(array $value)
- * @method $this addValue(string $name, mixed $value)
  */
 class Set extends AbstractSettingAction {
 
@@ -60,4 +59,15 @@ protected function processSettings(Result $result, $settingsBag, $meta, $domain)
     }
   }
 
+  /**
+   * Add an item to the values array
+   * @param string $settingName
+   * @param mixed $value
+   * @return $this
+   */
+  public function addValue($settingName, $value) {
+    $this->values[$settingName] = $value;
+    return $this;
+  }
+
 }

Civi/Api4/Generic/AbstractAction.php

@@ -151,7 +151,7 @@ public function __set($name, $value) {
    * @throws \API_Exception
    */
   public function setVersion($val) {
-    if ($val != 4) {
+    if ($val !== 4 && $val !== '4') {
       throw new \API_Exception('Cannot modify api version');
     }
     return $this;
@@ -172,7 +172,7 @@ public function addChain($name, AbstractAction $apiRequest, $index = NULL) {
   }
 
   /**
-   * Magic function to provide addFoo, getFoo and setFoo for params.
+   * Magic function to provide automatic getter/setter for params.
    *
    * @param $name
    * @param $arguments
@@ -185,10 +185,6 @@ public function __call($name, $arguments) {
       throw new \API_Exception('Unknown api parameter: ' . $name);
     }
     $mode = substr($name, 0, 3);
-    // Handle plural when adding to e.g. $values with "addValue" method.
-    if ($mode == 'add' && $this->paramExists($param . 's')) {
-      $param .= 's';
-    }
     if ($this->paramExists($param)) {
       switch ($mode) {
         case 'get':
@@ -197,18 +193,6 @@ public function __call($name, $arguments) {
         case 'set':
           $this->$param = $arguments[0];
           return $this;
-
-        case 'add':
-          if (!is_array($this->$param)) {
-            throw new \API_Exception('Cannot add to non-array param');
-          }
-          if (array_key_exists(1, $arguments)) {
-            $this->{$param}[$arguments[0]] = $arguments[1];
-          }
-          else {
-            $this->{$param}[] = $arguments[0];
-          }
-          return $this;
       }
     }
     throw new \API_Exception('Unknown api parameter: ' . $name);
@@ -221,12 +205,12 @@ public function __call($name, $arguments) {
    * This is basically the outer wrapper for api v4.
    *
    * @return \Civi\Api4\Generic\Result
+   * @throws \API_Exception
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public function execute() {
     /** @var \Civi\API\Kernel $kernel */
     $kernel = \Civi::service('civi_api_kernel');
-
     $result = $kernel->runRequest($this);
     if ($this->debug && (!$this->checkPermissions || \CRM_Core_Permission::check('view debug output'))) {
       $result->debug = array_merge($result->debug, $this->_debugOutput);

Civi/Api4/Generic/AbstractCreateAction.php

@@ -25,7 +25,6 @@
  * Base class for all "Create" api actions.
  *
  * @method $this setValues(array $values) Set all field values from an array of key => value pairs.
- * @method $this addValue($field, $value) Set field value.
  * @method array getValues() Get field values.
  *
  * @package Civi\Api4\Generic
@@ -40,12 +39,22 @@ abstract class AbstractCreateAction extends AbstractAction {
   protected $values = [];
 
   /**
-   * @param string $key
-   *
+   * @param string $fieldName
    * @return mixed|null
    */
-  public function getValue($key) {
-    return isset($this->values[$key]) ? $this->values[$key] : NULL;
+  public function getValue(string $fieldName) {
+    return isset($this->values[$fieldName]) ? $this->values[$fieldName] : NULL;
+  }
+
+  /**
+   * Add a field value.
+   * @param string $fieldName
+   * @param mixed $value
+   * @return $this
+   */
+  public function addValue(string $fieldName, $value) {
+    $this->values[$fieldName] = $value;
+    return $this;
   }
 
   /**

Civi/Api4/Generic/AbstractGetAction.php

@@ -28,8 +28,7 @@
  *
  * @package Civi\Api4\Generic
  *
- * @method $this addSelect(string $select)
- * @method $this setSelect(array $selects)
+ * @method $this setSelect(array $selects) Set array of fields to be selected (wildcard * allowed)
  * @method array getSelect()
  */
 abstract class AbstractGetAction extends AbstractQueryAction {
@@ -154,4 +153,14 @@ protected function _whereContains($field, $clauses = NULL) {
     return FALSE;
   }
 
+  /**
+   * Add one or more fields to be selected (wildcard * allowed)
+   * @param string ...$fieldNames
+   * @return $this
+   */
+  public function addSelect(string ...$fieldNames) {
+    $this->select = array_merge($this->select, $fieldNames);
+    return $this;
+  }
+
 }

Civi/Api4/Generic/AbstractQueryAction.php

@@ -79,17 +79,17 @@ abstract class AbstractQueryAction extends AbstractAction {
   protected $offset = 0;
 
   /**
-   * @param string $field
+   * @param string $fieldName
    * @param string $op
    * @param mixed $value
    * @return $this
    * @throws \API_Exception
    */
-  public function addWhere($field, $op, $value = NULL) {
+  public function addWhere(string $fieldName, string $op, $value = NULL) {
     if (!in_array($op, \CRM_Core_DAO::acceptedSQLOperators())) {
       throw new \API_Exception('Unsupported operator');
     }
-    $this->where[] = [$field, $op, $value];
+    $this->where[] = [$fieldName, $op, $value];
     return $this;
   }
 
@@ -103,7 +103,7 @@ public function addWhere($field, $op, $value = NULL) {
    * @return $this
    * @throws \API_Exception
    */
-  public function addClause($operator, $condition1) {
+  public function addClause(string $operator, $condition1) {
     if (!is_array($condition1[0])) {
       $condition1 = array_slice(func_get_args(), 1);
     }
@@ -112,17 +112,18 @@ public function addClause($operator, $condition1) {
   }
 
   /**
-   * @param string $field
+   * Adds to the orderBy clause
+   * @param string $fieldName
    * @param string $direction
    * @return $this
    */
-  public function addOrderBy($field, $direction = 'ASC') {
-    $this->orderBy[$field] = $direction;
+  public function addOrderBy(string $fieldName, $direction = 'ASC') {
+    $this->orderBy[$fieldName] = $direction;
     return $this;
   }
 
   /**
-   * A human-readable where clause, for the reading enjoyment of you humans.
+   * Produces a human-readable where clause, for the reading enjoyment of you humans.
    *
    * @param array $whereClause
    * @param string $op

Civi/Api4/Generic/AbstractSaveAction.php

@@ -24,11 +24,9 @@
 /**
  * Base class for all "Save" api actions.
  *
- * @method $this setRecords(array $records) Array of records.
- * @method $this addRecord($record) Add a record to update.
+ * @method $this setRecords(array $records) Set array of records to be saved.
  * @method array getRecords()
  * @method $this setDefaults(array $defaults) Array of defaults.
- * @method $this addDefault($name, $value) Add a default value.
  * @method array getDefaults()
  * @method $this setReload(bool $reload) Specify whether complete objects will be returned after saving.
  * @method bool getReload()
@@ -106,4 +104,25 @@ protected function getIdField() {
     return $this->idField;
   }
 
+  /**
+   * Add one or more records to be saved.
+   * @param array ...$records
+   * @return $this
+   */
+  public function addRecord(array ...$records) {
+    $this->records = array_merge($this->records, $records);
+    return $this;
+  }
+
+  /**
+   * Set default value for a field.
+   * @param string $fieldName
+   * @param mixed $defaultValue
+   * @return $this
+   */
+  public function addDefault(string $fieldName, $defaultValue) {
+    $this->defaults[$fieldName] = $defaultValue;
+    return $this;
+  }
+
 }

Civi/Api4/Generic/AbstractUpdateAction.php

@@ -25,7 +25,6 @@
  * Base class for all "Update" api actions
  *
  * @method $this setValues(array $values) Set all field values from an array of key => value pairs.
- * @method $this addValue($field, $value) Set field value.
  * @method array getValues() Get field values.
  * @method $this setReload(bool $reload) Specify whether complete objects will be returned after saving.
  * @method bool getReload()
@@ -53,12 +52,23 @@ abstract class AbstractUpdateAction extends AbstractBatchAction {
   protected $reload = FALSE;
 
   /**
-   * @param string $key
+   * @param string $fieldName
    *
    * @return mixed|null
    */
-  public function getValue($key) {
-    return isset($this->values[$key]) ? $this->values[$key] : NULL;
+  public function getValue(string $fieldName) {
+    return isset($this->values[$fieldName]) ? $this->values[$fieldName] : NULL;
+  }
+
+  /**
+   * Add an item to the values array
+   * @param string $fieldName
+   * @param mixed $value
+   * @return $this
+   */
+  public function addValue(string $fieldName, $value) {
+    $this->values[$fieldName] = $value;
+    return $this;
   }
 
 }

Civi/Api4/Generic/BasicGetFieldsAction.php

@@ -30,7 +30,6 @@
  * @method $this setLoadOptions(bool $value)
  * @method bool getLoadOptions()
  * @method $this setAction(string $value)
- * @method $this addValue(string $value)
  * @method $this setValues(array $values)
  * @method array getValues()
  */
@@ -127,6 +126,17 @@ public function getAction() {
     return $sub[$this->action] ?? $this->action;
   }
 
+  /**
+   * Add an item to the values array
+   * @param string $fieldName
+   * @param mixed $value
+   * @return $this
+   */
+  public function addValue(string $fieldName, $value) {
+    $this->values[$fieldName] = $value;
+    return $this;
+  }
+
   public function fields() {
     return [
       [