Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
30 changes: 21 additions & 9 deletions src/Admin/Filter/ProcessExecutionDurationFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
43 changes: 35 additions & 8 deletions tests/Admin/Filter/ProcessExecutionDurationFilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, array{int|float|string|null, string}>
*/
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'];
}

/**
Expand All @@ -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
Expand Down
Loading