From b50e216eff5817820aa8bafe60ff88f88923140f Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Fri, 2 Oct 2026 11:15:05 +0200 Subject: [PATCH] fix(task) #32 #33 #34 #35 #36 entity_manager option, readers executed again for each input, table optional with sql, null input errors, entities hydrated while iterating Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 + docs/cookbooks/database_to_csv_export.md | 5 +- docs/index.md | 8 +- docs/reference/tasks/database_reader_task.md | 8 +- .../tasks/doctrine_batchwriter_task.md | 8 +- docs/reference/tasks/doctrine_cleaner_task.md | 8 +- .../reference/tasks/doctrine_detacher_task.md | 6 +- docs/reference/tasks/doctrine_reader_task.md | 13 +- .../tasks/doctrine_refresher_task.md | 6 +- docs/reference/tasks/doctrine_remover_task.md | 11 +- docs/reference/tasks/doctrine_writer_task.md | 6 +- src/Task/Database/DatabaseReaderTask.php | 30 +++- .../EntityManager/AbstractDoctrineTask.php | 20 +++ .../EntityManager/DoctrineBatchWriterTask.php | 5 +- .../EntityManager/DoctrineCleanerTask.php | 6 +- .../EntityManager/DoctrineDetacherTask.php | 8 +- src/Task/EntityManager/DoctrineReaderTask.php | 18 ++- .../EntityManager/DoctrineRefresherTask.php | 6 +- .../EntityManager/DoctrineRemoverTask.php | 9 +- src/Task/EntityManager/DoctrineWriterTask.php | 6 +- .../Database/DatabaseReaderTaskSqliteTest.php | 153 ++++++++++++++++++ .../AbstractDoctrineTaskTest.php | 89 ++++++++++ .../DoctrineBatchWriterTaskTest.php | 8 + .../EntityManager/DoctrineCleanerTaskTest.php | 3 + .../DoctrineDetacherTaskTest.php | 1 + .../EntityManager/DoctrineReaderTaskTest.php | 86 ++++++++++ .../EntityManager/DoctrineRemoverTaskTest.php | 6 +- 27 files changed, 456 insertions(+), 84 deletions(-) create mode 100644 tests/Task/Database/DatabaseReaderTaskSqliteTest.php create mode 100644 tests/Task/EntityManager/AbstractDoctrineTaskTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b096c2..44247c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Latest ------ +### Fixes +* [#32](https://github.com/cleverage/doctrine-process-bundle/issues/32) Fix EntityManager tasks: use the entity manager given by the `entity_manager` option (it was ignored by every task except ClearEntityManagerTask), the one managing the entity class otherwise. Update documentation, add tests. +* [#33](https://github.com/cleverage/doctrine-process-bundle/issues/33) Fix DatabaseReaderTask and DoctrineReaderTask: execute the query again for each input (the input following a complete iteration was skipped). Update documentation, add tests. +* [#34](https://github.com/cleverage/doctrine-process-bundle/issues/34) Fix DatabaseReaderTask: the `table` option is only required when `sql` is not set. Update documentation, add tests. +* [#35](https://github.com/cleverage/doctrine-process-bundle/issues/35) Fix DoctrineDetacherTask error message on a null input (it named DoctrineWriterTask), and throw an explicit `\RuntimeException` on a null input in DoctrineRemoverTask (a `\TypeError` was triggered). Update documentation, add tests. +* [#36](https://github.com/cleverage/doctrine-process-bundle/issues/36) Fix DoctrineReaderTask: hydrate the entities one at a time while iterating (every entity was hydrated before the first output). Update documentation, add tests. + v3.1 ------ diff --git a/docs/cookbooks/database_to_csv_export.md b/docs/cookbooks/database_to_csv_export.md index 7e51f75..a16acbd 100644 --- a/docs/cookbooks/database_to_csv_export.md +++ b/docs/cookbooks/database_to_csv_export.md @@ -14,7 +14,6 @@ clever_age_process: read_books: service: '@CleverAge\DoctrineProcessBundle\Task\Database\DatabaseReaderTask' options: - table: 'book' # Required, even if a custom sql query is used sql: > SELECT b.id, b.title, a.firstname, a.lastname FROM book b @@ -78,5 +77,5 @@ How it works: To export entities instead of raw rows, replace the first task by a [DoctrineReaderTask](../reference/tasks/doctrine_reader_task.md) and read the values with property paths -(e.g. `code: 'author.lastname'`). Note that the DoctrineReaderTask loads all the matching entities in memory: for -big volumes, prefer the DatabaseReaderTask. +(e.g. `code: 'author.lastname'`). Note that the hydrated entities stay managed by the entity manager: for big +volumes, clear it regularly or prefer the DatabaseReaderTask. diff --git a/docs/index.md b/docs/index.md index f91b391..20c097d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -45,10 +45,10 @@ doctrine: [DatabaseUpdaterTask](reference/tasks/database_updater_task.md)) run raw SQL queries through Doctrine DBAL. Their `connection` option takes the name of a connection (a key under `doctrine.dbal.connections`); the default connection is used if it is not set. -* **EntityManager** tasks work with Doctrine ORM entities. Except for the - [ClearEntityManagerTask](reference/tasks/doctrine_clear_task.md), which takes the name of an entity manager (a key - under `doctrine.orm.entity_managers`) in its `entity_manager` option, they use the entity manager that manages the - class of the handled entity. +* **EntityManager** tasks work with Doctrine ORM entities. Their `entity_manager` option takes the name of an entity + manager (a key under `doctrine.orm.entity_managers`); if it is not set, they use the entity manager that manages the + class of the handled entity (the default entity manager for the + [ClearEntityManagerTask](reference/tasks/doctrine_clear_task.md)). See the [DoctrineBundle documentation](https://symfony.com/bundles/DoctrineBundle/current/configuration.html) for the configuration of multiple connections and entity managers. diff --git a/docs/reference/tasks/database_reader_task.md b/docs/reference/tasks/database_reader_task.md index 9a7c770..8e810f8 100644 --- a/docs/reference/tasks/database_reader_task.md +++ b/docs/reference/tasks/database_reader_task.md @@ -30,7 +30,7 @@ Options | Code | Type | Required | Default | Description | |-------------------|---------------|:--------:|-----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `table` | `string` | **X** | | Table to read from when `sql` is not set: the query is `SELECT tbl.* FROM tbl`.
Required even when `sql` is set (its value is then ignored) | +| `table` | `string` | | | Table to read from: the query is `SELECT tbl.* FROM
tbl`.
Required when `sql` is not set (ignored otherwise) | | `connection` | `string\|null` | | `null` | Name of the Doctrine DBAL connection (as defined in `doctrine.dbal.connections`). If `null`, the default connection is used | | `sql` | `string\|null` | | `null` | Custom SQL query to execute, with optional named (`:name`) or positional (`?`) parameters | | `limit` | `int\|null` | | `null` | Maximum number of rows. Only used when `sql` is not set | @@ -66,7 +66,6 @@ read_books: read_books: service: '@CleverAge\DoctrineProcessBundle\Task\Database\DatabaseReaderTask' options: - table: 'book' # Required but not used sql: > SELECT b.id, b.title, a.lastname AS author FROM book b INNER JOIN author a ON a.id = b.author_id @@ -89,7 +88,6 @@ get_params: read_books: service: '@CleverAge\DoctrineProcessBundle\Task\Database\DatabaseReaderTask' options: - table: 'book' sql: 'SELECT * FROM book WHERE id >= :min_id' input_as_params: true types: @@ -108,5 +106,5 @@ Notes process is finalized. * Array parameters (e.g. for an `IN (:ids)` clause) require an `ArrayParameterType` in `types`, for instance `ids: !php/enum Doctrine\DBAL\ArrayParameterType::INTEGER` with Doctrine DBAL 4. -* The task is designed to be executed once per process run (typically as the entry point): if it receives a new input - after having iterated over all the rows, that input only resets the task, which is skipped. +* The query is executed again for each input received by the task (e.g. after an iterable task): with + `input_as_params`, each input gives its own parameters. diff --git a/docs/reference/tasks/doctrine_batchwriter_task.md b/docs/reference/tasks/doctrine_batchwriter_task.md index b0b5032..f8e226d 100644 --- a/docs/reference/tasks/doctrine_batchwriter_task.md +++ b/docs/reference/tasks/doctrine_batchwriter_task.md @@ -26,10 +26,10 @@ input is only buffered, and on flush if there is no remaining entity. Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `batch_count` | `int` | | `10` | Number of entities to buffer before writing them to the database | -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing each entity class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|-----------------------------------------------------------------------------------------------------------------------------------------| +| `batch_count` | `int` | | `10` | Number of entities to buffer before writing them to the database | +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the class of each entity is used | Examples -------- diff --git a/docs/reference/tasks/doctrine_cleaner_task.md b/docs/reference/tasks/doctrine_cleaner_task.md index 6362f8c..4b09c76 100644 --- a/docs/reference/tasks/doctrine_cleaner_task.md +++ b/docs/reference/tasks/doctrine_cleaner_task.md @@ -3,7 +3,7 @@ DoctrineCleanerTask Clears the entity manager that manages the class of the entity received as input: **all** the entities of this entity manager are detached (not only the input entity). Useful when the entity manager is not the default one, as it -is guessed from the input. +is guessed from the input (unless the `entity_manager` option is set). Task reference -------------- @@ -24,9 +24,9 @@ No output is set. Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing the input's class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|----------------------------------------------------------------------------------------------------------------------------------| +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the input's class is used | Examples -------- diff --git a/docs/reference/tasks/doctrine_detacher_task.md b/docs/reference/tasks/doctrine_detacher_task.md index 20f6c23..17cf032 100644 --- a/docs/reference/tasks/doctrine_detacher_task.md +++ b/docs/reference/tasks/doctrine_detacher_task.md @@ -23,9 +23,9 @@ No output is set. Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing the input's class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|----------------------------------------------------------------------------------------------------------------------------------| +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the input's class is used | Examples -------- diff --git a/docs/reference/tasks/doctrine_reader_task.md b/docs/reference/tasks/doctrine_reader_task.md index 33c7aa2..7f47f93 100644 --- a/docs/reference/tasks/doctrine_reader_task.md +++ b/docs/reference/tasks/doctrine_reader_task.md @@ -33,7 +33,7 @@ Options | `limit` | `int\|null` | | `null` | Maximum number of entities | | `offset` | `int\|null` | | `null` | Index of the first entity | | `empty_log_level` | `string` | | `warning` | PSR log level (`Psr\Log\LogLevel` values) used to log an empty result set | -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing `class_name` | +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the `class_name` is used | Examples -------- @@ -74,13 +74,14 @@ read_books: Notes ----- -* The query is executed on the first execution of the task and **all** the matching entities are loaded in memory - before being output one by one. For big volumes, use `limit`/`offset`, clear the entity manager downstream (see - [ClearEntityManagerTask](doctrine_clear_task.md)) or read raw rows with the +* The query is executed on the first execution of the task, then the entities are hydrated one at a time while the + process iterates (`Query::toIterable()`). They stay managed by the entity manager: for big volumes, clear it + downstream (see [ClearEntityManagerTask](doctrine_clear_task.md)) or detach the entities (see + [DoctrineDetacherTask](doctrine_detacher_task.md)) to keep the memory usage low, or read raw rows with the [DatabaseReaderTask](database_reader_task.md). * Entities stay managed by the entity manager: they can be modified then saved with the [DoctrineWriterTask](doctrine_writer_task.md). -* The task is designed to be executed once per process run (typically as the entry point): if it receives a new input - after having iterated over all the entities, that input only resets the task, which is skipped. +* The query is executed again for each input received by the task (e.g. after an iterable task); the input itself + is not used. * For more complex queries, extend `CleverAge\DoctrineProcessBundle\Task\EntityManager\AbstractDoctrineQueryTask` (which provides the options above and a `getQueryBuilder()` method) or this task. diff --git a/docs/reference/tasks/doctrine_refresher_task.md b/docs/reference/tasks/doctrine_refresher_task.md index 4d4cb9e..55ed35c 100644 --- a/docs/reference/tasks/doctrine_refresher_task.md +++ b/docs/reference/tasks/doctrine_refresher_task.md @@ -23,9 +23,9 @@ Possible outputs Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing the input's class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|----------------------------------------------------------------------------------------------------------------------------------| +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the input's class is used | Examples -------- diff --git a/docs/reference/tasks/doctrine_remover_task.md b/docs/reference/tasks/doctrine_remover_task.md index 90f8711..b52f08f 100644 --- a/docs/reference/tasks/doctrine_remover_task.md +++ b/docs/reference/tasks/doctrine_remover_task.md @@ -11,8 +11,8 @@ Task reference Accepted inputs --------------- -`object`: a Doctrine managed entity. An object whose class is not managed by any entity manager throws an -`\UnexpectedValueException`. +`object`: a Doctrine managed entity. A `null` input throws a `\RuntimeException`, and an object whose class is not +managed by any entity manager throws an `\UnexpectedValueException`. Possible outputs ---------------- @@ -22,9 +22,9 @@ No output is set. Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing the input's class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|----------------------------------------------------------------------------------------------------------------------------------| +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the input's class is used | Examples -------- @@ -50,4 +50,3 @@ Notes * `flush()` writes **all** the pending changes of the entity manager, not only the removal. * Cascade and `orphanRemoval` rules of the entity mapping apply. To delete many rows at once, a single `DELETE` statement with the [DatabaseUpdaterTask](database_updater_task.md) is much faster. -* A `null` input is not supported (it throws a `\TypeError`). diff --git a/docs/reference/tasks/doctrine_writer_task.md b/docs/reference/tasks/doctrine_writer_task.md index 95296ac..dc2c44e 100644 --- a/docs/reference/tasks/doctrine_writer_task.md +++ b/docs/reference/tasks/doctrine_writer_task.md @@ -23,9 +23,9 @@ Possible outputs Options ------- -| Code | Type | Required | Default | Description | -|------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------| -| `entity_manager` | `string\|null` | | `null` | Inherited from the base Doctrine task but not used: the entity manager is the one managing the input's class | +| Code | Type | Required | Default | Description | +|------------------|----------------|:--------:|---------|----------------------------------------------------------------------------------------------------------------------------------| +| `entity_manager` | `string\|null` | | `null` | Name of the entity manager (as defined in `doctrine.orm.entity_managers`). If `null`, the one managing the input's class is used | Examples -------- diff --git a/src/Task/Database/DatabaseReaderTask.php b/src/Task/Database/DatabaseReaderTask.php index 6718e32..43f7070 100644 --- a/src/Task/Database/DatabaseReaderTask.php +++ b/src/Task/Database/DatabaseReaderTask.php @@ -25,6 +25,8 @@ use Doctrine\Persistence\ManagerRegistry; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; +use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; +use Symfony\Component\OptionsResolver\Options as ResolvedOptions; use Symfony\Component\OptionsResolver\OptionsResolver; /** @@ -32,7 +34,7 @@ * * @phpstan-type Options array{ * 'sql': ?string, - * 'table': string, + * 'table': ?string, * 'limit': ?int, * 'empty_log_level': string, * 'paginate': ?int, @@ -66,8 +68,16 @@ public function next(ProcessState $state): bool } $this->nextItem = $this->statement->fetchAssociative(); + if (false !== $this->nextItem) { + return true; + } + + // End of the iteration: the next input executes the query again + $this->statement->free(); + $this->statement = null; + $this->nextItem = null; - return (bool) $this->nextItem; + return false; } public function execute(ProcessState $state): void @@ -124,9 +134,11 @@ protected function initializeStatement(ProcessState $state): Result if (null === $sql) { $qb = $connection->createQueryBuilder(); + /** @var string $table Required without sql */ + $table = $options['table']; $qb ->select('tbl.*') - ->from($options['table'], 'tbl'); + ->from($table, 'tbl'); if ($options['limit']) { $qb->setMaxResults($options['limit']); @@ -150,10 +162,9 @@ protected function initializeStatement(ProcessState $state): Result protected function configureOptions(OptionsResolver $resolver): void { - $resolver->setRequired(['table']); - $resolver->setAllowedTypes('table', ['string']); $resolver->setDefaults( [ + 'table' => null, 'connection' => null, 'sql' => null, 'limit' => null, @@ -165,6 +176,15 @@ protected function configureOptions(OptionsResolver $resolver): void 'empty_log_level' => LogLevel::WARNING, ] ); + $resolver->setAllowedTypes('table', ['null', 'string']); + // Only used to build the query when sql is not given + $resolver->setNormalizer('table', static function (ResolvedOptions $options, ?string $table): ?string { + if (null === $table && null === $options['sql']) { + throw new MissingOptionsException('The option "table" is required when the option "sql" is not set.'); + } + + return $table; + }); $resolver->setAllowedTypes('connection', ['null', 'string']); $resolver->setAllowedTypes('sql', ['null', 'string']); $resolver->setAllowedTypes('paginate', ['null', 'int']); diff --git a/src/Task/EntityManager/AbstractDoctrineTask.php b/src/Task/EntityManager/AbstractDoctrineTask.php index 397db62..79e84c6 100644 --- a/src/Task/EntityManager/AbstractDoctrineTask.php +++ b/src/Task/EntityManager/AbstractDoctrineTask.php @@ -15,6 +15,7 @@ use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; use CleverAge\ProcessBundle\Model\ProcessState; +use Doctrine\ORM\EntityManagerInterface; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Persistence\ObjectManager; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -44,4 +45,23 @@ protected function getManager(ProcessState $state): ObjectManager return $this->doctrine->getManager($entityManagerName); } + + /** + * Entity manager given by the entity_manager option, or the one managing the class when the option is not set. + * + * @param class-string $class + */ + protected function getEntityManager(ProcessState $state, string $class): EntityManagerInterface + { + /** @var ?string $entityManagerName */ + $entityManagerName = $this->getOption($state, 'entity_manager'); + $entityManager = null === $entityManagerName + ? $this->doctrine->getManagerForClass($class) + : $this->doctrine->getManager($entityManagerName); + if (!$entityManager instanceof EntityManagerInterface) { + throw new \UnexpectedValueException("No manager found for class {$class}"); + } + + return $entityManager; + } } diff --git a/src/Task/EntityManager/DoctrineBatchWriterTask.php b/src/Task/EntityManager/DoctrineBatchWriterTask.php index 73d9de0..8e9164e 100644 --- a/src/Task/EntityManager/DoctrineBatchWriterTask.php +++ b/src/Task/EntityManager/DoctrineBatchWriterTask.php @@ -68,10 +68,7 @@ protected function writeBatch(ProcessState $state): void $entityManagers = new \SplObjectStorage(); foreach ($this->batch as $entity) { $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->persist($entity); $entityManagers->offsetSet($entityManager); } diff --git a/src/Task/EntityManager/DoctrineCleanerTask.php b/src/Task/EntityManager/DoctrineCleanerTask.php index 55d7fba..b75f9db 100644 --- a/src/Task/EntityManager/DoctrineCleanerTask.php +++ b/src/Task/EntityManager/DoctrineCleanerTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\Common\Util\ClassUtils; -use Doctrine\ORM\EntityManagerInterface; /** * Clean Doctrine entities from unit of work. @@ -30,10 +29,7 @@ public function execute(ProcessState $state): void } /** @var object $entity */ $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->clear(); } } diff --git a/src/Task/EntityManager/DoctrineDetacherTask.php b/src/Task/EntityManager/DoctrineDetacherTask.php index a72fa5b..f9ae23a 100644 --- a/src/Task/EntityManager/DoctrineDetacherTask.php +++ b/src/Task/EntityManager/DoctrineDetacherTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\Common\Util\ClassUtils; -use Doctrine\ORM\EntityManagerInterface; /** * Detach Doctrine entities from unit of work. @@ -26,14 +25,11 @@ public function execute(ProcessState $state): void { $entity = $state->getInput(); if (null === $entity) { - throw new \RuntimeException('DoctrineWriterTask does not allow null input'); + throw new \RuntimeException('DoctrineDetacherTask does not allow null input'); } /** @var object $entity */ $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->detach($entity); } } diff --git a/src/Task/EntityManager/DoctrineReaderTask.php b/src/Task/EntityManager/DoctrineReaderTask.php index 2b4ce1c..6fabc5e 100644 --- a/src/Task/EntityManager/DoctrineReaderTask.php +++ b/src/Task/EntityManager/DoctrineReaderTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\IterableTaskInterface; use CleverAge\ProcessBundle\Model\ProcessState; -use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\Persistence\ManagerRegistry; use Psr\Log\LoggerInterface; @@ -47,8 +46,14 @@ public function next(ProcessState $state): bool return false; } $this->iterator->next(); + if ($this->iterator->valid()) { + return true; + } + + // End of the iteration: the next input executes the query again + $this->iterator = null; - return $this->iterator->valid(); + return false; } public function execute(ProcessState $state): void @@ -58,10 +63,7 @@ public function execute(ProcessState $state): void if (!$this->iterator instanceof \Iterator) { /** @var class-string $class */ $class = $options['class_name']; - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $repository = $entityManager->getRepository($class); $this->initIterator($repository, $options); } @@ -100,7 +102,9 @@ protected function initIterator(EntityRepository $repository, array $options): v $options['offset'] ); - $this->iterator = new \ArrayIterator(iterator_to_array($qb->getQuery()->toIterable())); + // Keep the iterator, so the entities are hydrated one at a time while the process iterates + $iterable = $qb->getQuery()->toIterable(); + $this->iterator = \is_array($iterable) ? new \ArrayIterator($iterable) : new \IteratorIterator($iterable); $this->iterator->rewind(); } } diff --git a/src/Task/EntityManager/DoctrineRefresherTask.php b/src/Task/EntityManager/DoctrineRefresherTask.php index ebfa855..7e9d270 100644 --- a/src/Task/EntityManager/DoctrineRefresherTask.php +++ b/src/Task/EntityManager/DoctrineRefresherTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\Common\Util\ClassUtils; -use Doctrine\ORM\EntityManagerInterface; /** * Refreshes a Doctrine entity from the database. @@ -30,10 +29,7 @@ public function execute(ProcessState $state): void } /** @var object $entity */ $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->refresh($entity); $state->setOutput($entity); diff --git a/src/Task/EntityManager/DoctrineRemoverTask.php b/src/Task/EntityManager/DoctrineRemoverTask.php index 1be5850..6403d64 100644 --- a/src/Task/EntityManager/DoctrineRemoverTask.php +++ b/src/Task/EntityManager/DoctrineRemoverTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\Common\Util\ClassUtils; -use Doctrine\ORM\EntityManagerInterface; /** * Remove Doctrine entities. @@ -25,12 +24,12 @@ class DoctrineRemoverTask extends AbstractDoctrineTask public function execute(ProcessState $state): void { $entity = $state->getInput(); + if (null === $entity) { + throw new \RuntimeException('DoctrineRemoverTask does not allow null input'); + } /** @var object $entity */ $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->remove($entity); $entityManager->flush(); } diff --git a/src/Task/EntityManager/DoctrineWriterTask.php b/src/Task/EntityManager/DoctrineWriterTask.php index f069376..0c8653d 100644 --- a/src/Task/EntityManager/DoctrineWriterTask.php +++ b/src/Task/EntityManager/DoctrineWriterTask.php @@ -15,7 +15,6 @@ use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\Common\Util\ClassUtils; -use Doctrine\ORM\EntityManagerInterface; /** * Persists and flush Doctrine entities. @@ -37,10 +36,7 @@ protected function writeEntity(ProcessState $state): mixed throw new \RuntimeException('DoctrineWriterTask does not allow null input'); } $class = ClassUtils::getClass($entity); - $entityManager = $this->doctrine->getManagerForClass($class); - if (!$entityManager instanceof EntityManagerInterface) { - throw new \UnexpectedValueException("No manager found for class {$class}"); - } + $entityManager = $this->getEntityManager($state, $class); $entityManager->persist($entity); $entityManager->flush(); diff --git a/tests/Task/Database/DatabaseReaderTaskSqliteTest.php b/tests/Task/Database/DatabaseReaderTaskSqliteTest.php new file mode 100644 index 0000000..72e49e1 --- /dev/null +++ b/tests/Task/Database/DatabaseReaderTaskSqliteTest.php @@ -0,0 +1,153 @@ +connection = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true]); + $this->connection->executeStatement('CREATE TABLE book (id INTEGER PRIMARY KEY, title VARCHAR(255))'); + foreach (['It', 'Salem', 'Fahrenheit 451'] as $i => $title) { + $this->connection->insert('book', ['id' => $i + 1, 'title' => $title]); + } + } + + public function testReadTable(): void + { + [$task, $state] = $this->createTask(['table' => 'book']); + + self::assertSame( + [['id' => 1, 'title' => 'It'], ['id' => 2, 'title' => 'Salem'], ['id' => 3, 'title' => 'Fahrenheit 451']], + $this->iterate($task, $state, null) + ); + } + + public function testSqlWithoutTable(): void + { + [$task, $state] = $this->createTask(['sql' => 'SELECT title FROM book ORDER BY id']); + + self::assertSame([['title' => 'It'], ['title' => 'Salem'], ['title' => 'Fahrenheit 451']], $this->iterate($task, $state, null)); + } + + public function testTableOrSqlRequired(): void + { + $this->expectException(MissingOptionsException::class); + $this->expectExceptionMessage('The option "table" is required when the option "sql" is not set.'); + $this->createTask([]); + } + + public function testEachInputExecutesTheQueryAgain(): void + { + [$task, $state] = $this->createTask(['sql' => 'SELECT title FROM book ORDER BY id']); + $titles = [['title' => 'It'], ['title' => 'Salem'], ['title' => 'Fahrenheit 451']]; + + self::assertSame($titles, $this->iterate($task, $state, 'first')); + self::assertSame($titles, $this->iterate($task, $state, 'second')); + self::assertSame($titles, $this->iterate($task, $state, 'third')); + } + + public function testEachInputExecutesTheQueryAgainWithPagination(): void + { + [$task, $state] = $this->createTask(['sql' => 'SELECT id FROM book ORDER BY id', 'paginate' => 2]); + $pages = [[['id' => 1], ['id' => 2]], [['id' => 3]]]; + + self::assertSame($pages, $this->iterate($task, $state, 'first')); + self::assertSame($pages, $this->iterate($task, $state, 'second')); + } + + public function testInputAsParams(): void + { + [$task, $state] = $this->createTask(['sql' => 'SELECT title FROM book WHERE id = :id', 'input_as_params' => true]); + + self::assertSame([['title' => 'Salem']], $this->iterate($task, $state, ['id' => 2])); + self::assertSame([['title' => 'It']], $this->iterate($task, $state, ['id' => 1])); + } + + public function testArrayParameter(): void + { + [$task, $state] = $this->createTask([ + 'sql' => 'SELECT title FROM book WHERE id IN (:ids) ORDER BY id', + 'params' => ['ids' => [1, 3]], + 'types' => ['ids' => ArrayParameterType::INTEGER], + ]); + + self::assertSame([['title' => 'It'], ['title' => 'Fahrenheit 451']], $this->iterate($task, $state, null)); + } + + /** + * @param array $options + * + * @return array{DatabaseReaderTask, ProcessState} + */ + private function createTask(array $options): array + { + $doctrine = $this->createStub(ManagerRegistry::class); + $doctrine->method('getConnection')->willReturn($this->connection); + + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('read', DatabaseReaderTask::class, $options)); + + $task = new DatabaseReaderTask(new NullLogger(), $doctrine); + $task->initialize($state); + + return [$task, $state]; + } + + /** + * Execute the task for one input, then iterate until next() returns false, as the process manager does. + * + * @return list + */ + private function iterate(DatabaseReaderTask $task, ProcessState $state, mixed $input): array + { + $outputs = []; + $state->reset(true); + $state->setInput($input); + do { + $state->reset(false); + $task->execute($state); + if ($state->isSkipped()) { + break; + } + $outputs[] = $state->getOutput(); + } while ($task->next($state)); + + return $outputs; + } +} diff --git a/tests/Task/EntityManager/AbstractDoctrineTaskTest.php b/tests/Task/EntityManager/AbstractDoctrineTaskTest.php new file mode 100644 index 0000000..e81ab6f --- /dev/null +++ b/tests/Task/EntityManager/AbstractDoctrineTaskTest.php @@ -0,0 +1,89 @@ +createMock(EntityManagerInterface::class); + $entityManager->expects(self::once())->method('persist')->with($entity); + + $doctrine = $this->createMock(ManagerRegistry::class); + $doctrine->expects(self::once())->method('getManagerForClass')->with(\stdClass::class)->willReturn($entityManager); + $doctrine->expects(self::never())->method('getManager'); + + $this->execute($doctrine, [], $entity); + } + + public function testEntityManagerOption(): void + { + $entity = new \stdClass(); + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->expects(self::once())->method('persist')->with($entity); + + $doctrine = $this->createMock(ManagerRegistry::class); + $doctrine->expects(self::once())->method('getManager')->with('customer')->willReturn($entityManager); + $doctrine->expects(self::never())->method('getManagerForClass'); + + $this->execute($doctrine, ['entity_manager' => 'customer'], $entity); + } + + public function testNotAnEntityManager(): void + { + $doctrine = $this->createStub(ManagerRegistry::class); + $doctrine->method('getManager')->willReturn($this->createStub(ObjectManager::class)); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('No manager found for class stdClass'); + $this->execute($doctrine, ['entity_manager' => 'odm'], new \stdClass()); + } + + /** + * @param array $options + */ + private function execute(ManagerRegistry $doctrine, array $options, object $entity): void + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('write', DoctrineWriterTask::class, $options)); + $state->setInput($entity); + + $task = new DoctrineWriterTask($doctrine); + $task->initialize($state); + $task->execute($state); + } +} diff --git a/tests/Task/EntityManager/DoctrineBatchWriterTaskTest.php b/tests/Task/EntityManager/DoctrineBatchWriterTaskTest.php index 7a13d4a..af44313 100644 --- a/tests/Task/EntityManager/DoctrineBatchWriterTaskTest.php +++ b/tests/Task/EntityManager/DoctrineBatchWriterTaskTest.php @@ -44,6 +44,7 @@ public function testExecuteAddsEntityToBatchAndSkipsWhenBatchCountNotReached(): { $entity1 = new \stdClass(); $state = $this->createMock(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn($entity1); $task = $this->getTask(['batch_count' => 2]); @@ -63,6 +64,7 @@ public function testExecuteFlushesBatchWhenBatchCountReached(): void $entity2 = new \stdClass(); $state = $this->createMock(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturnOnConsecutiveCalls($entity1, $entity2); $state->expects($this->once())->method('setOutput')->with([$entity1, $entity2]); @@ -91,6 +93,7 @@ public function testExecuteFlushesBatchWhenBatchCountReached(): void public function testFlushCallsWriteBatch(): void { $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $task = $this->getMockBuilder(DoctrineBatchWriterTask::class) ->disableOriginalConstructor() ->onlyMethods(['writeBatch']) @@ -103,6 +106,7 @@ public function testFlushCallsWriteBatch(): void public function testWriteBatchWithEmptyBatchSkipsState(): void { $state = $this->createMock(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->expects($this->once())->method('setSkipped')->with(true); $managerRegistry = $this->createStub(ManagerRegistry::class); @@ -123,6 +127,7 @@ public function testWriteBatchPersistsAndFlushesEntities(): void $entity1 = new \stdClass(); $entity2 = new \stdClass(); $state = $this->createMock(ProcessState::class); // Use mock to set expectation on setSkipped + $state->method('getContextualizedOptions')->willReturn([]); $state->expects($this->never())->method('setSkipped'); // Should not be skipped if batch is not empty $entityManager = $this->createMock(EntityManagerInterface::class); @@ -148,6 +153,7 @@ public function testWriteBatchClearsBatchAfterFlushing(): void { $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $entityManager = $this->createStub(EntityManagerInterface::class); $managerRegistry = $this->createStub(ManagerRegistry::class); @@ -170,6 +176,7 @@ public function testWriteBatchSetsOutput(): void { $entity = new \stdClass(); $state = $this->createMock(ProcessState::class); // Use mock to set expectation on setOutput + $state->method('getContextualizedOptions')->willReturn([]); $state->expects($this->once())->method('setOutput')->with([$entity]); $entityManager = $this->createStub(EntityManagerInterface::class); @@ -193,6 +200,7 @@ public function testWriteBatchThrowsExceptionWhenNoManagerFound(): void $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $managerRegistry = $this->createStub(ManagerRegistry::class); $managerRegistry->method('getManagerForClass')->willReturn(null); // Simulate no manager found diff --git a/tests/Task/EntityManager/DoctrineCleanerTaskTest.php b/tests/Task/EntityManager/DoctrineCleanerTaskTest.php index 09d3cec..f28de3a 100644 --- a/tests/Task/EntityManager/DoctrineCleanerTaskTest.php +++ b/tests/Task/EntityManager/DoctrineCleanerTaskTest.php @@ -27,6 +27,7 @@ public function testExecuteWithEntity(): void { $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn($entity); $entityManager = $this->createMock(EntityManagerInterface::class); @@ -44,6 +45,7 @@ public function testExecuteWithNullInput(): void $this->expectException(\RuntimeException::class); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn(null); $managerRegistry = $this->createStub(ManagerRegistry::class); @@ -58,6 +60,7 @@ public function testExecuteWithNoManager(): void $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn($entity); $managerRegistry = $this->createStub(ManagerRegistry::class); diff --git a/tests/Task/EntityManager/DoctrineDetacherTaskTest.php b/tests/Task/EntityManager/DoctrineDetacherTaskTest.php index 0770d69..c55b8f5 100644 --- a/tests/Task/EntityManager/DoctrineDetacherTaskTest.php +++ b/tests/Task/EntityManager/DoctrineDetacherTaskTest.php @@ -59,6 +59,7 @@ public function testExecuteDetachesEntity(): void public function testExecuteThrowsExceptionOnNullInput(): void { $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('DoctrineDetacherTask does not allow null input'); $state = $this->createStub(ProcessState::class); $state->method('getInput')->willReturn(null); diff --git a/tests/Task/EntityManager/DoctrineReaderTaskTest.php b/tests/Task/EntityManager/DoctrineReaderTaskTest.php index 267a323..ab8ea83 100644 --- a/tests/Task/EntityManager/DoctrineReaderTaskTest.php +++ b/tests/Task/EntityManager/DoctrineReaderTaskTest.php @@ -14,6 +14,10 @@ namespace CleverAge\DoctrineProcessBundle\Tests\Task\EntityManager; use CleverAge\DoctrineProcessBundle\Task\EntityManager\DoctrineReaderTask; +use CleverAge\ProcessBundle\Configuration\ProcessConfiguration; +use CleverAge\ProcessBundle\Configuration\TaskConfiguration; +use CleverAge\ProcessBundle\Context\ContextualOptionResolver; +use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; @@ -24,6 +28,7 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; +use Psr\Log\NullLogger; #[CoversClass(DoctrineReaderTask::class)] class DoctrineReaderTaskTest extends TestCase @@ -40,6 +45,7 @@ public function testExecute(): void 'limit' => 10, 'offset' => 0, 'empty_log_level' => LogLevel::WARNING, + 'entity_manager' => null, ]; $entity = new \stdClass(); @@ -101,6 +107,7 @@ public function testExecuteEmpty(): void 'limit' => null, 'offset' => null, 'empty_log_level' => LogLevel::WARNING, + 'entity_manager' => null, ]; $query = $this->createStub(Query::class); @@ -158,6 +165,7 @@ public function testNext(): void 'limit' => null, 'offset' => null, 'empty_log_level' => LogLevel::WARNING, + 'entity_manager' => null, ]; $entity1 = new \stdClass(); @@ -232,6 +240,7 @@ public function testExecuteThrowsExceptionWhenNoManagerFound(): void 'limit' => null, 'offset' => null, 'empty_log_level' => LogLevel::WARNING, + 'entity_manager' => null, ]; $doctrine->method('getManagerForClass')->willReturn(null); @@ -257,4 +266,81 @@ protected function getOptions(?ProcessState $state = null): array $task->initialize($state); $task->execute($state); } + + public function testEntitiesAreHydratedWhileIterating(): void + { + $consumed = 0; + [$task, $state] = $this->createIteratingTask(static function () use (&$consumed): \Generator { + foreach (['entity1', 'entity2', 'entity3'] as $entity) { + ++$consumed; + yield (object) ['name' => $entity]; + } + }); + + $task->execute($state); + + // Only the first entity has been fetched from the query + self::assertSame(1, $consumed); + self::assertTrue($task->next($state)); + self::assertSame(2, $consumed); + } + + public function testEachInputExecutesTheQueryAgain(): void + { + $queries = 0; + [$task, $state] = $this->createIteratingTask(static function () use (&$queries): \Generator { + ++$queries; + yield (object) ['name' => 'entity1']; + yield (object) ['name' => 'entity2']; + }); + + foreach (['first', 'second', 'third'] as $input) { + $names = []; + do { + $state->reset(false); + $task->execute($state); + self::assertFalse($state->isSkipped(), "Input {$input} skipped"); + /** @var object{name: string} $output */ + $output = $state->getOutput(); + $names[] = $output->name; + } while ($task->next($state)); + + self::assertSame(['entity1', 'entity2'], $names); + } + self::assertSame(3, $queries); + } + + /** + * @param \Closure(): \Generator $results + * + * @return array{DoctrineReaderTask, ProcessState} + */ + private function createIteratingTask(\Closure $results): array + { + $query = $this->createStub(Query::class); + $query->method('toIterable')->willReturnCallback($results); + + $qb = $this->createStub(QueryBuilder::class); + $qb->method('getQuery')->willReturn($query); + + $repository = $this->createStub(EntityRepository::class); + $repository->method('createQueryBuilder')->willReturn($qb); + + $em = $this->createStub(EntityManagerInterface::class); + $em->method('getRepository')->willReturn($repository); + + $doctrine = $this->createStub(ManagerRegistry::class); + $doctrine->method('getManagerForClass')->willReturn($em); + + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('read', DoctrineReaderTask::class, ['class_name' => 'App\\Entity\\MyEntity'])); + + $task = new DoctrineReaderTask(new NullLogger(), $doctrine); + $task->initialize($state); + + return [$task, $state]; + } } diff --git a/tests/Task/EntityManager/DoctrineRemoverTaskTest.php b/tests/Task/EntityManager/DoctrineRemoverTaskTest.php index 7d689b8..488219a 100644 --- a/tests/Task/EntityManager/DoctrineRemoverTaskTest.php +++ b/tests/Task/EntityManager/DoctrineRemoverTaskTest.php @@ -27,6 +27,7 @@ public function testExecuteWithEntity(): void { $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn($entity); $entityManager = $this->createMock(EntityManagerInterface::class); @@ -42,9 +43,11 @@ public function testExecuteWithEntity(): void public function testExecuteWithNullInput(): void { - $this->expectException(\TypeError::class); // ClassUtils::getClass expects an object, null will cause a TypeError + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('DoctrineRemoverTask does not allow null input'); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn(null); $managerRegistry = $this->createStub(ManagerRegistry::class); @@ -59,6 +62,7 @@ public function testExecuteWithNoManager(): void $entity = new \stdClass(); $state = $this->createStub(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([]); $state->method('getInput')->willReturn($entity); $managerRegistry = $this->createStub(ManagerRegistry::class);