From 9bafff7c5f74d172f21de9ee776e08d25e071788 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Mon, 20 Jul 2020 14:30:35 -0400
Subject: [PATCH] APIv4 - Add BasicEntity helper class

---
 Civi/Api4/Generic/AbstractEntity.php          |  25 ++--
 Civi/Api4/Generic/BasicBatchAction.php        |   4 +-
 Civi/Api4/Generic/BasicCreateAction.php       |   4 +-
 Civi/Api4/Generic/BasicEntity.php             | 139 ++++++++++++++++++
 Civi/Api4/Generic/BasicGetAction.php          |  14 +-
 Civi/Api4/Generic/BasicSaveAction.php         |   4 +-
 Civi/Api4/Generic/BasicUpdateAction.php       |   4 +-
 Civi/Api4/Generic/DAOEntity.php               |  12 +-
 .../api/v4/Mock/Api4/MockArrayEntity.php      |  13 +-
 .../api/v4/Mock/Api4/MockBasicEntity.php      |  63 ++------
 10 files changed, 191 insertions(+), 91 deletions(-)
 create mode 100644 Civi/Api4/Generic/BasicEntity.php

diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php
index f2d0d26d08..269bcc31b3 100644
--- a/Civi/Api4/Generic/AbstractEntity.php
+++ b/Civi/Api4/Generic/AbstractEntity.php
@@ -24,23 +24,18 @@ use Civi\Api4\Utils\ReflectionUtils;
 /**
  * Base class for all api entities.
  *
- * When adding your own api from an extension, extend this class only
- * if your entity does not have an associated DAO. Otherwise extend DAOEntity.
+ * This is the most generic of 3 possible base classes for an APIv4 Entity
+ * (the other 2, which extend this class, are `BasicEntity` and `DAOEntity`).
  *
- * The recommended way to create a non-DAO-based api is to extend this class
- * and then add a getFields function and any other actions you wish, e.g.
- * - a get() function which returns BasicGetAction using your custom getter callback.
- * - a create() function which returns BasicCreateAction using your custom setter callback.
- * - a save() function which returns BasicSaveAction using your custom setter callback.
- * - an update() function which returns BasicUpdateAction using your custom setter callback.
- * - a delete() function which returns BasicBatchAction using your custom delete callback.
- * - a replace() function which returns BasicReplaceAction (no callback needed but
- *   depends on the existence of get, save & delete actions).
+ * Implementing an API by extending this class directly is appropriate when it does not implement
+ * all of the CRUD actions, or only a subset like `get` without `create`, `update` or `delete`;
+ * for example the RelationshipCache entity.
  *
- * Note that you can use the same setter callback function for update as create -
- * that function can distinguish between new & existing records by checking if the
- * unique identifier has been set (identifier field defaults to "id" but you can change
- * that when constructing BasicUpdateAction).
+ * For all other APIs that do implement CRUD it is recommended to use:
+ * 1. `DAOEntity` for all entities with a DAO (sql table).
+ * 2. `BasicEntity` for all others, e.g. file-based entities.
+ *
+ * An entity which extends this class directly must, at minimum, implement the `getFields` action.
  *
  * @see https://lab.civicrm.org/extensions/api4example
  */
diff --git a/Civi/Api4/Generic/BasicBatchAction.php b/Civi/Api4/Generic/BasicBatchAction.php
index bfd221131f..d1787bd44a 100644
--- a/Civi/Api4/Generic/BasicBatchAction.php
+++ b/Civi/Api4/Generic/BasicBatchAction.php
@@ -32,8 +32,7 @@ class BasicBatchAction extends AbstractBatchAction {
 
   /**
    * @var callable
-   *
-   * Function(array $item, BasicBatchAction $thisAction) => array
+   *   Function(array $item, BasicBatchAction $thisAction): array
    */
   private $doer;
 
@@ -52,7 +51,6 @@ class BasicBatchAction extends AbstractBatchAction {
    * @param string|array $select
    *   One or more fields to select from each matching item.
    * @param callable $doer
-   *   Function(array $item, BasicBatchAction $thisAction) => array
    */
   public function __construct($entityName, $actionName, $select = 'id', $doer = NULL) {
     parent::__construct($entityName, $actionName, $select);
diff --git a/Civi/Api4/Generic/BasicCreateAction.php b/Civi/Api4/Generic/BasicCreateAction.php
index 9f2209d121..6a5027d351 100644
--- a/Civi/Api4/Generic/BasicCreateAction.php
+++ b/Civi/Api4/Generic/BasicCreateAction.php
@@ -31,8 +31,7 @@ class BasicCreateAction extends AbstractCreateAction {
 
   /**
    * @var callable
-   *
-   * Function(array $item, BasicCreateAction $thisAction) => array
+   *   Function(array $item, BasicCreateAction $thisAction): array
    */
   private $setter;
 
@@ -42,7 +41,6 @@ class BasicCreateAction extends AbstractCreateAction {
    * @param string $entityName
    * @param string $actionName
    * @param callable $setter
-   *   Function(array $item, BasicCreateAction $thisAction) => array
    */
   public function __construct($entityName, $actionName, $setter = NULL) {
     parent::__construct($entityName, $actionName);
diff --git a/Civi/Api4/Generic/BasicEntity.php b/Civi/Api4/Generic/BasicEntity.php
new file mode 100644
index 0000000000..b582d29491
--- /dev/null
+++ b/Civi/Api4/Generic/BasicEntity.php
@@ -0,0 +1,139 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Generic;
+
+/**
+ * Base class for ad-hoc entities that implement CRUD actions.
+ *
+ * This is one of 3 possible base classes for an APIv4 Entity
+ * (the others are `DAOEntity` and `AbstractEntity`).
+ *
+ * This base class is for entities that do not have an associated DAO but do implement CRUD actions.
+ * It can be used in one of these ways:
+ *
+ * 1. Extend this class and define the static variables `$getter`, `$setter` & `$deleter` with callbacks to handle CRUD operations.
+ *    In that case there is no need to implement any actions other than `getFields`.
+ * 2. Override the `get`, `create`, `delete`, etc. methods with custom BasicAction implementations.
+ * 3. Some combination of the above two options, e.g. defining a callback for `$getter` and using the default `get` action,
+ *    but leaving `$deleter` unset and overriding the `delete` method with a custom BasicBatchAction to handle deletion.
+ *
+ * Note: the `replace` action does not require any callback as it internally calls the entity's `get`, `save` and `delete` actions.
+ */
+abstract class BasicEntity extends AbstractEntity {
+
+  /**
+   * Unique identifier for this entity.
+   *
+   * @var string
+   */
+  protected static $idField = 'id';
+
+  /**
+   * Function to read records. Used by `get` action.
+   *
+   * @var callable
+   *   Function(BasicGetAction $thisAction): array[]
+   *
+   * This function should return an array of records, and is passed a copy of the BasicGetAction object as its first argument.
+   * The simplest implementation is for it to return every record and the BasicGetAction automatically handle sorting and filtering.
+   *
+   * If performance is a concern, it can take advantage of some helper functions for e.g. fetching item(s) by id.
+   * @see BasicGetAction::getRecords()
+   */
+  protected static $getter;
+
+  /**
+   * Function to write a record. Used by `create`, `update` and `save`.
+   *
+   * @var callable
+   *   Function(array $item, BasicCreateAction|BasicSaveAction|BasicUpdateAction $thisAction): array
+   *
+   * This function is called once per write. It takes a single record as the first param, and a reference to
+   * the action object as the second.
+   *
+   * This callback should check the $idField of the record to determine whether the operation is a create or update.
+   *
+   * It should return the updated record as an array.
+   */
+  protected static $setter;
+
+  /**
+   * Function to delete records. Used by `delete` action.
+   *
+   * @var callable
+   *   Function(array $item, BasicBatchAction $thisAction): array
+   *
+   * This function is called once per delete. It takes a single record as the first param, and a reference to
+   * the action object as the second.
+   *
+   * This callback should check the $idField of the item to determine which record to delete.
+   *
+   * It should return the deleted record as an array.
+   */
+  protected static $deleter;
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicGetAction
+   */
+  public static function get($checkPermissions = TRUE) {
+    return (new BasicGetAction(static::class, __FUNCTION__, static::$getter))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicCreateAction
+   */
+  public static function create($checkPermissions = TRUE) {
+    return (new BasicCreateAction(static::class, __FUNCTION__, static::$setter))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicSaveAction
+   */
+  public static function save($checkPermissions = TRUE) {
+    return (new BasicSaveAction(static::class, __FUNCTION__, static::$idField, static::$setter))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicUpdateAction
+   */
+  public static function update($checkPermissions = TRUE) {
+    return (new BasicUpdateAction(static::class, __FUNCTION__, static::$idField, static::$setter))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicBatchAction
+   */
+  public static function delete($checkPermissions = TRUE) {
+    return (new BasicBatchAction(static::class, __FUNCTION__, static::$idField, static::$deleter))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param bool $checkPermissions
+   * @return BasicReplaceAction
+   */
+  public static function replace($checkPermissions = TRUE) {
+    return (new BasicReplaceAction(static::class, __FUNCTION__))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+}
diff --git a/Civi/Api4/Generic/BasicGetAction.php b/Civi/Api4/Generic/BasicGetAction.php
index e40215faa7..f436427c92 100644
--- a/Civi/Api4/Generic/BasicGetAction.php
+++ b/Civi/Api4/Generic/BasicGetAction.php
@@ -32,8 +32,7 @@ class BasicGetAction extends AbstractGetAction {
 
   /**
    * @var callable
-   *
-   * Function(BasicGetAction $thisAction) => array<array>
+   *   Function(BasicGetAction $thisAction): array[]
    */
   private $getter;
 
@@ -63,14 +62,15 @@ class BasicGetAction extends AbstractGetAction {
   }
 
   /**
-   * This Basic Get class is a general-purpose api for non-DAO-based entities.
+   * BasicGet is a general-purpose get action for non-DAO-based entities.
    *
    * Useful for fetching records from files or other places.
-   * You can specify any php function to retrieve the records, and this class will
-   * automatically filter, sort, select & limit the raw data from your callback.
+   * Specify any php function to retrieve the records, and this class will
+   * automatically filter, sort, select & limit the raw data from the callback.
    *
-   * You can implement this action in one of two ways:
-   * 1. Use this class directly by passing a callable ($getter) to the constructor.
+   * This action is implemented in one of two ways:
+   * 1. Invoke this class directly by passing a callable ($getter) to the constructor. BasicEntity does this by default.
+   *    The function is passed a copy of $this action as it's first argument.
    * 2. Extend this class and override this function.
    *
    * Either way, this function should return an array of arrays, each representing one retrieved object.
diff --git a/Civi/Api4/Generic/BasicSaveAction.php b/Civi/Api4/Generic/BasicSaveAction.php
index 38556e995c..4ddb4234df 100644
--- a/Civi/Api4/Generic/BasicSaveAction.php
+++ b/Civi/Api4/Generic/BasicSaveAction.php
@@ -32,8 +32,7 @@ class BasicSaveAction extends AbstractSaveAction {
 
   /**
    * @var callable
-   *
-   * Function(array $item, BasicCreateAction $thisAction) => array
+   *   Function(array $item, BasicCreateAction $thisAction): array
    */
   private $setter;
 
@@ -44,7 +43,6 @@ class BasicSaveAction extends AbstractSaveAction {
    * @param string $actionName
    * @param string $idField
    * @param callable $setter
-   *   Function(array $item, BasicCreateAction $thisAction) => array
    */
   public function __construct($entityName, $actionName, $idField = 'id', $setter = NULL) {
     parent::__construct($entityName, $actionName, $idField);
diff --git a/Civi/Api4/Generic/BasicUpdateAction.php b/Civi/Api4/Generic/BasicUpdateAction.php
index 57edbfba1f..a7bc7e9bb2 100644
--- a/Civi/Api4/Generic/BasicUpdateAction.php
+++ b/Civi/Api4/Generic/BasicUpdateAction.php
@@ -30,8 +30,7 @@ class BasicUpdateAction extends AbstractUpdateAction {
 
   /**
    * @var callable
-   *
-   * Function(array $item, BasicUpdateAction $thisAction) => array
+   *   Function(array $item, BasicUpdateAction $thisAction): array
    */
   private $setter;
 
@@ -43,7 +42,6 @@ class BasicUpdateAction extends AbstractUpdateAction {
    * @param string|array $select
    *   One or more fields to select from each matching item.
    * @param callable $setter
-   *   Function(array $item, BasicUpdateAction $thisAction) => array
    */
   public function __construct($entityName, $actionName, $select = 'id', $setter = NULL) {
     parent::__construct($entityName, $actionName, $select);
diff --git a/Civi/Api4/Generic/DAOEntity.php b/Civi/Api4/Generic/DAOEntity.php
index d291d22800..a7c47ee2e9 100644
--- a/Civi/Api4/Generic/DAOEntity.php
+++ b/Civi/Api4/Generic/DAOEntity.php
@@ -13,7 +13,17 @@
 namespace Civi\Api4\Generic;
 
 /**
- * Base class for DAO-based entities.
+ * Base class for DAO entities (sql tables).
+ *
+ * This is one of 3 possible base classes for an APIv4 Entity
+ * (the others are `BasicEntity` and `AbstractEntity`).
+ *
+ * This base class is used for entities that have an associated DAO and support CRUD operations.
+ *
+ * Entities that extend this class can override actions and add others on an ad-hoc basis.
+ *
+ * DAO entities which do not support all CRUD operations should instead extend AbstractEntity
+ * in order to implement just the actions appropriate to that entity.
  */
 abstract class DAOEntity extends AbstractEntity {
 
diff --git a/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
index d9ea166650..430d49bd0e 100644
--- a/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
+++ b/tests/phpunit/api/v4/Mock/Api4/MockArrayEntity.php
@@ -16,11 +16,8 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 
-
 namespace Civi\Api4;
 
-use Civi\Api4\Generic\BasicGetFieldsAction;
-
 /**
  * MockArrayEntity entity.
  *
@@ -30,8 +27,12 @@ use Civi\Api4\Generic\BasicGetFieldsAction;
  */
 class MockArrayEntity extends Generic\AbstractEntity {
 
-  public static function getFields() {
-    return new BasicGetFieldsAction(static::class, __FUNCTION__, function() {
+  /**
+   * @param bool $checkPermissions
+   * @return Generic\BasicGetFieldsAction
+   */
+  public static function getFields($checkPermissions = TRUE) {
+    return (new Generic\BasicGetFieldsAction(static::class, __FUNCTION__, function() {
       return [
         ['name' => 'field1'],
         ['name' => 'field2'],
@@ -40,7 +41,7 @@ class MockArrayEntity extends Generic\AbstractEntity {
         ['name' => 'field5'],
         ['name' => 'field6'],
       ];
-    });
+    }))->setCheckPermissions($checkPermissions);
   }
 
 }
diff --git a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
index b11a06c534..51c59220c3 100644
--- a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
+++ b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
@@ -26,13 +26,17 @@ use api\v4\Mock\MockEntityDataStorage;
  *
  * @package Civi\Api4
  */
-class MockBasicEntity extends Generic\AbstractEntity {
+class MockBasicEntity extends Generic\BasicEntity {
+
+  protected static $getter = [MockEntityDataStorage::CLASS, 'get'];
+  protected static $setter = [MockEntityDataStorage::CLASS, 'write'];
+  protected static $deleter = [MockEntityDataStorage::CLASS, 'delete'];
 
   /**
-   * @return Generic\BasicGetFieldsAction
+   * @inheritDoc
    */
-  public static function getFields() {
-    return new Generic\BasicGetFieldsAction(static::class, __FUNCTION__, function() {
+  public static function getFields($checkPermissions = TRUE) {
+    return (new Generic\BasicGetFieldsAction(__CLASS__, __FUNCTION__, function() {
       return [
         [
           'name' => 'id',
@@ -82,61 +86,20 @@ class MockBasicEntity extends Generic\AbstractEntity {
           ],
         ],
       ];
-    });
-  }
-
-  /**
-   * @return Generic\BasicGetAction
-   */
-  public static function get() {
-    return new Generic\BasicGetAction('MockBasicEntity', __FUNCTION__, [MockEntityDataStorage::CLASS, 'get']);
-  }
-
-  /**
-   * @return Generic\BasicCreateAction
-   */
-  public static function create() {
-    return new Generic\BasicCreateAction(static::class, __FUNCTION__, [MockEntityDataStorage::CLASS, 'write']);
-  }
-
-  /**
-   * @return Generic\BasicSaveAction
-   */
-  public static function save() {
-    return new Generic\BasicSaveAction(self::getEntityName(), __FUNCTION__, 'id', [MockEntityDataStorage::CLASS, 'write']);
-  }
-
-  /**
-   * @return Generic\BasicUpdateAction
-   */
-  public static function update() {
-    return new Generic\BasicUpdateAction(self::getEntityName(), __FUNCTION__, 'id', [MockEntityDataStorage::CLASS, 'write']);
+    }))->setCheckPermissions(TRUE);
   }
 
   /**
+   * @param bool $checkPermissions
    * @return Generic\BasicBatchAction
    */
-  public static function delete() {
-    return new Generic\BasicBatchAction('MockBasicEntity', __FUNCTION__, 'id', [MockEntityDataStorage::CLASS, 'delete']);
-  }
-
-  /**
-   * @return Generic\BasicBatchAction
-   */
-  public static function batchFrobnicate() {
-    return new Generic\BasicBatchAction('MockBasicEntity', __FUNCTION__, ['id', 'number'], function ($item) {
+  public static function batchFrobnicate($checkPermissions = TRUE) {
+    return (new Generic\BasicBatchAction(__CLASS__, __FUNCTION__, ['id', 'number'], function($item) {
       return [
         'id' => $item['id'],
         'frobnication' => $item['number'] * $item['number'],
       ];
-    });
-  }
-
-  /**
-   * @return Generic\BasicReplaceAction
-   */
-  public static function replace() {
-    return new Generic\BasicReplaceAction('MockBasicEntity', __FUNCTION__);
+    }))->setCheckPermissions($checkPermissions);
   }
 
 }
-- 
2.25.1