From 482a26e29686f1c889609061d94d83fef736aeec Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 31 May 2021 22:35:32 -0400 Subject: [PATCH] APIv4 - Return id_field as part of Entity.get All entities have a unique identifier field, usually named 'id' but some entities the field is named something else. e.g. Afform uses 'name' as the identifier. This returns the name of the field as part of Entity.get, and it's also available directly from each API class e.g. Contact::getInfo(). --- Civi/Api4/Entity.php | 4 + Civi/Api4/Generic/AbstractEntity.php | 1 + Civi/Api4/Generic/BasicEntity.php | 11 ++- ext/afform/core/Civi/Api4/Afform.php | 9 +++ .../api/v4/Action/BasicActionsTest.php | 78 ++++++++++--------- .../api/v4/Mock/Api4/MockBasicEntity.php | 8 +- .../api/v4/Mock/MockEntityDataStorage.php | 10 +-- 7 files changed, 76 insertions(+), 45 deletions(-) diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php index 6f91044bd0..aa67b99a8a 100644 --- a/Civi/Api4/Entity.php +++ b/Civi/Api4/Entity.php @@ -85,6 +85,10 @@ class Entity extends Generic\AbstractEntity { 'name' => 'dao', 'description' => 'Class name for dao-based entities', ], + [ + 'name' => 'id_field', + 'description' => 'Name of unique identifier field (e.g. "id")', + ], [ 'name' => 'label_field', 'description' => 'Field to show when displaying a record', diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php index 182b51ff76..72ef2bab19 100644 --- a/Civi/Api4/Generic/AbstractEntity.php +++ b/Civi/Api4/Generic/AbstractEntity.php @@ -139,6 +139,7 @@ abstract class AbstractEntity { 'type' => [self::stripNamespace(get_parent_class(static::class))], 'paths' => static::getEntityPaths(), 'class' => static::class, + 'id_field' => 'id', ]; // Add info for entities with a corresponding DAO $dao = \CRM_Core_DAO_AllCoreTables::getFullName($info['name']); diff --git a/Civi/Api4/Generic/BasicEntity.php b/Civi/Api4/Generic/BasicEntity.php index 96122f909a..66ea70c00e 100644 --- a/Civi/Api4/Generic/BasicEntity.php +++ b/Civi/Api4/Generic/BasicEntity.php @@ -132,8 +132,17 @@ abstract class BasicEntity extends AbstractEntity { * @return BasicReplaceAction */ public static function replace($checkPermissions = TRUE) { - return (new BasicReplaceAction(static::getEntityName(), __FUNCTION__)) + return (new BasicReplaceAction(static::getEntityName(), __FUNCTION__, static::$idField)) ->setCheckPermissions($checkPermissions); } + /** + * @inheritDoc + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['id_field'] = static::$idField; + return $info; + } + } diff --git a/ext/afform/core/Civi/Api4/Afform.php b/ext/afform/core/Civi/Api4/Afform.php index fb242744fb..a9b38c5005 100644 --- a/ext/afform/core/Civi/Api4/Afform.php +++ b/ext/afform/core/Civi/Api4/Afform.php @@ -224,4 +224,13 @@ class Afform extends Generic\AbstractEntity { ]; } + /** + * @inheritDoc + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['id_field'] = 'name'; + return $info; + } + } diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php index fa011b06d3..40048d6e4b 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -28,41 +28,47 @@ use Civi\Api4\MockBasicEntity; class BasicActionsTest extends UnitTestCase { private function replaceRecords(&$records) { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); + MockBasicEntity::delete()->addWhere('identifier', '>', 0)->execute(); foreach ($records as &$record) { - $record['id'] = MockBasicEntity::create()->setValues($record)->execute()->first()['id']; + $record['identifier'] = MockBasicEntity::create()->setValues($record)->execute()->first()['identifier']; } } + public function testGetInfo() { + $info = MockBasicEntity::getInfo(); + $this->assertEquals('MockBasicEntity', $info['name']); + $this->assertEquals('identifier', $info['id_field']); + } + public function testCrud() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); + MockBasicEntity::delete()->addWhere('identifier', '>', 0)->execute(); - $id1 = MockBasicEntity::create()->addValue('foo', 'one')->execute()->first()['id']; + $id1 = MockBasicEntity::create()->addValue('foo', 'one')->execute()->first()['identifier']; $result = MockBasicEntity::get()->execute(); $this->assertCount(1, $result); - $id2 = MockBasicEntity::create()->addValue('foo', 'two')->execute()->first()['id']; + $id2 = MockBasicEntity::create()->addValue('foo', 'two')->execute()->first()['identifier']; $result = MockBasicEntity::get()->selectRowCount()->execute(); $this->assertEquals(2, $result->count()); - MockBasicEntity::update()->addWhere('id', '=', $id2)->addValue('foo', 'new')->execute(); + MockBasicEntity::update()->addWhere('identifier', '=', $id2)->addValue('foo', 'new')->execute(); - $result = MockBasicEntity::get()->addOrderBy('id', 'DESC')->setLimit(1)->execute(); + $result = MockBasicEntity::get()->addOrderBy('identifier', 'DESC')->setLimit(1)->execute(); // The object's count() method will account for all results, ignoring limit, while the array results are limited $this->assertCount(2, $result); $this->assertCount(1, (array) $result); $this->assertEquals('new', $result->first()['foo']); $result = MockBasicEntity::save() - ->addRecord(['id' => $id1, 'foo' => 'one updated', 'weight' => '5']) - ->addRecord(['id' => $id2, 'group:label' => 'Second']) + ->addRecord(['identifier' => $id1, 'foo' => 'one updated', 'weight' => '5']) + ->addRecord(['identifier' => $id2, 'group:label' => 'Second']) ->addRecord(['foo' => 'three']) ->addDefault('color', 'pink') ->setReload(TRUE) ->execute() - ->indexBy('id'); + ->indexBy('identifier'); $this->assertTrue(5 === $result[$id1]['weight']); $this->assertEquals('new', $result[$id2]['foo']); @@ -73,7 +79,7 @@ class BasicActionsTest extends UnitTestCase { $this->assertEquals('pink', $item['color']); } - $ent1 = MockBasicEntity::get()->addWhere('id', '=', $id1)->execute()->first(); + $ent1 = MockBasicEntity::get()->addWhere('identifier', '=', $id1)->execute()->first(); $this->assertEquals('one updated', $ent1['foo']); $this->assertFalse(isset($ent1['group:label'])); @@ -90,7 +96,7 @@ class BasicActionsTest extends UnitTestCase { // This one wasn't selected but did get used by the WHERE clause; ensure it isn't returned $this->assertFalse(isset($ent2['group:label'])); - MockBasicEntity::delete()->addWhere('id', '=', $id2); + MockBasicEntity::delete()->addWhere('identifier', '=', $id2); $result = MockBasicEntity::get()->execute(); $this->assertEquals('one updated', $result->first()['foo']); } @@ -107,24 +113,24 @@ class BasicActionsTest extends UnitTestCase { // Keep red, change blue, delete green, and add yellow $replacements = [ - ['color' => 'red', 'id' => $objects[0]['id']], - ['color' => 'not blue', 'id' => $objects[1]['id']], + ['color' => 'red', 'identifier' => $objects[0]['identifier']], + ['color' => 'not blue', 'identifier' => $objects[1]['identifier']], ['color' => 'yellow'], ]; MockBasicEntity::replace()->addWhere('group', '=', 'one')->setRecords($replacements)->execute(); - $newObjects = MockBasicEntity::get()->addOrderBy('id', 'DESC')->execute()->indexBy('id'); + $newObjects = MockBasicEntity::get()->addOrderBy('identifier', 'DESC')->execute()->indexBy('identifier'); $this->assertCount(4, $newObjects); $this->assertEquals('yellow', $newObjects->first()['color']); - $this->assertEquals('not blue', $newObjects[$objects[1]['id']]['color']); + $this->assertEquals('not blue', $newObjects[$objects[1]['identifier']]['color']); // Ensure group two hasn't been altered - $this->assertEquals('orange', $newObjects[$objects[3]['id']]['color']); - $this->assertEquals('two', $newObjects[$objects[3]['id']]['group']); + $this->assertEquals('orange', $newObjects[$objects[3]['identifier']]['color']); + $this->assertEquals('two', $newObjects[$objects[3]['identifier']]['group']); } public function testBatchFrobnicate() { @@ -145,14 +151,14 @@ class BasicActionsTest extends UnitTestCase { $getFields = MockBasicEntity::getFields()->execute()->indexBy('name'); $this->assertCount(7, $getFields); - $this->assertEquals('Id', $getFields['id']['title']); + $this->assertEquals('Identifier', $getFields['identifier']['title']); // Ensure default data type is "String" when not specified $this->assertEquals('String', $getFields['color']['data_type']); // Getfields should default to loadOptions = false and reduce them to bool $this->assertTrue($getFields['group']['options']); $this->assertTrue($getFields['fruit']['options']); - $this->assertFalse($getFields['id']['options']); + $this->assertFalse($getFields['identifier']['options']); // Load simple options $getFields = MockBasicEntity::getFields() @@ -215,15 +221,15 @@ class BasicActionsTest extends UnitTestCase { $this->assertTrue($isFieldSelected->invoke($get, 'size', 'color', 'shape')); // With a non-empty "select" fieldsToSelect() will return fields needed to evaluate each clause. - $get->addSelect('id'); + $get->addSelect('identifier'); $this->assertTrue($isFieldSelected->invoke($get, 'color', 'shape', 'size')); - $this->assertTrue($isFieldSelected->invoke($get, 'id')); + $this->assertTrue($isFieldSelected->invoke($get, 'identifier')); $this->assertFalse($isFieldSelected->invoke($get, 'shape', 'size', 'weight')); $this->assertFalse($isFieldSelected->invoke($get, 'group')); $get->addClause('OR', ['shape', '=', 'round'], ['AND', [['size', '=', 'big'], ['weight', '!=', 'small']]]); $this->assertTrue($isFieldSelected->invoke($get, 'color')); - $this->assertTrue($isFieldSelected->invoke($get, 'id')); + $this->assertTrue($isFieldSelected->invoke($get, 'identifier')); $this->assertTrue($isFieldSelected->invoke($get, 'shape')); $this->assertTrue($isFieldSelected->invoke($get, 'size')); $this->assertTrue($isFieldSelected->invoke($get, 'group', 'weight')); @@ -242,7 +248,7 @@ class BasicActionsTest extends UnitTestCase { foreach (MockBasicEntity::get()->addSelect('*')->execute() as $result) { ksort($result); - $this->assertEquals(['color', 'group', 'id', 'shape', 'size', 'weight'], array_keys($result)); + $this->assertEquals(['color', 'group', 'identifier', 'shape', 'size', 'weight'], array_keys($result)); } $result = MockBasicEntity::get() @@ -262,39 +268,39 @@ class BasicActionsTest extends UnitTestCase { $result = MockBasicEntity::get() ->addWhere('color', 'IS NULL') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(1, $result); - $this->assertArrayHasKey($records[0]['id'], (array) $result); + $this->assertArrayHasKey($records[0]['identifier'], (array) $result); $result = MockBasicEntity::get() ->addWhere('color', 'IS EMPTY') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(2, $result); - $this->assertArrayNotHasKey($records[2]['id'], (array) $result); + $this->assertArrayNotHasKey($records[2]['identifier'], (array) $result); $result = MockBasicEntity::get() ->addWhere('color', 'IS NOT EMPTY') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(1, $result); - $this->assertArrayHasKey($records[2]['id'], (array) $result); + $this->assertArrayHasKey($records[2]['identifier'], (array) $result); $result = MockBasicEntity::get() ->addWhere('weight', 'IS NULL') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(1, $result); - $this->assertArrayHasKey($records[0]['id'], (array) $result); + $this->assertArrayHasKey($records[0]['identifier'], (array) $result); $result = MockBasicEntity::get() ->addWhere('weight', 'IS EMPTY') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(2, $result); - $this->assertArrayNotHasKey($records[2]['id'], (array) $result); + $this->assertArrayNotHasKey($records[2]['identifier'], (array) $result); $result = MockBasicEntity::get() ->addWhere('weight', 'IS NOT EMPTY') - ->execute()->indexBy('id'); + ->execute()->indexBy('identifier'); $this->assertCount(1, $result); - $this->assertArrayHasKey($records[2]['id'], (array) $result); + $this->assertArrayHasKey($records[2]['identifier'], (array) $result); } public function testContainsOperator() { diff --git a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php index 51c59220c3..88adad6f49 100644 --- a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php +++ b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php @@ -28,6 +28,8 @@ use api\v4\Mock\MockEntityDataStorage; */ class MockBasicEntity extends Generic\BasicEntity { + protected static $idField = 'identifier'; + protected static $getter = [MockEntityDataStorage::CLASS, 'get']; protected static $setter = [MockEntityDataStorage::CLASS, 'write']; protected static $deleter = [MockEntityDataStorage::CLASS, 'delete']; @@ -39,7 +41,7 @@ class MockBasicEntity extends Generic\BasicEntity { return (new Generic\BasicGetFieldsAction(__CLASS__, __FUNCTION__, function() { return [ [ - 'name' => 'id', + 'name' => 'identifier', 'data_type' => 'Integer', ], [ @@ -94,9 +96,9 @@ class MockBasicEntity extends Generic\BasicEntity { * @return Generic\BasicBatchAction */ public static function batchFrobnicate($checkPermissions = TRUE) { - return (new Generic\BasicBatchAction(__CLASS__, __FUNCTION__, ['id', 'number'], function($item) { + return (new Generic\BasicBatchAction(__CLASS__, __FUNCTION__, ['identifier', 'number'], function($item) { return [ - 'id' => $item['id'], + 'identifier' => $item['identifier'], 'frobnication' => $item['number'] * $item['number'], ]; }))->setCheckPermissions($checkPermissions); diff --git a/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php b/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php index 1f9d33d188..8c7c1b0271 100644 --- a/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php +++ b/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php @@ -33,18 +33,18 @@ class MockEntityDataStorage { } public static function write($record) { - if (empty($record['id'])) { - $record['id'] = self::$nextId++; - self::$data[$record['id']] = $record; + if (empty($record['identifier'])) { + $record['identifier'] = self::$nextId++; + self::$data[$record['identifier']] = $record; } else { - self::$data[$record['id']] = $record + self::$data[$record['id']]; + self::$data[$record['identifier']] = $record + self::$data[$record['identifier']]; } return $record; } public static function delete($record) { - unset(self::$data[$record['id']]); + unset(self::$data[$record['identifier']]); return $record; } -- 2.25.1