From 8a663ca8a37df670fddead732f0fa365beedd771 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 --- .github/workflows/tests.yml | 17 +- composer.json | 2 +- src/Ting/Repository/Hydrator.php | 19 ++- src/Ting/Repository/HydratorRelational.php | 1 + src/Ting/Repository/HydratorSingleObject.php | 5 + src/Ting/UnitOfWork.php | 164 ++++++++----------- tests/units/Ting/Repository/Hydrator.php | 4 +- tests/units/Ting/UnitOfWork.php | 15 -- 8 files changed, 96 insertions(+), 131 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5576034d..c28b5f04 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,20 +2,27 @@ name: Tests on: push: + branches: + - "master" pull_request: - schedule: - - cron: '0 0 * * MON' jobs: run: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest strategy: matrix: - php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + php: + - 8.0 + - 8.1 + - 8.2 + - 8.3 + - 8.4 + composer: + - v2 name: Run Unit Tests on PHP ${{ matrix.php }} steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Setup PHP uses: shivammathur/setup-php@v2 diff --git a/composer.json b/composer.json index 07aa4142..1c75dcf2 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "type": "library", "license": "Apache-2.0", "require": { - "php": ">=7.1", + "php": ">=8.0", "pimple/pimple": "^3.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..dccd34e1 100644 --- a/src/Ting/Repository/HydratorSingleObject.php +++ b/src/Ting/Repository/HydratorSingleObject.php @@ -34,6 +34,11 @@ class HydratorSingleObject extends Hydrator { + public function __construct() + { + parent::__construct(); + } + /** * @return \Generator */ diff --git a/src/Ting/UnitOfWork.php b/src/Ting/UnitOfWork.php index ca35c21b..176c6d34 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 WeakMap $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 = new WeakMap(); } /** @@ -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,19 @@ 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 + if (isset($this->entitiesShouldBePersisted[$entity]) === true + && $this->entitiesShouldBePersisted[$entity] === self::STATE_NEW ) { return true; } @@ -126,17 +126,16 @@ 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; + $this->entitiesShouldBePersisted[$entity] = $state; + $this->entities[$entity] = $entity; return $this; } @@ -145,17 +144,9 @@ 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) { - return true; - } - - return false; + return isset($this->entitiesShouldBePersisted[$entity]); } /** @@ -164,25 +155,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->tingUUID]) === false) { - $this->entitiesChanged[$entity->tingUUID] = []; + if (isset($this->entitiesChanged[$entity]) === false) { + $this->entitiesChanged[$entity] = []; } - if (isset($this->entitiesChanged[$entity->tingUUID][$propertyName]) === false) { - $this->entitiesChanged[$entity->tingUUID][$propertyName] = [$oldValue, null]; + if (isset($this->entitiesChanged[$entity][$propertyName]) === false) { + $this->entitiesChanged[$entity][$propertyName] = [$oldValue, null]; } - $this->entitiesChanged[$entity->tingUUID][$propertyName][1] = $newValue; + $this->entitiesChanged[$entity][$propertyName][1] = $newValue; } /** @@ -190,13 +177,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 +191,21 @@ 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; - } - - unset($this->entitiesChanged[$entity->tingUUID]); - unset($this->entitiesShouldBePersisted[$entity->tingUUID]); - unset($this->entities[$entity->tingUUID]); + $this->entitiesShouldBePersisted->offsetUnset($entity); + $this->entitiesChanged->offsetUnset($entity); + $this->entities->offsetUnset($entity); } /** * Stop watching changes on all entities */ - public function detachAll() + public function detachAll(): void { - $this->entitiesChanged = []; - $this->entitiesShouldBePersisted = []; - $this->entities = []; + $this->entitiesChanged = new WeakMap(); + $this->entitiesShouldBePersisted = new WeakMap(); + $this->entities = new WeakMap(); } /** @@ -235,14 +214,10 @@ public function detachAll() * @param NotifyPropertyInterface $entity * @return $this */ - public function pushDelete(NotifyPropertyInterface $entity) + public function pushDelete(NotifyPropertyInterface $entity): self { - if (isset($entity->tingUUID) === false) { - $entity->tingUUID = $this->generateUid(); - } - - $this->entitiesShouldBePersisted[$entity->tingUUID] = self::STATE_DELETE; - $this->entities[$entity->tingUUID] = $entity; + $this->entitiesShouldBePersisted[$entity] = self::STATE_DELETE; + $this->entities[$entity] = $entity; return $this; } @@ -253,14 +228,10 @@ 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 + if (isset($this->entitiesShouldBePersisted[$entity]) === true + && $this->entitiesShouldBePersisted[$entity] === self::STATE_DELETE ) { return true; } @@ -277,20 +248,20 @@ public function shouldBeRemoved(NotifyPropertyInterface $entity) * @throws Exception * @throws QueryException */ - public function process() + public function process(): void { - foreach ($this->entitiesShouldBePersisted as $uuid => $state) { + foreach ($this->entitiesShouldBePersisted as $entity => $state) { switch ($state) { case self::STATE_MANAGED: - $this->processManaged($uuid); + $this->processManaged($entity); break; case self::STATE_NEW: - $this->processNew($uuid); + $this->processNew($entity); break; case self::STATE_DELETE: - $this->processDelete($uuid); + $this->processDelete($entity); break; } } @@ -305,19 +276,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 +299,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 +311,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); + $this->entitiesShouldBePersisted->offsetUnset($entity); }, function () use ($entity) { throw new QueryException('Could not find repository matching entity "' . \get_class($entity) . '"'); @@ -353,17 +323,16 @@ function () use ($entity) { /** * Insert all applicable entities in database * - * @param $uuid + * @param NotifyPropertyInterface $entity * @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 +344,8 @@ function (Metadata $metadata) use ($entity, $uuid) { $metadata->setEntityPropertyForAutoIncrement($entity, $connection->master()); - unset($this->entitiesChanged[$uuid]); - unset($this->entitiesShouldBePersisted[$uuid]); + $this->entitiesChanged->offsetUnset($entity); + $this->entitiesShouldBePersisted->offsetUnset($entity); $this->manage($entity); }, @@ -389,16 +358,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 +393,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..a3cce00a 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();