From f23d54c9bc3eac32b266eac2ff24cb8ceab73331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jer=C3=B4me=20Bakker?= Date: Thu, 1 Aug 2024 10:27:17 +0200 Subject: [PATCH] chore(entity): move metadata related function into trait --- .../classes/Elgg/Traits/Entity/Metadata.php | 208 ++++++++++++++++++ engine/classes/ElggEntity.php | 200 +---------------- .../Entity/MetadataIntegrationTestCase.php} | 52 ++++- .../Elgg/Integration/ElggCoreEntityTest.php | 26 --- .../ElggGroupMetadataIntegrationTest.php | 16 ++ .../ElggObjectMetadataIntegrationTest.php | 16 ++ .../ElggSiteMetadataIntegrationTest.php | 41 ++++ .../ElggUserMetadataIntegrationTest.php | 16 ++ 8 files changed, 341 insertions(+), 234 deletions(-) create mode 100644 engine/classes/Elgg/Traits/Entity/Metadata.php rename engine/tests/{phpunit/integration/Elgg/Integration/ElggEntityMetadataTest.php => classes/Elgg/Traits/Entity/MetadataIntegrationTestCase.php} (87%) create mode 100644 engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggGroupMetadataIntegrationTest.php create mode 100644 engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggObjectMetadataIntegrationTest.php create mode 100644 engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggSiteMetadataIntegrationTest.php create mode 100644 engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggUserMetadataIntegrationTest.php diff --git a/engine/classes/Elgg/Traits/Entity/Metadata.php b/engine/classes/Elgg/Traits/Entity/Metadata.php new file mode 100644 index 00000000000..82ed7b03e33 --- /dev/null +++ b/engine/classes/Elgg/Traits/Entity/Metadata.php @@ -0,0 +1,208 @@ +getAllMetadata(); + return elgg_extract($name, $metadata); + } + + /** + * Get all entity metadata + * + * @return array + */ + public function getAllMetadata(): array { + if (!$this->guid) { + return array_map(function($values) { + return count($values) > 1 ? $values : $values[0]; + }, $this->temp_metadata); + } + + $this->_cached_metadata = _elgg_services()->metadataCache->getAll($this->guid); + + return $this->_cached_metadata; + } + + /** + * Set metadata on this entity. + * + * Plugin developers usually want to use the magic set method ($entity->name = 'value'). + * Use this method if you want to explicitly set the owner or access of the metadata. + * You cannot set the owner/access before the entity has been saved. + * + * @param string $name Name of the metadata + * @param mixed $value Value of the metadata (doesn't support assoc arrays) + * @param string $value_type 'text', 'integer', or '' for automatic detection + * @param bool $multiple Allow multiple values for a single name. + * Does not support associative arrays. + * + * @return bool + */ + public function setMetadata(string $name, mixed $value, string $value_type = '', bool $multiple = false): bool { + if ($value === null || $value === '') { + return $this->deleteMetadata($name); + } + + // normalize value to an array that we will loop over + // remove indexes if value already an array. + if (is_array($value)) { + $value = array_values($value); + } else { + $value = [$value]; + } + + // strip null values from array + $value = array_filter($value, function($var) { + return !is_null($var); + }); + + if (empty($this->guid)) { + // unsaved entity. store in temp array + return $this->setTempMetadata($name, $value, $multiple); + } + + // saved entity. persist md to db. + if (!$multiple) { + $current_metadata = $this->getMetadata($name); + + if ((is_array($current_metadata) || count($value) > 1 || $value === []) && isset($current_metadata)) { + // remove current metadata if needed + // need to remove access restrictions right now to delete + // because this is the expected behavior + $delete_result = elgg_call(ELGG_IGNORE_ACCESS, function() use ($name) { + return elgg_delete_metadata([ + 'guid' => $this->guid, + 'metadata_name' => $name, + 'limit' => false, + ]); + }); + + if ($delete_result === false) { + return false; + } + } + + if (count($value) > 1) { + // new value is a multiple valued metadata + $multiple = true; + } + } + + // create new metadata + foreach ($value as $value_tmp) { + $metadata = new \ElggMetadata(); + $metadata->entity_guid = $this->guid; + $metadata->name = $name; + $metadata->value = $value_tmp; + + if (!empty($value_type)) { + $metadata->value_type = $value_type; + } + + $md_id = _elgg_services()->metadataTable->create($metadata, $multiple); + if ($md_id === false) { + return false; + } + } + + return true; + } + + /** + * Set temp metadata on this entity. + * + * @param string $name Name of the metadata + * @param mixed $value Value of the metadata (doesn't support assoc arrays) + * @param bool $multiple Allow multiple values for a single name. + * Does not support associative arrays. + * + * @return bool + */ + protected function setTempMetadata(string $name, mixed $value, bool $multiple = false): bool { + // if overwrite, delete first + if (!$multiple) { + unset($this->temp_metadata[$name]); + if (count($value)) { + // only save if value array contains data + $this->temp_metadata[$name] = $value; + } + + return true; + } + + if (!isset($this->temp_metadata[$name])) { + $this->temp_metadata[$name] = []; + } + + $this->temp_metadata[$name] = array_merge($this->temp_metadata[$name], $value); + + return true; + } + + /** + * Deletes all metadata on this object (metadata.entity_guid = $this->guid). + * If you pass a name, only metadata matching that name will be deleted. + * + * @warning Calling this with no $name will clear all metadata on the entity. + * + * @param null|string $name The name of the metadata to remove. + * + * @return bool + * @since 1.8 + */ + public function deleteMetadata(string $name = null): bool { + if (!$this->guid) { + // remove from temp_metadata + if (isset($name)) { + if (isset($this->temp_metadata[$name])) { + unset($this->temp_metadata[$name]); + } + } else { + $this->temp_metadata = []; + } + + return true; + } + + return elgg_delete_metadata([ + 'guid' => $this->guid, + 'limit' => false, + 'metadata_name' => $name, + ]); + } +} diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php index 1be33b6adc0..b6a445962fc 100644 --- a/engine/classes/ElggEntity.php +++ b/engine/classes/ElggEntity.php @@ -6,6 +6,7 @@ use Elgg\Exceptions\DomainException as ElggDomainException; use Elgg\Exceptions\InvalidArgumentException as ElggInvalidArgumentException; use Elgg\Traits\Entity\Icons; +use Elgg\Traits\Entity\Metadata; use Elgg\Traits\Entity\Subscriptions; /** @@ -48,6 +49,7 @@ abstract class ElggEntity extends \ElggData { use Icons; + use Metadata; use Subscriptions; public const PRIMARY_ATTR_NAMES = [ @@ -79,13 +81,6 @@ abstract class ElggEntity extends \ElggData { 'time_deleted', ]; - /** - * Holds metadata until entity is saved. Once the entity is saved, - * metadata are written immediately to the database. - * @var array - */ - protected $temp_metadata = []; - /** * Holds annotations until entity is saved. Once the entity is saved, * annotations are written immediately to the database. @@ -111,18 +106,6 @@ abstract class ElggEntity extends \ElggData { */ protected $_is_cacheable = true; - /** - * Holds metadata key/value pairs acquired from the metadata cache - * Entity metadata may have mutated since last call to __get, - * do not rely on this value for any business logic - * This storage is intended to help with debugging objects during dump, - * because otherwise it's hard to tell what the object is from it's attributes - * - * @var array - * @internal - */ - protected $_cached_metadata; - /** * Create a new entity. * @@ -336,185 +319,6 @@ public function setDisplayName(string $display_name): void { $this->name = $display_name; } - /** - * Return the value of a piece of metadata. - * - * @param string $name Name - * - * @return mixed The value, or null if not found. - */ - public function getMetadata(string $name) { - $metadata = $this->getAllMetadata(); - return elgg_extract($name, $metadata); - } - - /** - * Get all entity metadata - * - * @return array - */ - public function getAllMetadata(): array { - if (!$this->guid) { - return array_map(function($values) { - return count($values) > 1 ? $values : $values[0]; - }, $this->temp_metadata); - } - - $this->_cached_metadata = _elgg_services()->metadataCache->getAll($this->guid); - - return $this->_cached_metadata; - } - - /** - * Set metadata on this entity. - * - * Plugin developers usually want to use the magic set method ($entity->name = 'value'). - * Use this method if you want to explicitly set the owner or access of the metadata. - * You cannot set the owner/access before the entity has been saved. - * - * @param string $name Name of the metadata - * @param mixed $value Value of the metadata (doesn't support assoc arrays) - * @param string $value_type 'text', 'integer', or '' for automatic detection - * @param bool $multiple Allow multiple values for a single name. - * Does not support associative arrays. - * - * @return bool - */ - public function setMetadata(string $name, $value, string $value_type = '', bool $multiple = false): bool { - - if ($value === null || $value === '') { - return $this->deleteMetadata($name); - } - - // normalize value to an array that we will loop over - // remove indexes if value already an array. - if (is_array($value)) { - $value = array_values($value); - } else { - $value = [$value]; - } - - // strip null values from array - $value = array_filter($value, function($var) { - return !is_null($var); - }); - - if (empty($this->guid)) { - // unsaved entity. store in temp array - return $this->setTempMetadata($name, $value, $multiple); - } - - // saved entity. persist md to db. - if (!$multiple) { - $current_metadata = $this->getMetadata($name); - - if ((is_array($current_metadata) || count($value) > 1 || $value === []) && isset($current_metadata)) { - // remove current metadata if needed - // need to remove access restrictions right now to delete - // because this is the expected behavior - $delete_result = elgg_call(ELGG_IGNORE_ACCESS, function() use ($name) { - return elgg_delete_metadata([ - 'guid' => $this->guid, - 'metadata_name' => $name, - 'limit' => false, - ]); - }); - - if ($delete_result === false) { - return false; - } - } - - if (count($value) > 1) { - // new value is a multiple valued metadata - $multiple = true; - } - } - - // create new metadata - foreach ($value as $value_tmp) { - $metadata = new ElggMetadata(); - $metadata->entity_guid = $this->guid; - $metadata->name = $name; - $metadata->value = $value_tmp; - - if (!empty($value_type)) { - $metadata->value_type = $value_type; - } - - $md_id = _elgg_services()->metadataTable->create($metadata, $multiple); - if ($md_id === false) { - return false; - } - } - - return true; - } - - /** - * Set temp metadata on this entity. - * - * @param string $name Name of the metadata - * @param mixed $value Value of the metadata (doesn't support assoc arrays) - * @param bool $multiple Allow multiple values for a single name. - * Does not support associative arrays. - * - * @return bool - */ - protected function setTempMetadata(string $name, $value, bool $multiple = false): bool { - // if overwrite, delete first - if (!$multiple) { - unset($this->temp_metadata[$name]); - if (count($value)) { - // only save if value array contains data - $this->temp_metadata[$name] = $value; - } - - return true; - } - - if (!isset($this->temp_metadata[$name])) { - $this->temp_metadata[$name] = []; - } - - $this->temp_metadata[$name] = array_merge($this->temp_metadata[$name], $value); - - return true; - } - - /** - * Deletes all metadata on this object (metadata.entity_guid = $this->guid). - * If you pass a name, only metadata matching that name will be deleted. - * - * @warning Calling this with no $name will clear all metadata on the entity. - * - * @param null|string $name The name of the metadata to remove. - * - * @return bool - * @since 1.8 - */ - public function deleteMetadata(string $name = null): bool { - - if (!$this->guid) { - // remove from temp_metadata - if (isset($name)) { - if (isset($this->temp_metadata[$name])) { - unset($this->temp_metadata[$name]); - } - } else { - $this->temp_metadata = []; - } - - return true; - } - - return elgg_delete_metadata([ - 'guid' => $this->guid, - 'limit' => false, - 'metadata_name' => $name, - ]); - } - /** * Get a piece of volatile (non-persisted) data on this entity. * diff --git a/engine/tests/phpunit/integration/Elgg/Integration/ElggEntityMetadataTest.php b/engine/tests/classes/Elgg/Traits/Entity/MetadataIntegrationTestCase.php similarity index 87% rename from engine/tests/phpunit/integration/Elgg/Integration/ElggEntityMetadataTest.php rename to engine/tests/classes/Elgg/Traits/Entity/MetadataIntegrationTestCase.php index c4d06e77929..2a202686b4b 100644 --- a/engine/tests/phpunit/integration/Elgg/Integration/ElggEntityMetadataTest.php +++ b/engine/tests/classes/Elgg/Traits/Entity/MetadataIntegrationTestCase.php @@ -1,28 +1,32 @@ entity = $this->createObject(); + parent::up(); + + $this->entity = $this->getEntity(); - $this->unsaved_entity = new \ElggObject(); + $this->unsaved_entity = $this->getUnsavedEntity(); $this->entities = [ $this->entity, @@ -35,6 +39,10 @@ public function down() { unset($this->entities); } + abstract protected function getEntity(): \ElggEntity; + + abstract protected function getUnsavedEntity(): \ElggEntity; + public function testEntitySetSingleValue() { foreach ($this->entities as $entity) { $entity->foo = 'bar'; @@ -340,6 +348,30 @@ public function testCanGetAllEntityMetadata() { $this->assertEquals($metadata['foo3'], $entity->foo3); } } - - + + /** + * @dataProvider emptyValues + */ + public function testSetMetadataEmpty($empty_value) { + foreach ($this->entities as $entity) { + $entity->setMetadata('foo', 'bar'); + $this->assertEquals('bar', $entity->getMetadata('foo')); + $this->assertEquals('bar', $entity->foo); + + // remove metadata by setting to empty value + $this->assertTrue($entity->setMetadata('foo', $empty_value)); + $this->assertNull($entity->foo); + $this->assertNull($entity->getMetadata('foo')); + + // removing non-existing data should also return true + $this->assertTrue($entity->setMetadata('foo', $empty_value)); + } + } + + public static function emptyValues() { + return [ + [''], + [null], + ]; + } } diff --git a/engine/tests/phpunit/integration/Elgg/Integration/ElggCoreEntityTest.php b/engine/tests/phpunit/integration/Elgg/Integration/ElggCoreEntityTest.php index bfd10e31e71..4726472c12c 100644 --- a/engine/tests/phpunit/integration/Elgg/Integration/ElggCoreEntityTest.php +++ b/engine/tests/phpunit/integration/Elgg/Integration/ElggCoreEntityTest.php @@ -649,32 +649,6 @@ public static function entitiesNotTypesMatch() { ]; } - /** - * @dataProvider emptyValues - */ - public function testSetMetadataEmpty($empty_value) { - $object = $this->createObject(); - - $object->setMetadata('foo', 'bar'); - $this->assertEquals('bar', $object->getMetadata('foo')); - $this->assertEquals('bar', $object->foo); - - // remove metadata by setting to empty value - $this->assertTrue($object->setMetadata('foo', $empty_value)); - $this->assertNull($object->foo); - $this->assertNull($object->getMetadata('foo')); - - // removing unexisting data should also return true - $this->assertTrue($object->setMetadata('foo', $empty_value)); - } - - public static function emptyValues() { - return [ - [''], - [null], - ]; - } - public function testDeleteDeadloopPrevented() { $user = $this->getAdmin(); diff --git a/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggGroupMetadataIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggGroupMetadataIntegrationTest.php new file mode 100644 index 00000000000..3a0be331e48 --- /dev/null +++ b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggGroupMetadataIntegrationTest.php @@ -0,0 +1,16 @@ +createGroup(); + } + + protected function getUnsavedEntity(): \ElggEntity { + return new \ElggGroup(); + } +} diff --git a/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggObjectMetadataIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggObjectMetadataIntegrationTest.php new file mode 100644 index 00000000000..c166db26734 --- /dev/null +++ b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggObjectMetadataIntegrationTest.php @@ -0,0 +1,16 @@ +createObject(); + } + + protected function getUnsavedEntity(): \ElggEntity { + return new \ElggObject(); + } +} diff --git a/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggSiteMetadataIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggSiteMetadataIntegrationTest.php new file mode 100644 index 00000000000..79046c7a60e --- /dev/null +++ b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggSiteMetadataIntegrationTest.php @@ -0,0 +1,41 @@ + $this->entity->guid, + 'limit' => false, + ]); + $names = []; + foreach ($metadata as $md) { + $names[] = $md->name; + } + $this->initial_metadata_names = array_unique($names); + } + + public function down() { + foreach ($this->initial_metadata_names as $name) { + unset($this->entity->{$name}); + } + + parent::down(); + } + + protected function getEntity(): \ElggEntity { + return $this->createSite(); + } + + protected function getUnsavedEntity(): \ElggEntity { + return new \ElggSite(); + } +} diff --git a/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggUserMetadataIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggUserMetadataIntegrationTest.php new file mode 100644 index 00000000000..f0184e601cc --- /dev/null +++ b/engine/tests/phpunit/integration/Elgg/Traits/Entity/ElggUserMetadataIntegrationTest.php @@ -0,0 +1,16 @@ +createUser(); + } + + protected function getUnsavedEntity(): \ElggEntity { + return new \ElggUser(); + } +}