diff --git a/UPGRADE-1.0.md b/UPGRADE-1.0.md index c341d34b..7d7cbeec 100644 --- a/UPGRADE-1.0.md +++ b/UPGRADE-1.0.md @@ -242,3 +242,32 @@ public function purge(Query $query): void ``` The built-in `DoctrineDBALJobExecutionStorage` and `FilesystemJobExecutionStorage` already implement this method — no action required if you use either of them. + +--- + +### OpenSpout — `HeaderStrategy` and `SheetFilter` interface changes (BREAKING) + +The two interfaces have new, intentional public contracts. The previous `@internal` methods have been removed. + +#### `HeaderStrategy` + +`setHeaders()` and `getItem()` are replaced by a single method: + +```diff +-public function setHeaders(array $headers): bool; +-public function getItem(array $row): array; ++public function process(array $row, bool $isFirstRow): array|null; +``` + +`process()` receives the raw row and whether it is the first row of the sheet. Return `null` to skip the row (e.g. it is a header row), or return an array to yield it as an item. + +#### `SheetFilter` + +`list()` is replaced by `accepts()`: + +```diff +-public function list(ReaderInterface $reader): Generator; ++public function accepts(SheetInterface $sheet): bool; +``` + +`accepts()` receives a single sheet and returns whether it should be read. diff --git a/docs/docs/bridges/doctrine-dbal/insert-writer.php b/docs/docs/bridges/doctrine-dbal/insert-writer.php index 7dff3de7..51014458 100644 --- a/docs/docs/bridges/doctrine-dbal/insert-writer.php +++ b/docs/docs/bridges/doctrine-dbal/insert-writer.php @@ -2,13 +2,22 @@ declare(strict_types=1); +use Doctrine\DBAL\Types\Types; use Doctrine\Persistence\ConnectionRegistry; use Yokai\Batch\Bridge\Doctrine\DBAL\DoctrineDBALInsertWriter; /** @var ConnectionRegistry $connectionRegistry */ +// Column types are inferred from the table schema automatically new DoctrineDBALInsertWriter( doctrine: $connectionRegistry, table: 'user', connection: null, // will use default one, but you can pick any registered connection name ); + +// Or provide explicit type hints to override auto-detection +new DoctrineDBALInsertWriter( + doctrine: $connectionRegistry, + table: 'user', + types: ['created_at' => Types::DATETIME_IMMUTABLE, 'active' => Types::BOOLEAN], +); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5d4ae7ad..e726526a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5,11 +5,6 @@ parameters: count: 1 path: src/batch-openspout/src/Reader/FlatFileReader.php - - - message: "#^Parameter \\#1 \\$headers of method Yokai\\\\Batch\\\\Bridge\\\\OpenSpout\\\\Reader\\\\HeaderStrategy\\:\\:setHeaders\\(\\) expects array\\, array\\ given\\.$#" - count: 1 - path: src/batch-openspout/src/Reader/FlatFileReader.php - - message: "#^Parameter \\#1 \\$options of class OpenSpout\\\\Reader\\\\CSV\\\\Reader constructor expects OpenSpout\\\\Reader\\\\CSV\\\\Options\\|null, OpenSpout\\\\Reader\\\\CSV\\\\Options\\|OpenSpout\\\\Reader\\\\ODS\\\\Options\\|OpenSpout\\\\Reader\\\\XLSX\\\\Options\\|null given\\.$#" count: 1 @@ -26,20 +21,10 @@ parameters: path: src/batch-openspout/src/Reader/FlatFileReader.php - - message: "#^Parameter \\#1 \\$row of method Yokai\\\\Batch\\\\Bridge\\\\OpenSpout\\\\Reader\\\\HeaderStrategy\\:\\:getItem\\(\\) expects array\\, array\\ given\\.$#" + message: "#^Parameter \\#1 \\$row of method Yokai\\\\Batch\\\\Bridge\\\\OpenSpout\\\\Reader\\\\HeaderStrategy\\:\\:process\\(\\) expects array\\, array\\ given\\.$#" count: 1 path: src/batch-openspout/src/Reader/FlatFileReader.php - - - message: "#^Method Yokai\\\\Batch\\\\Bridge\\\\OpenSpout\\\\Reader\\\\SheetFilter\\:\\:list\\(\\) has parameter \\$reader with generic interface OpenSpout\\\\Reader\\\\ReaderInterface but does not specify its types\\: T$#" - count: 1 - path: src/batch-openspout/src/Reader/SheetFilter.php - - - - message: "#^Method Yokai\\\\Batch\\\\Bridge\\\\OpenSpout\\\\Reader\\\\SheetFilter\\:\\:list\\(\\) return type with generic interface OpenSpout\\\\Reader\\\\SheetInterface does not specify its types\\: T$#" - count: 1 - path: src/batch-openspout/src/Reader/SheetFilter.php - - message: "#^Parameter \\#1 \\$options of class OpenSpout\\\\Writer\\\\CSV\\\\Writer constructor expects OpenSpout\\\\Writer\\\\CSV\\\\Options\\|null, OpenSpout\\\\Writer\\\\CSV\\\\Options\\|OpenSpout\\\\Writer\\\\ODS\\\\Options\\|OpenSpout\\\\Writer\\\\XLSX\\\\Options\\|null given\\.$#" count: 1 diff --git a/src/batch-doctrine-dbal/src/DoctrineDBALInsertWriter.php b/src/batch-doctrine-dbal/src/DoctrineDBALInsertWriter.php index 2344cd3b..59817705 100644 --- a/src/batch-doctrine-dbal/src/DoctrineDBALInsertWriter.php +++ b/src/batch-doctrine-dbal/src/DoctrineDBALInsertWriter.php @@ -5,6 +5,8 @@ namespace Yokai\Batch\Bridge\Doctrine\DBAL; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\ParameterType; +use Doctrine\DBAL\Types\Type; use Doctrine\Persistence\ConnectionRegistry; use Yokai\Batch\Exception\UnexpectedValueException; use Yokai\Batch\Job\Item\ItemWriterInterface; @@ -13,30 +15,62 @@ * This {@see ItemWriterInterface} will insert all items to a single table, * via a Doctrine {@see Connection}. * All items must be arrays. + * Column types are inferred from the table schema on first write when not provided explicitly. */ -final readonly class DoctrineDBALInsertWriter implements ItemWriterInterface +final class DoctrineDBALInsertWriter implements ItemWriterInterface { - private Connection $connection; + private readonly Connection $connection; + /** + * @var array, string|ParameterType|Type>|array|null + */ + private array|null $types; + + /** + * @param array, string|ParameterType|Type>|array|null $types + * Column type hints for DBAL binding. When null, types are resolved lazily on first write + * via table schema introspection. Pass an empty array to disable type resolution entirely. + */ public function __construct( ConnectionRegistry $doctrine, - private string $table, + /** + * @var non-empty-string + */ + private readonly string $table, string|null $connection = null, + array|null $types = null, ) { $connection ??= $doctrine->getDefaultConnectionName(); - /** @var Connection $connection */ $connection = $doctrine->getConnection($connection); + /** @var Connection $connection */ $this->connection = $connection; + $this->types = $types; } public function write(iterable $items): void { + $this->types ??= $this->resolveTypes(); + foreach ($items as $item) { if (!\is_array($item)) { throw UnexpectedValueException::type('array', $item); } - $this->connection->insert($this->table, $item); + $this->connection->insert($this->table, $item, $this->types); } } + + /** + * @return array + */ + private function resolveTypes(): array + { + $types = []; + $table = $this->connection->createSchemaManager()->introspectTableByUnquotedName($this->table); + foreach ($table->getColumns() as $column) { + $types[$column->getObjectName()->toString()] = $column->getType(); + } + + return $types; + } } diff --git a/src/batch-doctrine-dbal/tests/DoctrineDBALInsertWriterTest.php b/src/batch-doctrine-dbal/tests/DoctrineDBALInsertWriterTest.php index 8587be3a..7f184f4c 100644 --- a/src/batch-doctrine-dbal/tests/DoctrineDBALInsertWriterTest.php +++ b/src/batch-doctrine-dbal/tests/DoctrineDBALInsertWriterTest.php @@ -4,6 +4,7 @@ namespace Yokai\Batch\Tests\Bridge\Doctrine\DBAL; +use Doctrine\DBAL\Schema\Exception\TableDoesNotExist; use Doctrine\DBAL\Types\Types; use Yokai\Batch\Bridge\Doctrine\DBAL\DoctrineDBALInsertWriter; use Yokai\Batch\Exception\UnexpectedValueException; @@ -38,10 +39,72 @@ public function test(): void ], $this->findAll('persons')); } + public function testAutoDetectsColumnTypes(): void + { + $this->createTable('persons', [ + 'id' => Types::INTEGER, + 'firstName' => Types::STRING, + ]); + + $writer = new DoctrineDBALInsertWriter($this->doctrine, 'persons'); + + $writer->write([ + ['id' => 1, 'firstName' => 'John'], + ['id' => 2, 'firstName' => 'Jane'], + ]); + + self::assertSame([ + ['id' => '1', 'firstName' => 'John'], + ['id' => '2', 'firstName' => 'Jane'], + ], $this->findAll('persons')); + } + + public function testWithExplicitTypes(): void + { + $this->createTable('events', [ + 'name' => Types::STRING, + 'occurred_at' => Types::DATETIME_IMMUTABLE, + 'active' => Types::BOOLEAN, + ]); + + $writer = new DoctrineDBALInsertWriter( + $this->doctrine, + 'events', + types: [ + 'occurred_at' => Types::DATETIME_IMMUTABLE, + 'active' => Types::BOOLEAN, + ], + ); + + $writer->write([ + ['name' => 'signup', 'occurred_at' => new \DateTimeImmutable('2024-01-15 10:00:00'), 'active' => true], + ['name' => 'logout', 'occurred_at' => new \DateTimeImmutable('2024-01-16 08:30:00'), 'active' => false], + ]); + + self::assertSame([ + ['name' => 'signup', 'occurred_at' => '2024-01-15 10:00:00', 'active' => '1'], + ['name' => 'logout', 'occurred_at' => '2024-01-16 08:30:00', 'active' => '0'], + ], $this->findAll('events')); + } + public function testItemNotAnArray(): void { + $this->createTable('persons', [ + 'firstName' => Types::STRING, + 'lastName' => Types::STRING, + ]); $this->expectException(UnexpectedValueException::class); $writer = new DoctrineDBALInsertWriter($this->doctrine, 'persons'); $writer->write(['string']); } + + public function testTableDoesNotExists(): void + { + $this->expectException(TableDoesNotExist::class); + $writer = new DoctrineDBALInsertWriter($this->doctrine, 'persons'); + $writer->write([ + ['firstName' => 'John', 'lastName' => 'Doe'], + ['firstName' => 'Jane', 'lastName' => 'Doe'], + ]); + } } diff --git a/src/batch-openspout/src/Reader/FlatFileReader.php b/src/batch-openspout/src/Reader/FlatFileReader.php index c07750cf..8ffe5816 100644 --- a/src/batch-openspout/src/Reader/FlatFileReader.php +++ b/src/batch-openspout/src/Reader/FlatFileReader.php @@ -59,12 +59,8 @@ public function read(): iterable $reader->open($path); foreach ($this->rows($reader) as $rowIndex => $row) { - if ($rowIndex === 1 && !$this->headerStrategy->setHeaders($row)) { - continue; - } - try { - yield $this->headerStrategy->getItem($row); + $item = $this->headerStrategy->process($row, $rowIndex === 1); } catch (InvalidRowSizeException $exception) { $this->jobExecution->addWarning( new Warning( @@ -78,7 +74,15 @@ public function read(): iterable ['headers' => $exception->getHeaders(), 'row' => $exception->getRow()], ), ); + + continue; } + + if ($item === null) { + continue; + } + + yield $item; } $reader->close(); @@ -89,7 +93,10 @@ public function read(): iterable */ private function rows(ReaderInterface $reader): Generator { - foreach ($this->sheetFilter->list($reader) as $sheet) { + foreach ($reader->getSheetIterator() as $sheet) { + if (!$this->sheetFilter->accepts($sheet)) { + continue; + } /** @var int $rowIndex */ /** @var Row $row */ foreach ($sheet->getRowIterator() as $rowIndex => $row) { diff --git a/src/batch-openspout/src/Reader/HeaderStrategy.php b/src/batch-openspout/src/Reader/HeaderStrategy.php index ea64b591..da0487c2 100644 --- a/src/batch-openspout/src/Reader/HeaderStrategy.php +++ b/src/batch-openspout/src/Reader/HeaderStrategy.php @@ -56,35 +56,32 @@ public static function none(array|null $headers = null): self } /** - * @param list $headers - * @internal - */ - public function setHeaders(array $headers): bool - { - if ($this->mode === self::NONE) { - return true; // row should be read, will be considered as an item - } - if ($this->mode === self::COMBINE) { - $this->headers = $headers; - } - - return false; // row should be skipped, will not be considered as an item - } - - /** - * Build the associative item, a combination of headers and values. - * - * @throws InvalidRowSizeException + * Process a row from the file. + * Returns null if the row should be skipped (e.g. it is a header row). + * Returns an array if the row should be yielded as an item. * * @param list $row * - * @return array|list - * @internal + * @return array|list|null + * + * @throws InvalidRowSizeException */ - public function getItem(array $row): array + public function process(array $row, bool $isFirstRow): array|null { + if ($isFirstRow) { + if ($this->mode === self::COMBINE) { + $this->headers = $row; + + return null; + } + if ($this->mode === self::SKIP) { + return null; + } + // NONE mode: fall through and treat first row as a regular item + } + if ($this->headers === null) { - return $row; // headers were not set, read row as is + return $row; } try { diff --git a/src/batch-openspout/src/Reader/SheetFilter.php b/src/batch-openspout/src/Reader/SheetFilter.php index e501debf..7f0b9ae2 100644 --- a/src/batch-openspout/src/Reader/SheetFilter.php +++ b/src/batch-openspout/src/Reader/SheetFilter.php @@ -5,8 +5,7 @@ namespace Yokai\Batch\Bridge\OpenSpout\Reader; use Closure; -use Generator; -use OpenSpout\Reader\ReaderInterface; +use OpenSpout\Reader\RowIteratorInterface; use OpenSpout\Reader\SheetInterface; /** @@ -54,17 +53,12 @@ public static function nameIs(string $name, string ...$names): self } /** - * Iterate over valid sheets for the provided filter. + * Whether the given sheet should be read. * - * @return Generator - * @internal + * @param SheetInterface $sheet */ - public function list(ReaderInterface $reader): Generator + public function accepts(SheetInterface $sheet): bool { - foreach ($reader->getSheetIterator() as $sheet) { - if (($this->accept)($sheet)) { - yield $sheet; - } - } + return ($this->accept)($sheet); } } diff --git a/src/batch-openspout/tests/Reader/HeaderStrategyTest.php b/src/batch-openspout/tests/Reader/HeaderStrategyTest.php index e03da0e0..05fae191 100644 --- a/src/batch-openspout/tests/Reader/HeaderStrategyTest.php +++ b/src/batch-openspout/tests/Reader/HeaderStrategyTest.php @@ -14,89 +14,86 @@ public function testNoneMode(): void { $strategy = HeaderStrategy::none(); - // In none mode: setHeaders returns true (row is an item) - self::assertTrue($strategy->setHeaders(['col1', 'col2'])); + // In none mode: first row is not skipped, it is treated as a regular item + self::assertSame(['col1', 'col2'], $strategy->process(['col1', 'col2'], true)); - // In none mode without predefined headers: getItem returns row as-is + // Subsequent rows are returned as-is $row = ['foo', 'bar']; - self::assertSame($row, $strategy->getItem($row)); + self::assertSame($row, $strategy->process($row, false)); } public function testNoneModeWithPredefinedHeaders(): void { $strategy = HeaderStrategy::none(['prenom', 'nom']); - // In none mode: setHeaders returns true (row is an item) - self::assertTrue($strategy->setHeaders(['ignored', 'headers'])); + // In none mode with predefined headers: first row is combined with those headers + self::assertSame(['prenom' => 'ignored', 'nom' => 'headers'], $strategy->process(['ignored', 'headers'], true)); - // In none mode with predefined headers: getItem combines headers with row - $row = ['John', 'Doe']; - self::assertSame(['prenom' => 'John', 'nom' => 'Doe'], $strategy->getItem($row)); + // Subsequent rows are also combined with the predefined headers + self::assertSame(['prenom' => 'John', 'nom' => 'Doe'], $strategy->process(['John', 'Doe'], false)); } public function testSkipMode(): void { $strategy = HeaderStrategy::skip(); - // In skip mode: setHeaders returns false (row should be skipped) - self::assertFalse($strategy->setHeaders(['firstName', 'lastName'])); + // In skip mode: first row is skipped (null returned) + self::assertNull($strategy->process(['firstName', 'lastName'], true)); - // In skip mode without predefined headers: getItem returns row as-is + // Subsequent rows are returned as-is (no headers to combine with) $row = ['John', 'Doe']; - self::assertSame($row, $strategy->getItem($row)); + self::assertSame($row, $strategy->process($row, false)); } public function testSkipModeWithPredefinedHeaders(): void { $strategy = HeaderStrategy::skip(['prenom', 'nom']); - // In skip mode: setHeaders returns false (row should be skipped) - self::assertFalse($strategy->setHeaders(['ignored', 'headers'])); + // In skip mode: first row is always skipped regardless of predefined headers + self::assertNull($strategy->process(['ignored', 'headers'], true)); - // In skip mode with predefined headers: getItem combines headers with row - $row = ['Jean', 'Dupont']; - self::assertSame(['prenom' => 'Jean', 'nom' => 'Dupont'], $strategy->getItem($row)); + // Subsequent rows are combined with the predefined headers + self::assertSame(['prenom' => 'Jean', 'nom' => 'Dupont'], $strategy->process(['Jean', 'Dupont'], false)); } public function testCombineMode(): void { $strategy = HeaderStrategy::combine(); - // In combine mode: setHeaders returns false (header row should be skipped) - self::assertFalse($strategy->setHeaders(['firstName', 'lastName'])); + // In combine mode: first row is skipped and stored as headers + self::assertNull($strategy->process(['firstName', 'lastName'], true)); - // In combine mode: getItem combines stored headers with row values - $row = ['John', 'Doe']; - self::assertSame(['firstName' => 'John', 'lastName' => 'Doe'], $strategy->getItem($row)); + // Subsequent rows are combined with the stored headers + self::assertSame(['firstName' => 'John', 'lastName' => 'Doe'], $strategy->process(['John', 'Doe'], false)); } public function testCombineModeConvertsHeadersToString(): void { $strategy = HeaderStrategy::combine(); - // Even with non-string header values, they get converted to string - $strategy->setHeaders([42, 3.14]); + // Non-string header values are converted to strings via array_combine + self::assertNull($strategy->process([42, 3.14], true)); $row = ['foo', 'bar']; - self::assertSame(['42' => 'foo', '3.14' => 'bar'], $strategy->getItem($row)); + self::assertSame(['42' => 'foo', '3.14' => 'bar'], $strategy->process($row, false)); } - public function testGetItemThrowsOnSizeMismatch(): void + public function testProcessThrowsOnSizeMismatch(): void { $strategy = HeaderStrategy::combine(); - $strategy->setHeaders(['firstName', 'lastName']); + $strategy->process(['firstName', 'lastName'], true); $this->expectException(InvalidRowSizeException::class); - $strategy->getItem(['OnlyOneValue']); + $strategy->process(['OnlyOneValue'], false); } - public function testGetItemSizeMismatchExceptionContainsData(): void + public function testProcessSizeMismatchExceptionContainsData(): void { $strategy = HeaderStrategy::combine(); - $strategy->setHeaders(['firstName', 'lastName']); + $strategy->process(['firstName', 'lastName'], true); try { - $strategy->getItem(['OnlyOne']); + $strategy->process(['OnlyOne'], false); self::fail('Expected exception was not thrown'); } catch (InvalidRowSizeException $exception) { self::assertSame(['firstName', 'lastName'], $exception->getHeaders()); diff --git a/src/batch-openspout/tests/Reader/SheetFilterTest.php b/src/batch-openspout/tests/Reader/SheetFilterTest.php index 54359663..9e226129 100644 --- a/src/batch-openspout/tests/Reader/SheetFilterTest.php +++ b/src/batch-openspout/tests/Reader/SheetFilterTest.php @@ -14,11 +14,7 @@ final class SheetFilterTest extends TestCase public function testAllAcceptsEverySheet(): void { - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - $sheets = \array_values(\iterator_to_array(SheetFilter::all()->list($reader))); - $reader->close(); + $sheets = $this->filterSheets(SheetFilter::all()); self::assertCount(2, $sheets); self::assertSame(0, $sheets[0]->getIndex()); @@ -27,11 +23,7 @@ public function testAllAcceptsEverySheet(): void public function testIndexIsFiltersToSingleIndex(): void { - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - $sheets = \array_values(\iterator_to_array(SheetFilter::indexIs(1)->list($reader))); - $reader->close(); + $sheets = $this->filterSheets(SheetFilter::indexIs(1)); self::assertCount(1, $sheets); self::assertSame(1, $sheets[0]->getIndex()); @@ -39,11 +31,7 @@ public function testIndexIsFiltersToSingleIndex(): void public function testIndexIsFiltersToMultipleIndexes(): void { - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - $sheets = \array_values(\iterator_to_array(SheetFilter::indexIs(0, 1)->list($reader))); - $reader->close(); + $sheets = $this->filterSheets(SheetFilter::indexIs(0, 1)); self::assertCount(2, $sheets); self::assertSame(0, $sheets[0]->getIndex()); @@ -52,11 +40,7 @@ public function testIndexIsFiltersToMultipleIndexes(): void public function testNameIsFiltersToMatchingSheet(): void { - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - $sheets = \array_values(\iterator_to_array(SheetFilter::nameIs('Français')->list($reader))); - $reader->close(); + $sheets = $this->filterSheets(SheetFilter::nameIs('Français')); self::assertCount(1, $sheets); self::assertSame('Français', $sheets[0]->getName()); @@ -64,32 +48,36 @@ public function testNameIsFiltersToMatchingSheet(): void public function testNameIsFiltersToMultipleNames(): void { - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - // Retrieve both sheet names to avoid hardcoding the first sheet's name - $allSheets = \array_values(\iterator_to_array(SheetFilter::all()->list($reader))); - $reader->close(); - + // Retrieve first sheet name dynamically to avoid hardcoding it + $allSheets = $this->filterSheets(SheetFilter::all()); $firstName = $allSheets[0]->getName(); - $reader = new XLSXReader(); - $reader->open(self::MULTI_TABS); - - $sheets = \array_values(\iterator_to_array(SheetFilter::nameIs($firstName, 'Français')->list($reader))); - $reader->close(); + $sheets = $this->filterSheets(SheetFilter::nameIs($firstName, 'Français')); self::assertCount(2, $sheets); } public function testFilterWithNoMatch(): void + { + $sheets = $this->filterSheets(SheetFilter::nameIs('NonExistent')); + + self::assertCount(0, $sheets); + } + + private function filterSheets(SheetFilter $filter): array { $reader = new XLSXReader(); $reader->open(self::MULTI_TABS); - $sheets = \iterator_to_array(SheetFilter::nameIs('NonExistent')->list($reader)); + $sheets = []; + foreach ($reader->getSheetIterator() as $sheet) { + if ($filter->accepts($sheet)) { + $sheets[] = $sheet; + } + } + $reader->close(); - self::assertCount(0, $sheets); + return $sheets; } } diff --git a/src/batch-symfony-console/src/CommandRunner.php b/src/batch-symfony-console/src/CommandRunner.php index fd78e1b7..6f1720c7 100644 --- a/src/batch-symfony-console/src/CommandRunner.php +++ b/src/batch-symfony-console/src/CommandRunner.php @@ -9,25 +9,25 @@ /** * Utility class that knows how to run command asynchronously. - * - * @internal Please do not use in your project. - * @final But only for tests mock. */ -class CommandRunner +final class CommandRunner implements CommandRunnerInterface { private readonly string $consolePath; private readonly null|PhpExecutableFinder $phpLocator; + private readonly ShellRunnerInterface $shellRunner; public function __construct( string $binDir, private readonly string $logDir, PhpExecutableFinder|null $phpLocator = null, + ShellRunnerInterface|null $shellRunner = null, ) { - $this->consolePath = \implode(DIRECTORY_SEPARATOR, [$binDir, 'console']); + $this->consolePath = \implode(\DIRECTORY_SEPARATOR, [$binDir, 'console']); if (\class_exists(PhpExecutableFinder::class)) { $phpLocator ??= new PhpExecutableFinder(); } $this->phpLocator = $phpLocator; + $this->shellRunner = $shellRunner ?? new ShellRunner(); } /** @@ -37,7 +37,7 @@ public function __construct( */ public function runAsync(string $commandName, string $logFilename, array $arguments = []): void { - $this->exec( + $this->shellRunner->exec( \sprintf( '%s >> %s 2>&1 &', $this->buildCommand($commandName, $arguments), @@ -46,14 +46,6 @@ public function runAsync(string $commandName, string $logFilename, array $argume ); } - /** - * @codeCoverageIgnore - */ - protected function exec(string $command): void - { - \exec($command); - } - /** * @param array $arguments */ diff --git a/src/batch-symfony-console/src/CommandRunnerInterface.php b/src/batch-symfony-console/src/CommandRunnerInterface.php new file mode 100644 index 00000000..ae74a18f --- /dev/null +++ b/src/batch-symfony-console/src/CommandRunnerInterface.php @@ -0,0 +1,18 @@ + $arguments + */ + public function runAsync(string $commandName, string $logFilename, array $arguments = []): void; +} diff --git a/src/batch-symfony-console/src/RunCommandJobLauncher.php b/src/batch-symfony-console/src/RunCommandJobLauncher.php index c07b0213..cd12cfa3 100644 --- a/src/batch-symfony-console/src/RunCommandJobLauncher.php +++ b/src/batch-symfony-console/src/RunCommandJobLauncher.php @@ -22,7 +22,7 @@ { public function __construct( private JobExecutionFactory $jobExecutionFactory, - private CommandRunner $commandRunner, + private CommandRunnerInterface $commandRunner, private JobExecutionStorageInterface $jobExecutionStorage, private string $logFilename = 'batch_execute.log', ) { diff --git a/src/batch-symfony-console/src/ShellRunner.php b/src/batch-symfony-console/src/ShellRunner.php new file mode 100644 index 00000000..877addca --- /dev/null +++ b/src/batch-symfony-console/src/ShellRunner.php @@ -0,0 +1,16 @@ +createMock(ShellRunnerInterface::class); + $phpLocator = $this->createStub(PhpExecutableFinder::class); $phpLocator->method('find')->willReturn('/usr/bin/php'); - return $this->getMockBuilder(CommandRunner::class) - ->onlyMethods(['exec']) - ->setConstructorArgs(['/path/to/bin', '/path/to/logs', $phpLocator]) - ->getMock(); + $runner = new CommandRunner('/path/to/bin', '/path/to/logs', $phpLocator, $shellRunner); + + return [$runner, $shellRunner]; } public function testRunAsync(): void { - $runner = $this->createRunner(); - $runner->expects($this->once()) + [$runner, $shellRunner] = $this->createRunner(); + + $shellRunner->expects($this->once()) ->method('exec') ->with( '/usr/bin/php /path/to/bin/console yokai:testing:test 1 ' . diff --git a/src/batch-symfony-console/tests/RunCommandJobLauncherTest.php b/src/batch-symfony-console/tests/RunCommandJobLauncherTest.php index 94c9d5d2..3834bfc7 100644 --- a/src/batch-symfony-console/tests/RunCommandJobLauncherTest.php +++ b/src/batch-symfony-console/tests/RunCommandJobLauncherTest.php @@ -7,7 +7,7 @@ use PHPUnit\Framework\TestCase; use PHPUnit\Framework\MockObject\MockObject; use Yokai\Batch\BatchStatus; -use Yokai\Batch\Bridge\Symfony\Console\CommandRunner; +use Yokai\Batch\Bridge\Symfony\Console\CommandRunnerInterface; use Yokai\Batch\Bridge\Symfony\Console\RunCommandJobLauncher; use Yokai\Batch\Factory\JobExecutionFactory; use Yokai\Batch\Factory\JobExecutionLoggerFactory\InMemoryJobExecutionLoggerFactory; @@ -22,8 +22,8 @@ public function testLaunch(): void $config = ['_id' => '123456789', 'foo' => ['bar']]; $arguments = ['job' => 'testing', 'configuration' => '{"_id":"123456789","foo":["bar"]}']; - /** @var MockObject&CommandRunner $commandRunner */ - $commandRunner = $this->createMock(CommandRunner::class); + /** @var MockObject&CommandRunnerInterface $commandRunner */ + $commandRunner = $this->createMock(CommandRunnerInterface::class); $commandRunner->expects($this->once()) ->method('runAsync') ->with('yokai:batch:run', 'test.log', $arguments); diff --git a/src/batch-symfony-console/tests/ShellRunnerTest.php b/src/batch-symfony-console/tests/ShellRunnerTest.php new file mode 100644 index 00000000..3828d2bd --- /dev/null +++ b/src/batch-symfony-console/tests/ShellRunnerTest.php @@ -0,0 +1,27 @@ +exec(\sprintf('echo hello > %s', \escapeshellarg($outputFile))); + + self::assertFileExists($outputFile); + self::assertStringContainsString('hello', \file_get_contents($outputFile)); + } finally { + if (\file_exists($outputFile)) { + \unlink($outputFile); + } + } + } +}