From 47f72a8264bfeefdcceaffff7c9505a16b016ac5 Mon Sep 17 00:00:00 2001 From: Victor GREBOT Date: Tue, 17 Dec 2024 16:46:53 +0100 Subject: [PATCH] refacto: use Weakmap for UnitOfWork internal storage instead of uid indexed arrays --- CHANGELOG | 2 + composer.json | 2 +- src/Ting/Repository/Hydrator.php | 19 ++- src/Ting/Repository/HydratorRelational.php | 1 + src/Ting/Repository/HydratorSingleObject.php | 1 - src/Ting/UnitOfWork.php | 166 ++++++++----------- tests/units/Ting/Repository/Hydrator.php | 4 +- tests/units/Ting/UnitOfWork.php | 17 -- 8 files changed, 87 insertions(+), 125 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e6579131..c6e51770 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,9 @@ 3.10.0 (unreleased): + * Drop PHP 7 * Remove hard dependency to pimple, add it in suggest * Collection is now JsonSerializable * Add DateTimeImmutable serialization + * Refacto: use, when possible, Weakmap for UnitOfWork internal storage instead of uuid indexed array * Fix: Query Generator now handle null values 3.9.1 (2024-07-19): diff --git a/composer.json b/composer.json index 96ab5046..981eb330 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "type": "library", "license": "Apache-2.0", "require": { - "php": ">=7.1", + "php": ">=8.0", "aura/sqlquery": "^2.6", "doctrine/cache": "^1.6" }, diff --git a/src/Ting/Repository/Hydrator.php b/src/Ting/Repository/Hydrator.php index f2ef4153..9f2c3db7 100644 --- a/src/Ting/Repository/Hydrator.php +++ b/src/Ting/Repository/Hydrator.php @@ -31,6 +31,7 @@ use CCMBenchmark\Ting\Serializer\UnserializeInterface; use CCMBenchmark\Ting\UnitOfWork; use Generator; +use WeakMap; /** * @template T @@ -45,7 +46,7 @@ class Hydrator implements HydratorInterface protected $objectDatabase = []; protected $objectSchema = []; protected $unserializeAliases = []; - protected $alreadyManaged = []; + protected WeakMap $alreadyManaged; protected $references = []; protected $metadataList = []; @@ -69,6 +70,11 @@ class Hydrator implements HydratorInterface */ protected $identityMap = false; + public function __construct() + { + $this->alreadyManaged = new WeakMap(); + } + /** * @param bool $enable * @throws Exception @@ -243,7 +249,7 @@ private function hasVirtualObject(array $result) */ private function unserializeVirtualObjectProperty(\stdClass $virtualObject) { - foreach ($this->unserializeAliases as $aliasName => list($unserialize, $options)) { + foreach ($this->unserializeAliases as $aliasName => [$unserialize, $options]) { if (isset($virtualObject->$aliasName) === true) { $virtualObject->$aliasName = $unserialize->unserialize($virtualObject->$aliasName, $options); } @@ -326,7 +332,6 @@ function (Metadata $metadata) use ($column, &$result) { if ($this->identityMap === false) { // If identityMap is disabled, it clones the object and reset UUID $result[$column['table']] = clone $this->references[$ref]; - unset($result[$column['table']]->tingUUID); } else { // If identityMap is enabled, it uses the same object $result[$column['table']] = $this->references[$ref]; @@ -440,13 +445,9 @@ function (Metadata $metadata) use ($column, &$result) { */ private function manageIfYouCan($entity) { - if (isset($entity->tingUUID) === true && isset($this->alreadyManaged[$entity->tingUUID]) === true) { - return; - } - - if (\is_object($entity) === true && ($entity instanceof NotifyPropertyInterface) === true) { + if (\is_object($entity) === true && ($entity instanceof NotifyPropertyInterface) === true && $this->alreadyManaged->offsetExists($entity) === false) { $this->unitOfWork->manage($entity); - $this->alreadyManaged[$entity->tingUUID] = true; + $this->alreadyManaged[$entity] = true; } } } diff --git a/src/Ting/Repository/HydratorRelational.php b/src/Ting/Repository/HydratorRelational.php index a7b629fc..8d3ec9d4 100644 --- a/src/Ting/Repository/HydratorRelational.php +++ b/src/Ting/Repository/HydratorRelational.php @@ -72,6 +72,7 @@ final class HydratorRelational extends Hydrator public function __construct() { + parent::__construct(); $this->config = new \SplDoublyLinkedList(); } diff --git a/src/Ting/Repository/HydratorSingleObject.php b/src/Ting/Repository/HydratorSingleObject.php index 0af5141e..4ac446a5 100644 --- a/src/Ting/Repository/HydratorSingleObject.php +++ b/src/Ting/Repository/HydratorSingleObject.php @@ -33,7 +33,6 @@ */ class HydratorSingleObject extends Hydrator { - /** * @return \Generator */ diff --git a/src/Ting/UnitOfWork.php b/src/Ting/UnitOfWork.php index ca35c21b..b4f7bf41 100644 --- a/src/Ting/UnitOfWork.php +++ b/src/Ting/UnitOfWork.php @@ -30,6 +30,7 @@ use CCMBenchmark\Ting\Entity\PropertyListenerInterface; use CCMBenchmark\Ting\Query\QueryFactoryInterface; use CCMBenchmark\Ting\Repository\Metadata; +use WeakMap; class UnitOfWork implements PropertyListenerInterface { @@ -40,9 +41,9 @@ class UnitOfWork implements PropertyListenerInterface protected $connectionPool = null; protected $metadataRepository = null; protected $queryFactory = null; - protected $entities = []; - protected $entitiesChanged = []; - protected $entitiesShouldBePersisted = []; + protected WeakMap $entities; + protected WeakMap $entitiesChanged; + protected array $entitiesShouldBePersisted; protected $statements = []; /** @@ -58,6 +59,9 @@ public function __construct( $this->connectionPool = $connectionPool; $this->metadataRepository = $metadataRepository; $this->queryFactory = $queryFactory; + $this->entities = new WeakMap(); + $this->entitiesChanged = new WeakMap(); + $this->entitiesShouldBePersisted = []; } /** @@ -74,7 +78,7 @@ protected function generateUid() */ protected function generateUUID() { - error_log(sprintf('%s::generateUUID() method is deprecated as of version 3.6 of Ting and will be removed in 4.0. Use %s::generateUid() instead.', self::class, self::class), E_USER_DEPRECATED); + error_log(sprintf('%s::generateUUID() method is deprecated as of version 3.6 of Ting and will be removed in 4.0. Use %s::generateUid() instead.',self::class, self::class),E_USER_DEPRECATED); return $this->generateUid(); } @@ -84,10 +88,10 @@ protected function generateUUID() * * @param NotifyPropertyInterface $entity */ - public function manage(NotifyPropertyInterface $entity) + public function manage(NotifyPropertyInterface $entity): void { - if (isset($entity->tingUUID) === false) { - $entity->tingUUID = $this->generateUid(); + if (isset($this->entities[$entity]) === false) { + $this->entities[$entity] = true; } $entity->addPropertyListener($this); @@ -97,23 +101,20 @@ public function manage(NotifyPropertyInterface $entity) * @param NotifyPropertyInterface $entity * @return bool - true if the entity is managed */ - public function isManaged(NotifyPropertyInterface $entity) + public function isManaged(NotifyPropertyInterface $entity): bool { - return isset($entity->tingUUID); + return isset($this->entities[$entity]); } /** * @param NotifyPropertyInterface $entity * @return bool - true if the entity has not been persisted yet */ - public function isNew(NotifyPropertyInterface $entity) + public function isNew(NotifyPropertyInterface $entity): bool { - if (isset($entity->tingUUID) === false) { - return false; - } - - if (isset($this->entitiesShouldBePersisted[$entity->tingUUID]) === true - && $this->entitiesShouldBePersisted[$entity->tingUUID] === self::STATE_NEW + $hash = spl_object_hash($entity); + if (isset($this->entitiesShouldBePersisted[$hash]) === true + && $this->entitiesShouldBePersisted[$hash]['state'] === self::STATE_NEW ) { return true; } @@ -126,17 +127,17 @@ public function isNew(NotifyPropertyInterface $entity) * @param NotifyPropertyInterface $entity * @return $this */ - public function pushSave(NotifyPropertyInterface $entity) + public function pushSave(NotifyPropertyInterface $entity): self { $state = self::STATE_MANAGED; - if (isset($entity->tingUUID) === false) { - $entity->tingUUID = $this->generateUid(); + if (isset($this->entities[$entity]) === false) { $state = self::STATE_NEW; } - $this->entitiesShouldBePersisted[$entity->tingUUID] = $state; - $this->entities[$entity->tingUUID] = $entity; + $hash = spl_object_hash($entity); + $this->entitiesShouldBePersisted[$hash] = ['state' => $state, 'entity' => $entity]; + $this->entities[$entity] = $entity; return $this; } @@ -145,13 +146,10 @@ public function pushSave(NotifyPropertyInterface $entity) * @param NotifyPropertyInterface $entity * @return bool */ - public function shouldBePersisted(NotifyPropertyInterface $entity) + public function shouldBePersisted(NotifyPropertyInterface $entity): bool { - if (isset($entity->tingUUID) === false) { - return false; - } - - if (isset($this->entitiesShouldBePersisted[$entity->tingUUID]) === true) { + $hash = spl_object_hash($entity); + if (isset($this->entitiesShouldBePersisted[$hash]) === true) { return true; } @@ -164,25 +162,21 @@ public function shouldBePersisted(NotifyPropertyInterface $entity) * @param mixed $oldValue * @param mixed $newValue */ - public function propertyChanged(NotifyPropertyInterface $entity, $propertyName, $oldValue, $newValue) + public function propertyChanged(NotifyPropertyInterface $entity, $propertyName, $oldValue, $newValue): void { if ($oldValue === $newValue) { return; } - if (isset($entity->tingUUID) === false) { - $entity->tingUUID = $this->generateUid(); + if (isset($this->entitiesChanged[$entity]) === false) { + $this->entitiesChanged[$entity] = []; } - if (isset($this->entitiesChanged[$entity->tingUUID]) === false) { - $this->entitiesChanged[$entity->tingUUID] = []; + if (isset($this->entitiesChanged[$entity][$propertyName]) === false) { + $this->entitiesChanged[$entity][$propertyName] = [$oldValue, null]; } - if (isset($this->entitiesChanged[$entity->tingUUID][$propertyName]) === false) { - $this->entitiesChanged[$entity->tingUUID][$propertyName] = [$oldValue, null]; - } - - $this->entitiesChanged[$entity->tingUUID][$propertyName][1] = $newValue; + $this->entitiesChanged[$entity][$propertyName][1] = $newValue; } /** @@ -190,13 +184,9 @@ public function propertyChanged(NotifyPropertyInterface $entity, $propertyName, * @param string $propertyName * @return bool */ - public function isPropertyChanged(NotifyPropertyInterface $entity, $propertyName) + public function isPropertyChanged(NotifyPropertyInterface $entity, string $propertyName): bool { - if (isset($entity->tingUUID) === false) { - return false; - } - - if (isset($this->entitiesChanged[$entity->tingUUID][$propertyName]) === true) { + if (isset($this->entitiesChanged[$entity][$propertyName]) === true) { return true; } @@ -208,25 +198,24 @@ public function isPropertyChanged(NotifyPropertyInterface $entity, $propertyName * * @param NotifyPropertyInterface $entity */ - public function detach(NotifyPropertyInterface $entity) + public function detach(NotifyPropertyInterface $entity): void { - if (isset($entity->tingUUID) === false) { - return; + $hash = spl_object_hash($entity); + if ($this->entitiesShouldBePersisted[$hash]) { + unset($this->entitiesShouldBePersisted[$hash]); } - - unset($this->entitiesChanged[$entity->tingUUID]); - unset($this->entitiesShouldBePersisted[$entity->tingUUID]); - unset($this->entities[$entity->tingUUID]); + $this->entitiesChanged->offsetUnset($entity); + $this->entities->offsetUnset($entity); } /** * Stop watching changes on all entities */ - public function detachAll() + public function detachAll(): void { - $this->entitiesChanged = []; + $this->entitiesChanged = new WeakMap(); $this->entitiesShouldBePersisted = []; - $this->entities = []; + $this->entities = new WeakMap(); } /** @@ -237,12 +226,9 @@ public function detachAll() */ public function pushDelete(NotifyPropertyInterface $entity) { - if (isset($entity->tingUUID) === false) { - $entity->tingUUID = $this->generateUid(); - } - - $this->entitiesShouldBePersisted[$entity->tingUUID] = self::STATE_DELETE; - $this->entities[$entity->tingUUID] = $entity; + $hash = spl_object_hash($entity); + $this->entitiesShouldBePersisted[$hash] = ['state' => self::STATE_DELETE, 'entity' => $entity]; + $this->entities[$entity] = $entity; return $this; } @@ -253,14 +239,11 @@ public function pushDelete(NotifyPropertyInterface $entity) * @param NotifyPropertyInterface $entity * @return bool */ - public function shouldBeRemoved(NotifyPropertyInterface $entity) + public function shouldBeRemoved(NotifyPropertyInterface $entity): bool { - if (isset($entity->tingUUID) === false) { - return false; - } - - if (isset($this->entitiesShouldBePersisted[$entity->tingUUID]) === true - && $this->entitiesShouldBePersisted[$entity->tingUUID] === self::STATE_DELETE + $hash = spl_object_hash($entity); + if (isset($this->entitiesShouldBePersisted[$hash]) === true + && $this->entitiesShouldBePersisted[$hash]['state'] === self::STATE_DELETE ) { return true; } @@ -277,20 +260,20 @@ public function shouldBeRemoved(NotifyPropertyInterface $entity) * @throws Exception * @throws QueryException */ - public function process() + public function process(): void { - foreach ($this->entitiesShouldBePersisted as $uuid => $state) { - switch ($state) { + foreach ($this->entitiesShouldBePersisted as $details) { + switch ($details['state']) { case self::STATE_MANAGED: - $this->processManaged($uuid); + $this->processManaged($details['entity']); break; case self::STATE_NEW: - $this->processNew($uuid); + $this->processNew($details['entity']); break; case self::STATE_DELETE: - $this->processDelete($uuid); + $this->processDelete($details['entity']); break; } } @@ -305,19 +288,18 @@ public function process() /** * Update all applicable entities in database * - * @param $uuid + * @param NotifyPropertyInterface $entity * @throws Exception * @throws QueryException */ - protected function processManaged($uuid) + protected function processManaged(NotifyPropertyInterface $entity): void { - if (isset($this->entitiesChanged[$uuid]) === false) { + if (isset($this->entitiesChanged[$entity]) === false) { return; } - $entity = $this->entities[$uuid]; $properties = []; - foreach ($this->entitiesChanged[$uuid] as $property => $values) { + foreach ($this->entitiesChanged[$entity] as $property => $values) { if ($values[0] !== $values[1]) { $properties[$property] = $values; } @@ -329,7 +311,7 @@ protected function processManaged($uuid) $this->metadataRepository->findMetadataForEntity( $entity, - function (Metadata $metadata) use ($entity, $properties, $uuid) { + function (Metadata $metadata) use ($entity, $properties) { $connection = $metadata->getConnection($this->connectionPool); $query = $metadata->generateQueryForUpdate( $connection, @@ -341,8 +323,8 @@ function (Metadata $metadata) use ($entity, $properties, $uuid) { $this->addStatementToClose($query->getStatementName(), $connection->master()); $query->prepareExecute()->execute(); - unset($this->entitiesChanged[$uuid]); - unset($this->entitiesShouldBePersisted[$uuid]); + $this->entitiesChanged->offsetUnset($entity); + unset($this->entitiesShouldBePersisted[spl_object_hash($entity)]); }, function () use ($entity) { throw new QueryException('Could not find repository matching entity "' . \get_class($entity) . '"'); @@ -352,18 +334,15 @@ function () use ($entity) { /** * Insert all applicable entities in database - * - * @param $uuid + * @param string $hash * @throws Exception * @throws QueryException */ - protected function processNew($uuid) + protected function processNew(NotifyPropertyInterface $entity): void { - $entity = $this->entities[$uuid]; - $this->metadataRepository->findMetadataForEntity( $entity, - function (Metadata $metadata) use ($entity, $uuid) { + function (Metadata $metadata) use ($entity) { $connection = $metadata->getConnection($this->connectionPool); $query = $metadata->generateQueryForInsert( $connection, @@ -375,8 +354,8 @@ function (Metadata $metadata) use ($entity, $uuid) { $metadata->setEntityPropertyForAutoIncrement($entity, $connection->master()); - unset($this->entitiesChanged[$uuid]); - unset($this->entitiesShouldBePersisted[$uuid]); + $this->entitiesChanged->offsetUnset($entity); + unset($this->entitiesShouldBePersisted[spl_object_hash($entity)]); $this->manage($entity); }, @@ -389,16 +368,15 @@ function () use ($entity) { /** * Delete all flagged entities from database * - * @param $uuid + * @param NotifyPropertyInterface $entity * @throws Exception * @throws QueryException */ - protected function processDelete($uuid) + protected function processDelete(NotifyPropertyInterface $entity): void { - $entity = $this->entities[$uuid]; $properties = []; - if (isset($this->entitiesChanged[$uuid])) { - foreach ($this->entitiesChanged[$uuid] as $property => $values) { + if (isset($this->entitiesChanged[$entity])) { + foreach ($this->entitiesChanged[$entity] as $property => $values) { if ($values[0] !== $values[1]) { $properties[$property] = $values; } @@ -425,7 +403,7 @@ function () use ($entity) { ); } - protected function addStatementToClose($statementName, DriverInterface $connection) + protected function addStatementToClose($statementName, DriverInterface $connection): void { if (isset($this->statements[$statementName]) === false) { $this->statements[$statementName] = []; diff --git a/tests/units/Ting/Repository/Hydrator.php b/tests/units/Ting/Repository/Hydrator.php index f62bef1d..75eb2ecb 100644 --- a/tests/units/Ting/Repository/Hydrator.php +++ b/tests/units/Ting/Repository/Hydrator.php @@ -712,9 +712,7 @@ public function testHydrateWithMapObjectShouldHydrateToMethodOfObjectWithAManage ->and($hydrator->mapObjectTo('cit', 'bouh', 'setCity')) ->then($iterator = $hydrator->setResult($result)->getIterator()) ->then($data = $iterator->current()) - ->object($city = $data['bouh']->getOriginalCity()) - ->string($city->tingUUID) - ->isNotNull(); + ->object($city = $data['bouh']->getOriginalCity()); } public function testHydrateWithUnserializeAlias() diff --git a/tests/units/Ting/UnitOfWork.php b/tests/units/Ting/UnitOfWork.php index a896b22d..0de78cf0 100644 --- a/tests/units/Ting/UnitOfWork.php +++ b/tests/units/Ting/UnitOfWork.php @@ -87,21 +87,6 @@ public function testIsManagedShouldReturnFalse() ->isFalse(); } - public function testIsManagedWithUUIDShouldReturnTrue() - { - $mockEntity = new \mock\tests\fixtures\model\Bouh(); - $mockEntity->tingUUID = uniqid(); - - $this - ->if($unitOfWork = new \CCMBenchmark\Ting\UnitOfWork( - $this->services->get('ConnectionPool'), - $this->services->get('MetadataRepository'), - $this->services->get('QueryFactory') - )) - ->boolean($unitOfWork->isManaged($mockEntity)) - ->isTrue(); - } - public function testSave() { $mockEntity = new \mock\tests\fixtures\model\Bouh(); @@ -154,7 +139,6 @@ public function testPersistManagedEntityShouldNotMarkedNew() public function testIsPropertyChangedWithUUIDShouldReturnFalse() { $mockEntity = new \mock\tests\fixtures\model\Bouh(); - $mockEntity->tingUUID = uniqid(); $this ->if($unitOfWork = new \CCMBenchmark\Ting\UnitOfWork( @@ -238,7 +222,6 @@ public function testDetachAll() public function testShouldBeRemovedWithUUIDShouldReturnFalse() { $mockEntity = new \mock\tests\fixtures\model\Bouh(); - $mockEntity->tingUUID = uniqid(); $this ->if($unitOfWork = new \CCMBenchmark\Ting\UnitOfWork(