diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8f97dd3..88cd07b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,6 +29,12 @@ jobs: symfony-require: [''] variant: [normal] include: + # Lowest versions allowed by composer.json, on the lowest supported PHP version + - php-version: '8.2' + dependencies: lowest + allowed-to-fail: false + symfony-require: '' + variant: normal - php-version: '8.2' dependencies: highest allowed-to-fail: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dacf99..749e4d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#158](https://github.com/cleverage/ui-process-bundle/issues/158) CI: test the lowest versions allowed by `composer.json` (`--prefer-lowest`, PHP 8.2). Require `symfony/scheduler` ^6.4.44 || ^7.4.17 || ^8.1.5: the `every` expressions validation (`EveryExpressionValidator`) relies on the `PeriodicalTrigger` rejecting the intervals that do not move the run date forward or resolve to zero seconds (older versions loop forever on e.g. "yesterday", and fail with a `TypeError` on an invalid expression). `ProcessExecutionDurationFilter`: the number of seconds is cast to an integer and inlined in the DQL instead of being bound as a parameter (#111): with DBAL 3, the SQLite platform puts the `SECOND` intervals inside a string literal, so the parameter was never bound ("column index out of range" error on SQLite with DBAL 3). * [#156](https://github.com/cleverage/ui-process-bundle/issues/156) `LogRecord::contextIsEmpty()` (deprecated since v3.1, use `hasContextInfo()`) triggers a deprecation at runtime with `trigger_deprecation()` (`symfony/deprecation-contracts`, now required). Add tests. * [#154](https://github.com/cleverage/ui-process-bundle/issues/154) Add a `.gitattributes`: the development files (`.docker/`, `.github/`, `tests/`, quality tools and PHPUnit configurations, `Makefile`) are no longer part of the archive installed by Composer (`dist`), remove the unused `package.json` (Webpack Encore leftover of the Symfony skeleton: no `webpack.config.js`, no `assets/`). * [#149](https://github.com/cleverage/ui-process-bundle/issues/149) Clean up composer.json: remove the unused `symfony/dotenv`, `symfony/runtime`, `ext-ctype` and `ext-iconv` requirements (applications relying on the bundle to install `symfony/dotenv` / `symfony/runtime` must require them, as the Symfony skeleton does); replace `doctrine/common` by `doctrine/collections` (only `ArrayCollection` is used); require `twig/twig`, `symfony/twig-bundle`, `psr/log`, `doctrine/persistence`, `doctrine/migrations` and `symfony/event-dispatcher-contracts`, used directly but only installed transitively; add `twig/extra-bundle` and `symfony/property-access` (used by the tests) to the dev requirements. diff --git a/composer.json b/composer.json index 570028c..2abd4aa 100644 --- a/composer.json +++ b/composer.json @@ -69,7 +69,7 @@ "symfony/password-hasher": "^6.4 || ^7.4 || ^8", "symfony/process": "^6.4 || ^7.4 || ^8", "symfony/routing": "^6.4 || ^7.4 || ^8", - "symfony/scheduler": "^6.4 || ^7.4 || ^8", + "symfony/scheduler": "^6.4.44 || ^7.4.17 || ^8.1.5", "symfony/security-bundle": "^6.4 || ^7.4 || ^8", "symfony/security-core": "^6.4 || ^7.4 || ^8", "symfony/security-http": "^6.4 || ^7.4 || ^8", diff --git a/src/Admin/Filter/ProcessExecutionDurationFilter.php b/src/Admin/Filter/ProcessExecutionDurationFilter.php index 06acf11..e562947 100644 --- a/src/Admin/Filter/ProcessExecutionDurationFilter.php +++ b/src/Admin/Filter/ProcessExecutionDurationFilter.php @@ -37,23 +37,35 @@ public static function new(string $propertyName, ?string $label = null): self public function apply(QueryBuilder $queryBuilder, FilterDataDto $filterDataDto, ?FieldDto $fieldDto, EntityDto $entityDto): void { - $parameter = $filterDataDto->getParameterName(); if (\in_array($filterDataDto->getComparison(), ['=', '>', '>=', '<', '<=', '!='], true)) { $queryBuilder->andWhere( - \sprintf('entity.endDate %s date_add(entity.startDate, :%s, \'SECOND\')', $filterDataDto->getComparison(), $parameter) + \sprintf( + 'entity.endDate %s date_add(entity.startDate, %s, \'SECOND\')', + $filterDataDto->getComparison(), + $this->toSeconds($filterDataDto->getValue()) + ) ); - $queryBuilder->setParameter($parameter, $filterDataDto->getValue()); } elseif ('between' === $filterDataDto->getComparison()) { - $parameter2 = $filterDataDto->getParameter2Name(); $queryBuilder->andWhere( \sprintf( - 'entity.endDate BETWEEN date_add(entity.startDate, :%s, \'SECOND\') and date_add(entity.startDate, :%s, \'SECOND\')', - $parameter, - $parameter2 + 'entity.endDate BETWEEN date_add(entity.startDate, %s, \'SECOND\') and date_add(entity.startDate, %s, \'SECOND\')', + $this->toSeconds($filterDataDto->getValue()), + $this->toSeconds($filterDataDto->getValue2()) ) ); - $queryBuilder->setParameter($parameter, $filterDataDto->getValue()); - $queryBuilder->setParameter($parameter2, $filterDataDto->getValue2()); } } + + /** + * The number of seconds is cast to an integer and inlined in the DQL instead of being bound as a parameter: with + * DBAL 3, SqlitePlatform generates the SECOND intervals inside a string literal (DATETIME(date, '+? SECOND')), so + * a bound parameter is never bound ("column index out of range"). + */ + private function toSeconds(mixed $value): string + { + $seconds = is_numeric($value) ? (int) $value : 0; + + // A negative number is not a DQL literal + return $seconds < 0 ? \sprintf('(%d)', $seconds) : (string) $seconds; + } } diff --git a/tests/Admin/Filter/ProcessExecutionDurationFilterTest.php b/tests/Admin/Filter/ProcessExecutionDurationFilterTest.php index 491e1bc..66ea2d1 100644 --- a/tests/Admin/Filter/ProcessExecutionDurationFilterTest.php +++ b/tests/Admin/Filter/ProcessExecutionDurationFilterTest.php @@ -47,12 +47,41 @@ public function testApplyWithComparison(string $comparison): void self::assertSame( 'SELECT entity FROM '.ProcessExecution::class.' entity WHERE entity.endDate '.$comparison - .' date_add(entity.startDate, :duration_0, \'SECOND\')', + .' date_add(entity.startDate, 60, \'SECOND\')', $queryBuilder->getDQL() ); - // Bound as a parameter, not concatenated in the DQL - self::assertCount(1, $queryBuilder->getParameters()); - self::assertSame(60, $queryBuilder->getParameter('duration_0')?->getValue()); + self::assertCount(0, $queryBuilder->getParameters()); + } + + /** + * The value is cast to an integer before being inlined in the DQL (not bound: DBAL 3 SqlitePlatform does not bind + * the SECOND intervals). + * + * @param int|float|string|null $value + */ + #[DataProvider('provideValues')] + public function testValueIsCastToAnInteger(mixed $value, string $expected): void + { + $queryBuilder = $this->apply(['comparison' => ComparisonType::GT, 'value' => $value]); + + self::assertSame( + 'SELECT entity FROM '.ProcessExecution::class.' entity WHERE entity.endDate > ' + .'date_add(entity.startDate, '.$expected.', \'SECOND\')', + $queryBuilder->getDQL() + ); + } + + /** + * @return iterable + */ + public static function provideValues(): iterable + { + yield 'integer' => [3600, '3600']; + yield 'float' => [1.9, '1']; + yield 'numeric string' => ['42', '42']; + yield 'negative' => [-5, '(-5)']; + yield 'not numeric' => ["1') OR 1=1 OR ('", '0']; + yield 'null' => [null, '0']; } /** @@ -74,12 +103,10 @@ public function testApplyBetween(): void self::assertSame( 'SELECT entity FROM '.ProcessExecution::class.' entity WHERE entity.endDate BETWEEN ' - .'date_add(entity.startDate, :duration_0, \'SECOND\') and date_add(entity.startDate, :duration_1, \'SECOND\')', + .'date_add(entity.startDate, 10, \'SECOND\') and date_add(entity.startDate, 120, \'SECOND\')', $queryBuilder->getDQL() ); - self::assertCount(2, $queryBuilder->getParameters()); - self::assertSame(10, $queryBuilder->getParameter('duration_0')?->getValue()); - self::assertSame(120, $queryBuilder->getParameter('duration_1')?->getValue()); + self::assertCount(0, $queryBuilder->getParameters()); } public function testApplyWithUnsupportedComparisonDoesNothing(): void