Added support for Symfony 6. - #32
Conversation
|
@mSprunskas please check. The library is deprecated but still used in multiple apps so I added support for Symfony 6 and fixed tests issues. |
mSprunskas
left a comment
There was a problem hiding this comment.
Other findings, could be irrelevant
Initial Findings
Issue 1: CRITICAL - CI YAML Wrong Matrix Variable Name
File: .github/workflows/ci.yaml:37
Issue: Uses ${{ matrix.php-versions }} but the matrix variable is defined as php (line 22), not php-versions.
php-version: ${{ matrix.php-versions }} # WRONG - should be matrix.php
Impact: The PHP version won't be set correctly. The shivammathur/setup-php action will receive an undefined/empty value, causing tests to run with the system default PHP instead of the matrix-specified versions.
---
Issue 2: MEDIUM - UsernamePasswordToken Incompatible with Symfony 5.3
File: Tests/Security/RoleAndIpStrategyTest.php:44
Issue: The test uses 3-argument constructor signature:
$token = new UsernamePasswordToken($user, 'firewall', ['ROLE_ADMIN', 'ROLE_FEATURE_MANAGER']);
This signature works on Symfony 5.4+ and 6.x, but in https://github.com/symfony/security-core/blob/5.3/Authentication/Token/UsernamePasswordToken.php:
public function __construct($user, $credentials, string $firewallName, array $roles = [])
Since InMemoryUser exists in Symfony 5.3+ (triggering this code path), the test will fail on Symfony 5.3 with a TypeError (array passed as $firewallName where string expected).
Impact: Test failure on Symfony 5.3. The comment says "Symfony 5.x, 6.x" but only works for Symfony 5.4+/6.x.
---
Issue 3: LOW - CI Matrix Missing PHP 8.3
File: .github/workflows/ci.yaml:22-27
Issue: PHP versions jump from 8.2 to 8.4, skipping 8.3:
php:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
- '8.4' # Missing 8.3
Impact: No test coverage for PHP 8.3.
mSprunskas
left a comment
There was a problem hiding this comment.
Some issues
Issue 1 — CRITICAL: CI symfony matrix variable is defined but never used
File: .github/workflows/ci.yaml:29-32
The symfony matrix dimension defines three values (4.*, 5.*, 6.*), creating a 3× multiplier on jobs. However, no step references ${{ matrix.symfony }}:
- The ramsey/composer-install@v2 step (line 57-59) only uses dependency-versions — it does not pin a Symfony version.
- The job name (line 45) does not include ${{ matrix.symfony }}, resulting in duplicate, indistinguishable job names (confirmed from the https://github.com/paysera/lib-rest-bundle/actions/runs/21752974891 — multiple jobs have the exact same name like "PHP
8.4 with highest dependencies tests on ubuntu-latest").
Impact: The CI runs 32 jobs but only 12 are unique — 20 jobs are exact duplicates. The exclude rules for Symfony 6 + PHP 7.4/8.0 (lines 39-43) are also functionally meaningless. Which Symfony version actually gets installed depends entirely on dependency:
lowest (resolves to Symfony 4.4.50) vs dependency: highest (resolves to latest Symfony 6.x on PHP 8.1+), not the symfony matrix variable.
Fix: Add a step to pin the Symfony version before installing dependencies, e.g.:
- name: Require Symfony ${{ matrix.symfony }}
run: |
composer require --no-update \
"symfony/config:${{ matrix.symfony }}" \
"symfony/dependency-injection:${{ matrix.symfony }}" \
"symfony/validator:${{ matrix.symfony }}" \
"symfony/security-core:${{ matrix.symfony }}" \
"symfony/routing:${{ matrix.symfony }}" \
"symfony/http-foundation:${{ matrix.symfony }}" \
"symfony/http-kernel:${{ matrix.symfony }}"
And include ${{ matrix.symfony }} in the job name.
---
Issue 2 — MINOR: Inconsistent YAML indentation in CI matrix
File: .github/workflows/ci.yaml:20-35
The operating-system and php list items use 2-space relative indentation (10 total), while the newly added symfony, dependency, and exclude blocks use 4-space relative indentation (12 total):
php: # 8-space key indent
- '7.4' # 10-space item (2 relative) — existing
symfony: # 8-space key indent
- '4.*' # 12-space item (4 relative) — new
dependency: # 8-space key indent
- 'lowest' # 12-space item (4 relative) — new
This is valid YAML but inconsistent style within the same block.
---
Issue 3 — MINOR: CHANGELOG entry for 5.2.0 is incomplete
File: CHANGELOG.md:3-6
The 5.2.0 entry only mentions:
- Support for Symfony 6.x
But the changeset includes several other notable changes not documented:
- CI fix: corrected ${{ matrix.php-versions }} → ${{ matrix.php }} (was a bug causing PHP version to not be set)
- CI expansion: added PHP 8.3 and 8.4 to the test matrix
- ApiException.php: fixed implicit nullable type deprecation for PHP 8.4 compatibility
- composer.json: narrowed Symfony constraints from ^4.0 || ^5.0 to ^4.4.50 || ^5.4.19 || ^6.0.19 (drops support for Symfony 4.0-4.4.49 and 5.0-5.4.18)
- composer.json: bumped phpunit/phpunit from ^8.0 to ^9.5 and mockery/mockery from ^1.2 to ^1.4
- Test refactoring for Symfony 5.4+/6.x compatibility
The constraint narrowing in particular could affect downstream consumers and should be documented.
the most interesting one is that depdencies are inconsistent in jobs, e.g. https://productionresultssa15.blob.core.windows.net/actions-results/7eab9828-de33-4a81-aecc-60e0efbc1884/workflow-job-run-708832ff-3ea5-5c80-81ef-a2b48aa88220/logs/job/job-logs.txt?rsct=text%2Fplain&se=2026-02-06T14%3A12%3A29Z&sig=ctxP8uUJg%2BxSjAp8BsdGHW4tFGoSE%2Fo3uCjWsCoZ7k4%3D&ske=2026-02-06T17%3A52%3A54Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2026-02-06T13%3A52%3A54Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-11-05&sp=r&spr=https&sr=b&st=2026-02-06T14%3A02%3A24Z&sv=2025-11-05 has two different symfony/http-* lib versions
❯ why different major versions exist in https://productionresultssa15.blob.core.windows.net/actions-results/7eab9828-de33-4a81-aecc-60e0efbc1884/workflow-job-run-708832ff-3ea5-5c80-81ef-a2b48aa88220/logs/job/job-logs.txt?rsct=text%2Fplain&se=2026-02-06T14
%3A12%3A29Z&sig=ctxP8uUJg%2BxSjAp8BsdGHW4tFGoSE%2Fo3uCjWsCoZ7k4%3D&ske=2026-02-06T17%3A52%3A54Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2026-02-06T13%3A52%3A54Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-11-05&sp=r&spr=https&sr=b&s
t=2026-02-06T14%3A02%3A24Z&sv=2025-11-05 for symfony/http- packages?
● Fetch(https://productionresultssa15.blob.core.windows.net/actions-results/7eab9828-de33-4a81-aecc-60e0efbc1884/workflow-job-run-708832ff-3ea5-5c80-81ef-a2b48aa88220/logs/job/job-logs.txt?rsct=text%2Fplain&se=2026-02-06T14%3A12%3A29Z&sig=ctxP8uUJg%2BxSjAp8Bsd
GHW4tFGoSE%2Fo3uCjWsCoZ7k4%3D&ske=2026-02-06T17%3A52%3A54Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2026-02-06T13%3A52%3A54Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-11-05&sp=r&spr=https&sr=b&st=2026-02-06T14%3A02%3A24Z&sv=2025-11
-05)
⎿ Received 34.9KB (200 OK)
● This is a great catch and directly related to Issue #1 from the review. The log shows a problematic mix:
┌─────────────────────────┬──────────────────┐
│ Package │ Resolved Version │
├─────────────────────────┼──────────────────┤
│ symfony/http-kernel │ v4.4.50 │
├─────────────────────────┼──────────────────┤
│ symfony/http-foundation │ v5.4.50 │
├─────────────────────────┼──────────────────┤
│ symfony/config │ v5.4.19 │
├─────────────────────────┼──────────────────┤
│ symfony/security-core │ v5.4.19 │
├─────────────────────────┼──────────────────┤
│ symfony/routing │ v5.4.19 │
├─────────────────────────┼──────────────────┤
│ symfony/console │ v3.4.0 │
├─────────────────────────┼──────────────────┤
│ symfony/property-access │ v3.0.0 │
└─────────────────────────┴──────────────────┘
Components span Symfony 3.x, 4.x, and 5.x in a single install. That happens because of how the constraint ^4.4.50 || ^5.4.19 || ^6.0.19 interacts with --prefer-lowest:
1. The || constraint treats each package independently. Composer doesn't enforce "all Symfony packages must be at the same major version." Each package resolves to its own lowest satisfying version.
2. Transitive dependencies force some packages into higher ranges. For example, symfony/security-core v5.4.19 likely requires symfony/http-foundation ^5.4, which pushes http-foundation up to 5.x. But symfony/http-kernel has no such transitive pressure, so it
stays at 4.4.50.
3. Indirect dependencies aren't constrained at all. Packages like symfony/console (v3.4.0) and symfony/property-access (v3.0.0) are pulled in as transitive dependencies of other packages and aren't listed in composer.json — so they resolve to whatever their
parent requires as a minimum.
This is the practical consequence of the symfony matrix variable not being used (Issue #1). The CI appears to test "Symfony 4, 5, and 6" but actually tests a Frankenstein mix of versions that no real application would ever run. It passes, but it doesn't
validate what it claims to validate.
To properly pin Symfony versions, a step before composer install should constrain all Symfony packages to the same major version. This also requires adding the indirect Symfony dependencies (or using Symfony Flex's SYMFONY_REQUIRE env var) to prevent version
mixing.
7f14796 to
7372ce4
Compare
7372ce4 to
8c4bdc0
Compare
|
@mSprunskas ok I made few more changes. Asked Claude to review PR, no major issues except the version and insecure libs for composer. |
No description provided.