From 22ca186c2a1729443177c37edcdbdfb04208944d Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:43:23 +0530 Subject: [PATCH 01/42] EE-283 Allow Symfony 7.4 and require every Symfony component the bundle uses framework-bundle, security-bundle and validator (and yaml in require-dev) now allow ^7.4. The code also uses config, dependency-injection, http-foundation, http-kernel, routing, security-core and property-access directly, but composer.json did not name them. They arrived through other packages with no limit, so on PHP 8.3 an install next to the 6.4 bundles took routing, http-foundation and security-core 7.4, and 144 of the 259 tests failed. They are now required at the same constraint as the bundles. psr/log also allows ^3.0. The doctrine/orm dev floor moves to 2.6.3: with the lowest dependencies on PHP 7.4, 2.6.0 failed 140 tests with "continue targeting switch is equivalent to break". Co-Authored-By: Claude Opus 5.5 (1M context) --- composer.json | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index f80e408..cbcd5c2 100644 --- a/composer.json +++ b/composer.json @@ -17,24 +17,31 @@ "require": { "php": "^7.1 || ^8.0", "ext-json": "*", - "symfony/framework-bundle": "^3.4.34|^4.3|^5.4|^6.0", - "symfony/security-bundle": "^3.4.34|^4.3|^5.4|^6.0", - "symfony/validator": "^3.4.34|^4.3|^5.4|^6.0", + "symfony/config": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/dependency-injection": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/framework-bundle": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/http-foundation": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/http-kernel": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/property-access": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/routing": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/security-bundle": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/security-core": "^3.4.34|^4.3|^5.4|^6.0|^7.4", + "symfony/validator": "^3.4.34|^4.3|^5.4|^6.0|^7.4", "paysera/lib-normalization-bundle": "^1.1.0", "paysera/lib-normalization": "^1.2", "paysera/lib-object-wrapper": "~0.1", "paysera/lib-pagination": "^1.0", "paysera/lib-dependency-injection": "^1.3.0", - "psr/log": "^1.0|^2.0", + "psr/log": "^1.0|^2.0|^3.0", "doctrine/persistence": "^1.3.8 || ^2.0.1 || ^3.0", "doctrine/annotations": "^1.14 || ^2.0" }, "require-dev": { "phpunit/phpunit": "^7.5 || ^9.6", "mockery/mockery": "^1.3.6", - "symfony/yaml": "^3.4.34|^4.3|^5.4|^6.0", + "symfony/yaml": "^3.4.34|^4.3|^5.4|^6.0|^7.4", "doctrine/doctrine-bundle": "^1.12.0|^2.1", - "doctrine/orm": "^2.5.14" + "doctrine/orm": "^2.6.3" }, "config": { "bin-dir": "bin" From 50f98a2d4970e1bc01d490a114e4ed6fd678a59f Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:43:36 +0530 Subject: [PATCH 02/42] EE-283 Run the workflow on Symfony 7 The Symfony axis gains 7.*, excluded on PHP 7.1 to 8.1 because Symfony 7 needs PHP 8.2, so PHP 8.2 and 8.3 each get one Symfony 7 job with the highest dependencies. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8e5582..7b80428 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,7 @@ jobs: - '4.*' - '5.*' - '6.*' + - '7.*' dependency: - 'highest' include: @@ -46,6 +47,12 @@ jobs: - { php: '7.3', symfony: '6.*' } - { php: '7.4', symfony: '6.*' } - { php: '8.0', symfony: '6.*' } + - { php: '7.1', symfony: '7.*' } + - { php: '7.2', symfony: '7.*' } + - { php: '7.3', symfony: '7.*' } + - { php: '7.4', symfony: '7.*' } + - { php: '8.0', symfony: '7.*' } + - { php: '8.1', symfony: '7.*' } steps: - name: Checkout From 95571729fbf14c85ba561a448bee3ad423e5ff7b Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:46:29 +0530 Subject: [PATCH 03/42] EE-283 Let the test application run on Symfony 7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symfony 7 reads no Doctrine annotations and dropped two settings the test application used, so on 7.4 the suite could not boot. Each line now gets what it accepts: - The fixture entities are mapped in Doctrine XML (one mapping for every Doctrine version) instead of @ORM docblocks. - The routes defined in XML move to explicit_routing.xml. Symfony 7 imports that file and the attribute controllers only (sf7_routing.yml); the two docblock-routed controllers that other tests rely on get attribute twins with the same paths. The docblock half of FunctionalAnnotationsTest is skipped on Symfony 7. - The Symfony 7 security settings omit enable_authenticator_manager (removed in 7.0). - The fixture bundle's Configuration declares its TreeBuilder return type. - HttpKernelHelper looked for the constant "…HttpKernelInterfaceMAIN_REQUEST" (no "::"), so it always fell back to MASTER_REQUEST, which Symfony 7 removed. - EntityValidatorTest no longer marks three cases as asserting nothing: they expect the validator to be called once. With PHPUnit 7.5 and Mockery 1.3 those cases were reported as risky because Mockery counted its expectations as assertions. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../AttributedPagedQueryController.php | 38 ++++++++++++++ .../AttributedPersistedEntityController.php | 31 ++++++++++++ .../DependencyInjection/Configuration.php | 5 +- .../Entity/PersistedEntity.php | 9 ---- .../Entity/SimplePersistedEntity.php | 8 --- .../config/doctrine/PersistedEntity.orm.xml | 12 +++++ .../doctrine/SimplePersistedEntity.orm.xml | 10 ++++ .../Resources/config/explicit_routing.xml | 49 +++++++++++++++++++ .../Resources/config/routing.xml | 43 +--------------- .../Resources/config/services.xml | 4 ++ .../FixtureTestBundle/Service/TestHelper.php | 9 ++++ tests/Functional/Fixtures/TestKernel.php | 3 +- .../Functional/Fixtures/config/sf7_common.yml | 15 ++++++ .../Fixtures/config/sf7_routing.yml | 9 ++++ .../Functional/FunctionalAnnotationsTest.php | 3 ++ tests/Functional/FunctionalTestCase.php | 7 ++- tests/Unit/Helper/HttpKernelHelper.php | 2 +- .../Validation/EntityValidatorTest.php | 2 +- 18 files changed, 192 insertions(+), 67 deletions(-) create mode 100644 tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php create mode 100644 tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php create mode 100644 tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml create mode 100644 tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml create mode 100644 tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.xml create mode 100644 tests/Functional/Fixtures/config/sf7_common.yml create mode 100644 tests/Functional/Fixtures/config/sf7_routing.yml diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php new file mode 100644 index 0000000..2e69312 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php @@ -0,0 +1,38 @@ +entityManager = $entityManager; + } + + #[Route(path: '/paged-query/simple', methods: ['GET'])] + #[Query(parameterName: 'pager')] + #[Query(parameterName: 'filter')] + public function findSimplePersistedEntities(Pager $pager, PersistedEntityFilter $filter): PagedQuery + { + /** @var PersistedEntityRepository $repository */ + $repository = $this->entityManager->getRepository(PersistedEntity::class); + $configuredQuery = $repository->buildConfiguredQuery($filter); + return new PagedQuery($configuredQuery, $pager); + } +} diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php new file mode 100644 index 0000000..ed79485 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php @@ -0,0 +1,31 @@ +getId()); + } + + #[Route(path: '/simple-persisted-entities/{identifier}', methods: ['GET'])] + #[PathAttribute(parameterName: 'entity', pathPartName: 'identifier')] + public function findSimplePersistedEntity(SimplePersistedEntity $entity): Response + { + return new Response((string)$entity->getId()); + } +} diff --git a/tests/Functional/Fixtures/FixtureTestBundle/DependencyInjection/Configuration.php b/tests/Functional/Fixtures/FixtureTestBundle/DependencyInjection/Configuration.php index 31ced6a..87392e8 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/DependencyInjection/Configuration.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/DependencyInjection/Configuration.php @@ -13,10 +13,7 @@ */ class Configuration implements ConfigurationInterface { - /** - * {@inheritdoc} - */ - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('paysera_fixture_test'); diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Entity/PersistedEntity.php b/tests/Functional/Fixtures/FixtureTestBundle/Entity/PersistedEntity.php index 5864530..3246bbf 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Entity/PersistedEntity.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Entity/PersistedEntity.php @@ -3,24 +3,15 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Entity; -use Doctrine\ORM\Mapping as ORM; - -/** - * @ORM\Entity(repositoryClass="Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Repository\PersistedEntityRepository") - */ class PersistedEntity { /** - * @ORM\Id - * @ORM\GeneratedValue(strategy="NONE") - * @ORM\Column(type="integer") * @var int|null */ private $id; /** * @var string|null - * @ORM\Column(type="string") */ private $someField; diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Entity/SimplePersistedEntity.php b/tests/Functional/Fixtures/FixtureTestBundle/Entity/SimplePersistedEntity.php index 81a8437..2bcbf96 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Entity/SimplePersistedEntity.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Entity/SimplePersistedEntity.php @@ -3,17 +3,9 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Entity; -use Doctrine\ORM\Mapping as ORM; - -/** - * @ORM\Entity - */ class SimplePersistedEntity { /** - * @ORM\Id - * @ORM\GeneratedValue(strategy="NONE") - * @ORM\Column(type="integer") * @var int|null */ private $id; diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml new file mode 100644 index 0000000..9587ae8 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml new file mode 100644 index 0000000..e3b92e0 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml @@ -0,0 +1,10 @@ + + + + + + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.xml new file mode 100644 index 0000000..64b6faf --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.xml @@ -0,0 +1,49 @@ + + + + + + Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1Action + + + + Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1bAction + + + + + Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action2 + + + + paysera_fixture_test.controller.default_controller::action3 + + + paysera_fixture_test.controller.default_controller::action4 + + + + Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action5 + + + + + paysera_fixture_test.controller.default_controller::action + + + paysera_fixture_test.controller.default_controller::actionWithReturn + + + + Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action + + + + paysera_fixture_test.controller.default_controller::actionWithMultipleParameters + + + paysera_fixture_test.controller.default_controller::actionWithMultipleParameters + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml index 3a06c43..3118327 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml @@ -11,46 +11,5 @@ - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1Action - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1bAction - - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action2 - - - - paysera_fixture_test.controller.default_controller::action3 - - - paysera_fixture_test.controller.default_controller::action4 - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action5 - - - - - paysera_fixture_test.controller.default_controller::action - - - paysera_fixture_test.controller.default_controller::actionWithReturn - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action - - - - paysera_fixture_test.controller.default_controller::actionWithMultipleParameters - - - paysera_fixture_test.controller.default_controller::actionWithMultipleParameters - + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml index 46836bd..24833de 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml @@ -92,6 +92,10 @@ public="true"> + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php index a3133ff..abaf8f6 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php @@ -5,6 +5,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; +use Symfony\Component\HttpKernel\Kernel; class TestHelper { @@ -12,4 +13,12 @@ public static function phpAttributeSupportExists(): bool { return class_exists(AttributeRouteControllerLoader::class); } + + /** + * Symfony 7 removed the Doctrine annotation reader from routing: @Route docblocks are no longer read. + */ + public static function docblockRoutingSupportExists(): bool + { + return Kernel::MAJOR_VERSION < 7; + } } diff --git a/tests/Functional/Fixtures/TestKernel.php b/tests/Functional/Fixtures/TestKernel.php index 9fc2a51..bb7f618 100644 --- a/tests/Functional/Fixtures/TestKernel.php +++ b/tests/Functional/Fixtures/TestKernel.php @@ -43,7 +43,8 @@ public function registerContainerConfiguration(LoaderInterface $loader) $loader->load(__DIR__ . '/config/' . $this->configFile); $loader->load(__DIR__ . '/config/' . $this->commonFile); - if (TestHelper::phpAttributeSupportExists()) { + // Symfony 7 gets its routes from its own common file (sf7_common.yml): no @Route docblock imports + if (TestHelper::phpAttributeSupportExists() && TestHelper::docblockRoutingSupportExists()) { $loader->load(__DIR__ . '/config/attributed_common.yml'); } } diff --git a/tests/Functional/Fixtures/config/sf7_common.yml b/tests/Functional/Fixtures/config/sf7_common.yml new file mode 100644 index 0000000..8cf8d69 --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_common.yml @@ -0,0 +1,15 @@ +framework: + router: + resource: '%kernel.project_dir%/tests/Functional/Fixtures/config/sf7_routing.yml' + +security: + firewalls: + config: + pattern: ^/(config)/ + security: false + main: + http_basic: ~ + stateless: true + password_hashers: + Symfony\Component\Security\Core\User\InMemoryUser: + algorithm: plaintext diff --git a/tests/Functional/Fixtures/config/sf7_routing.yml b/tests/Functional/Fixtures/config/sf7_routing.yml new file mode 100644 index 0000000..80ec812 --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_routing.yml @@ -0,0 +1,9 @@ +# Symfony 7 reads no docblock annotations: the controllers routed by @Route docblocks are not imported here, their +# attribute twins in Controller/Attribute/ are. +paysera_fixture_test: + resource: "@PayseraFixtureTestBundle/Resources/config/explicit_routing.xml" + prefix: / + +paysera_fixture_attributed_test: + resource: '@PayseraFixtureTestBundle/Resources/config/attributed_routing.xml' + prefix: / diff --git a/tests/Functional/FunctionalAnnotationsTest.php b/tests/Functional/FunctionalAnnotationsTest.php index 41b6f7f..5781488 100644 --- a/tests/Functional/FunctionalAnnotationsTest.php +++ b/tests/Functional/FunctionalAnnotationsTest.php @@ -52,6 +52,9 @@ private function makeTest( if ($pathPrefix === 'attributed' && !TestHelper::phpAttributeSupportExists()) { $this->markTestSkipped('Unsupported environment'); } + if ($pathPrefix === 'annotated' && !TestHelper::docblockRoutingSupportExists()) { + $this->markTestSkipped('Symfony 7 reads no @Route docblocks'); + } $request->server->set( 'REQUEST_URI', diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index 0a1f80e..dbe494a 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -31,7 +31,12 @@ abstract class FunctionalTestCase extends TestCase */ protected function setUpContainer($testCase, $commonFile = 'common.yml') { - $prefix = Kernel::MAJOR_VERSION <= 4 ? 'legacy_' : ''; + $prefix = ''; + if (Kernel::MAJOR_VERSION <= 4) { + $prefix = 'legacy_'; + } elseif (Kernel::MAJOR_VERSION >= 7) { + $prefix = 'sf7_'; + } $this->kernel = new TestKernel($testCase, $prefix . $commonFile); $this->kernel->boot(); return $this->kernel->getContainer(); diff --git a/tests/Unit/Helper/HttpKernelHelper.php b/tests/Unit/Helper/HttpKernelHelper.php index 67a50b0..4e3d23a 100644 --- a/tests/Unit/Helper/HttpKernelHelper.php +++ b/tests/Unit/Helper/HttpKernelHelper.php @@ -10,7 +10,7 @@ class HttpKernelHelper { public static function getMainRequestConstValue(): int { - if (defined(HttpKernelInterface::class . 'MAIN_REQUEST')) { + if (defined(HttpKernelInterface::class . '::MAIN_REQUEST')) { return HttpKernelInterface::MAIN_REQUEST; } diff --git a/tests/Unit/Service/Validation/EntityValidatorTest.php b/tests/Unit/Service/Validation/EntityValidatorTest.php index 6f53884..493bc57 100644 --- a/tests/Unit/Service/Validation/EntityValidatorTest.php +++ b/tests/Unit/Service/Validation/EntityValidatorTest.php @@ -46,6 +46,7 @@ public function testValidate($expectedException, ValidationOptions $validationOp $entity = new stdClass(); $validator ->shouldReceive('validate') + ->once() ->with($entity, null, $groups) ->andReturn(new ConstraintViolationList($violationList)) ; @@ -67,7 +68,6 @@ public function testValidate($expectedException, ValidationOptions $validationOp if ($expectedException !== null) { $this->fail('Expected exception'); } - $this->expectNotToPerformAssertions(); } catch (ApiException $exception) { $this->assertEquals($expectedException, $exception); } From 669f40a8762989d8de77d0223e13c04fbad3df71 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:47:27 +0530 Subject: [PATCH 04/42] EE-283 Declare the return types Symfony 7 and 8 require Configuration::getConfigTreeBuilder() now returns TreeBuilder: Symfony 7's ConfigurationInterface declares that type, and without it the bundle cannot be loaded on Symfony 7 (fatal "must be compatible"). PayseraApiExtension::load() now returns void, which Symfony 8 requires; Symfony 7.4 only reports the missing type as a deprecation. A subclass that overrides either method without the type stops compiling. No such subclass exists in the applications that use the bundle. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/DependencyInjection/Configuration.php | 2 +- src/DependencyInjection/PayseraApiExtension.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 99b4c98..8faa5a7 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -10,7 +10,7 @@ class Configuration implements ConfigurationInterface { - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('paysera_api'); $rootNode = method_exists($treeBuilder, 'getRootNode') diff --git a/src/DependencyInjection/PayseraApiExtension.php b/src/DependencyInjection/PayseraApiExtension.php index 2a1db01..a60661b 100644 --- a/src/DependencyInjection/PayseraApiExtension.php +++ b/src/DependencyInjection/PayseraApiExtension.php @@ -17,7 +17,7 @@ class PayseraApiExtension extends Extension { - public function load(array $configs, ContainerBuilder $container) + public function load(array $configs, ContainerBuilder $container): void { $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); From 0d1924a842bd816cb4c37ba9f91d798fc4fd8edc Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:49:22 +0530 Subject: [PATCH 05/42] EE-283 Pick the same locale from Accept-Language on every Symfony line The locale listener asked Request::getPreferredLanguage() to choose between a placeholder "default" and the configured locales, and treated "default" as no match. Symfony 7.1 rewrote that method to match by prefix, so "default" now wins whenever a request asks for German ("default" starts with "de"), and it also ranks the languages differently (en-US now matches "en" before a de-CH;q=0.9 matches "de"). The listener now matches the header itself with the rule Symfony 3.4 to 7.0 apply: the first header language, in the client's order of preference, that is a configured locale, where a regional variant also offers its primary language unless the header lists it. The results are unchanged below Symfony 7.1. The existing eight test cases stay as they were; five cases are added, three of which fail on Symfony 7.4 with the old listener. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 31 ++++++++++++++++++---- tests/Unit/Listener/LocaleListenerTest.php | 30 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index 241101f..2e4863e 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -42,13 +42,34 @@ public function onKernelRequest($event) } } - private function resolveFromHeaders(Request $request) + /** + * The first language of Accept-Language, in the client's order of preference, that is a configured locale; a + * regional variant (de_CH) also offers its primary language (de) unless the header lists that language itself. + * + * This is the rule Request::getPreferredLanguage() applied up to Symfony 7.0. Symfony 7.1 changed it, and passing a + * placeholder for "no match" stopped working there ("default" starts with "de"), so the listener matches itself and + * picks the same locale on every Symfony line. + */ + private function resolveFromHeaders(Request $request): ?string { - $defaultLocale = 'default'; - $preferredLanguage = $request->getPreferredLanguage(array_merge([$defaultLocale], $this->locales)); + $languages = $request->getLanguages(); + $candidates = []; + foreach ($languages as $language) { + $candidates[] = $language; + $separatorPosition = strpos($language, '_'); + if ($separatorPosition === false) { + continue; + } + $primaryLanguage = substr($language, 0, $separatorPosition); + if (!in_array($primaryLanguage, $languages, true)) { + $candidates[] = $primaryLanguage; + } + } - if ($preferredLanguage !== null && $preferredLanguage !== $defaultLocale) { - return $preferredLanguage; + foreach ($candidates as $candidate) { + if (in_array($candidate, $this->locales, true)) { + return $candidate; + } } return null; diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index c404e8e..9946670 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -96,6 +96,36 @@ public function provider() 'en-US,en;q=0.8, de-CH;q=0.9', true, ], + 'German among other locales' => [ + 'de', + ['en', 'lt', 'de'], + 'de', + true, + ], + 'German region among other locales' => [ + 'de', + ['en', 'lt', 'de'], + 'de-DE, en;q=0.5', + true, + ], + 'primary language is not added when the header lists it' => [ + 'de', + ['de', 'en'], + 'en-US, en;q=0.5, de;q=0.9', + true, + ], + 'no match keeps the locale' => [ + 'unchanged', + ['en', 'lt', 'de'], + 'fr-FR, fr;q=0.9', + true, + ], + 'no header keeps the locale' => [ + 'unchanged', + ['en', 'lt', 'de'], + '', + true, + ], ]; } } From bc4d6eddab52d96fe226fbe9526f4497fa8ba510 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:52:02 +0530 Subject: [PATCH 06/42] EE-283 Refuse the bundle's docblock annotations on Symfony 7 Symfony 7 gives the route loader no Doctrine annotation reader, so on Symfony 7 a controller routed with #[Route] but configured with the bundle's docblock annotations (@Body, @Query, @PathAttribute, @ResponseNormalization, @RequiredPermissions, @Validation, @BodyContentType) was loaded with none of those options, without any error: an endpoint protected by @RequiredPermissions would have answered everyone. Where the route loader has no annotation reader (Symfony 7 and later), loading such a route now fails with a ConfigurationException that names the controller method, the annotations it uses and the attributes to use instead (the #[...] attributes exist since 1.8.0). The annotations are found without an annotation reader: DocblockAnnotationFinder resolves each docblock tag through the use imports of the file that declares it. Symfony 4.4 to 6.4 are unchanged: the reader still applies the annotations there. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 101 ++++++++++++++ .../RoutingLoader/RoutingAttributeLoader.php | 40 ++++++ .../DocblockAnnotationFinderTest.php | 67 ++++++++++ .../Fixtures/AliasedImportController.php | 20 +++ .../Fixtures/AttributeOnlyController.php | 25 ++++ .../Fixtures/ChildWithoutImports.php | 9 ++ .../Fixtures/DirectImportController.php | 26 ++++ ...blockOptionsOnAttributeRouteController.php | 19 +++ .../Fixtures/ParentWithDocblockOptions.php | 17 +++ .../RoutingAttributeLoaderTest.php | 124 ++++++++++++++++++ 10 files changed, 448 insertions(+) create mode 100644 src/Service/RoutingLoader/DocblockAnnotationFinder.php create mode 100644 tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildWithoutImports.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/DirectImportController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ParentWithDocblockOptions.php create mode 100644 tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php new file mode 100644 index 0000000..d5f3f47 --- /dev/null +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -0,0 +1,101 @@ +findInDocblock($class->getDocComment(), $class), + $this->findInDocblock($method->getDocComment(), $method->getDeclaringClass()) + ); + + return array_values(array_unique($found)); + } + + /** + * @param string|false $docComment + * @return string[] + */ + private function findInDocblock($docComment, ReflectionClass $declaringClass): array + { + if ($docComment === false || strpos($docComment, '@') === false) { + return []; + } + + preg_match_all('/@(\\\\?[A-Za-z_][A-Za-z0-9_]*(?:\\\\[A-Za-z_][A-Za-z0-9_]*)*)/', $docComment, $matches); + $imports = $this->readImports($declaringClass); + + $found = []; + foreach ($matches[1] as $name) { + $className = $this->resolveClassName($name, $imports, $declaringClass->getNamespaceName()); + if (is_subclass_of($className, RestAnnotationInterface::class)) { + $found[] = ltrim((new ReflectionClass($className))->getName(), '\\'); + } + } + + return $found; + } + + /** + * @return array imported class or namespace name by its lower-case alias + */ + private function readImports(ReflectionClass $class): array + { + $fileName = $class->getFileName(); + if ($fileName === false) { + return []; + } + + preg_match_all( + '/^\s*use\s+(\\\\?[A-Za-z_][A-Za-z0-9_\\\\]*)(?:\s+as\s+([A-Za-z_][A-Za-z0-9_]*))?\s*;/mi', + (string)file_get_contents($fileName), + $matches, + PREG_SET_ORDER + ); + + $imports = []; + foreach ($matches as $match) { + $importedName = ltrim($match[1], '\\'); + $parts = explode('\\', $importedName); + $alias = isset($match[2]) && $match[2] !== '' ? $match[2] : end($parts); + $imports[strtolower($alias)] = $importedName; + } + + return $imports; + } + + /** + * @param array $imports + */ + private function resolveClassName(string $name, array $imports, string $namespace): string + { + if ($name[0] === '\\') { + return ltrim($name, '\\'); + } + + $parts = explode('\\', $name, 2); + $alias = strtolower($parts[0]); + if (isset($imports[$alias])) { + return $imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : ''); + } + + return $namespace === '' ? $name : $namespace . '\\' . $name; + } +} diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index 955f27d..43ca860 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -6,6 +6,7 @@ use Paysera\Bundle\ApiBundle\Annotation\RestAnnotationInterface; use Paysera\Bundle\ApiBundle\Attribute\RestAttributeInterface; +use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use ReflectionClass; use ReflectionMethod; @@ -59,8 +60,16 @@ protected function configureRoute( $this->loadAttributes($route, $class, $method); } + /** + * @throws ConfigurationException on Symfony 7 and later when the controller uses the bundle's docblock annotations + */ private function loadAnnotations(Route $route, ReflectionClass $class, ReflectionMethod $method): void { + if (!property_exists($this, 'reader')) { + $this->refuseDocblockAnnotations($class, $method); + return; + } + if (!isset($this->reader)) { return; } @@ -88,6 +97,37 @@ private function loadAnnotations(Route $route, ReflectionClass $class, Reflectio ); } + /** + * Symfony 7 gives the route loader no annotation reader, so these options would be ignored without a word — an + * endpoint would lose its required permissions. Fail at route loading instead and name the attributes to use. + * + * @throws ConfigurationException + */ + private function refuseDocblockAnnotations(ReflectionClass $class, ReflectionMethod $method): void + { + $annotations = (new DocblockAnnotationFinder())->findBundleAnnotations($class, $method); + if ($annotations === []) { + return; + } + + $attributes = []; + foreach ($annotations as $annotation) { + $attributes[] = sprintf( + '#[%s]', + str_replace('\\Annotation\\', '\\Attribute\\', $annotation) + ); + } + + throw new ConfigurationException(sprintf( + '%s::%s() configures its REST endpoint with docblock annotations (%s), which Symfony 7 does not read, ' + . 'so the endpoint would run without those options. Use the attributes instead: %s.', + $class->getName(), + $method->getName(), + implode(', ', $annotations), + implode(', ', $attributes) + )); + } + private function loadAttributes(Route $route, ReflectionClass $class, ReflectionMethod $method): void { $attributes = array_merge($class->getAttributes(), $method->getAttributes()); diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php new file mode 100644 index 0000000..9aee6e6 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -0,0 +1,67 @@ +assertSame( + $expectedAnnotations, + $finder->findBundleAnnotations(new ReflectionClass($className), new ReflectionMethod($className, $methodName)) + ); + } + + public static function controllerDataProvider(): array + { + return [ + 'class imports, class and method docblocks, each annotation once' => [ + DirectImportController::class, + 'create', + [RequiredPermissions::class, Body::class], + ], + 'namespace alias, class alias and a fully qualified name' => [ + AliasedImportController::class, + 'find', + [Query::class, Validation::class, PathAttribute::class], + ], + 'attributes, Symfony tags and attribute class names are not the annotations' => [ + AttributeOnlyController::class, + 'create', + [], + ], + 'a method declared in a parent resolves through the parent file imports' => [ + ChildWithoutImports::class, + 'inherited', + [ResponseNormalization::class], + ], + ]; + } +} diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php new file mode 100644 index 0000000..1684d31 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php @@ -0,0 +1,20 @@ +requestHelper = Mockery::mock(RestRequestHelper::class); + $this->annotationOptionsBuilder = Mockery::mock(RestRequestAnnotationOptionsBuilder::class); + $this->attributeOptionsBuilder = Mockery::mock(RestRequestAttributeOptionsBuilder::class); + } + + public function testRefusesTheBundleDocblockAnnotationsWhereSymfonyReadsNone() + { + $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); + $loader = $this->createLoader(); + $this->requestHelper->shouldNotReceive('setOptionsForRoute'); + + try { + $loader->load(DocblockOptionsOnAttributeRouteController::class); + $this->fail('The route with the bundle\'s docblock annotations was loaded without them'); + } catch (ConfigurationException $exception) { + $this->assertStringContainsString( + DocblockOptionsOnAttributeRouteController::class . '::show() configures its REST endpoint with ' + . 'docblock annotations (' . RequiredPermissions::class . ')', + $exception->getMessage() + ); + $this->assertStringContainsString( + 'Use the attributes instead: #[Paysera\Bundle\ApiBundle\Attribute\RequiredPermissions].', + $exception->getMessage() + ); + } + } + + public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() + { + $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); + $loader = $this->createLoader(); + $options = new RestRequestOptions(); + $this->attributeOptionsBuilder->shouldReceive('buildOptions')->once()->andReturn($options); + $this->requestHelper->shouldReceive('setOptionsForRoute')->once()->with(Mockery::any(), $options); + + $routes = $loader->load(AttributeOnlyController::class); + + $this->assertCount(1, $routes); + } + + public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSymfony7() + { + if (!class_exists(AttributeRouteControllerLoader::class) + || !property_exists(AttributeRouteControllerLoader::class, 'reader') + ) { + $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader'); + } + $loader = $this->createLoader(new AnnotationReader()); + $options = new RestRequestOptions(); + $this->annotationOptionsBuilder + ->shouldReceive('buildOptions') + ->once() + ->with(Mockery::on(function (array $annotations) { + return count($annotations) === 1 && $annotations[0] instanceof RequiredPermissions; + }), Mockery::any()) + ->andReturn($options) + ; + $this->requestHelper->shouldReceive('setOptionsForRoute')->once()->with(Mockery::any(), $options); + + $routes = $loader->load(DocblockOptionsOnAttributeRouteController::class); + + $this->assertCount(1, $routes); + } + + private function skipUnlessTheRouteLoaderHasNoAnnotationReader() + { + if (!class_exists(AttributeRouteControllerLoader::class) + || property_exists(AttributeRouteControllerLoader::class, 'reader') + ) { + $this->markTestSkipped('Symfony 6.4 and older give the route loader an annotation reader'); + } + } + + private function createLoader(...$constructorArguments): RoutingAttributeLoader + { + $loader = new RoutingAttributeLoader(...$constructorArguments); + $loader->setRequestHelper($this->requestHelper); + $loader->setRestRequestAnnotationOptionsBuilder($this->annotationOptionsBuilder); + $loader->setRestRequestAttributeOptionsBuilder($this->attributeOptionsBuilder); + + return $loader; + } +} From 18b949ac530e8c331b8137ef31aaaed7dca4a3f0 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:52:23 +0530 Subject: [PATCH 07/42] EE-283 Declare the optional parameters nullable explicitly Eleven parameters in nine files were written "Type $x = null", which PHP 8.4 reports as a deprecated implicit nullable type. They are now "?Type $x = null": the same type on every PHP version the bundle supports (7.1 and later), so no caller or subclass is affected. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Attribute/Body.php | 2 +- src/Attribute/BodyContentType.php | 2 +- src/Attribute/PathAttribute.php | 4 ++-- src/Attribute/Query.php | 2 +- src/Attribute/RequiredPermissions.php | 2 +- src/Entity/QueryResolverOptions.php | 2 +- src/Exception/ApiException.php | 2 +- src/Listener/RestExceptionListener.php | 2 +- src/Service/Validation/EntityValidator.php | 4 ++-- 9 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Attribute/Body.php b/src/Attribute/Body.php index a04ded8..8dcd5f5 100644 --- a/src/Attribute/Body.php +++ b/src/Attribute/Body.php @@ -34,7 +34,7 @@ class Body implements RestAttributeInterface public function __construct( array $options = [], - string $parameterName = null, + ?string $parameterName = null, ?string $denormalizationType = null, ?string $denormalizationGroup = null, ?bool $optional = null diff --git a/src/Attribute/BodyContentType.php b/src/Attribute/BodyContentType.php index d1b0f1a..d42f9ea 100644 --- a/src/Attribute/BodyContentType.php +++ b/src/Attribute/BodyContentType.php @@ -23,7 +23,7 @@ class BodyContentType implements RestAttributeInterface public function __construct( array $options = [], - array $supportedContentTypes = null, + ?array $supportedContentTypes = null, bool $jsonEncodedBody = false ) { $this->setSupportedContentTypes($options['supportedContentTypes'] ?? $supportedContentTypes); diff --git a/src/Attribute/PathAttribute.php b/src/Attribute/PathAttribute.php index 471f9a7..fca6827 100644 --- a/src/Attribute/PathAttribute.php +++ b/src/Attribute/PathAttribute.php @@ -35,8 +35,8 @@ class PathAttribute implements RestAttributeInterface public function __construct( array $options = [], - string $parameterName = null, - string $pathPartName = null, + ?string $parameterName = null, + ?string $pathPartName = null, ?string $resolverType = null, ?bool $resolutionMandatory = null ) { diff --git a/src/Attribute/Query.php b/src/Attribute/Query.php index ea47fb8..0a23815 100644 --- a/src/Attribute/Query.php +++ b/src/Attribute/Query.php @@ -35,7 +35,7 @@ class Query implements RestAttributeInterface public function __construct( array $options = [], - string $parameterName = null, + ?string $parameterName = null, ?string $denormalizationType = null, ?string $denormalizationGroup = null, ?Validation $validation = null diff --git a/src/Attribute/RequiredPermissions.php b/src/Attribute/RequiredPermissions.php index 648d0c1..4960d7a 100644 --- a/src/Attribute/RequiredPermissions.php +++ b/src/Attribute/RequiredPermissions.php @@ -18,7 +18,7 @@ class RequiredPermissions implements RestAttributeInterface public function __construct( array $options = [], - array $permissions = null + ?array $permissions = null ) { $this->setPermissions($options['permissions'] ?? $permissions); } diff --git a/src/Entity/QueryResolverOptions.php b/src/Entity/QueryResolverOptions.php index b898a53..e3701c9 100644 --- a/src/Entity/QueryResolverOptions.php +++ b/src/Entity/QueryResolverOptions.php @@ -58,7 +58,7 @@ public function setDenormalizationGroup($denormalizationGroup): self * @param ValidationOptions|null $validationOptions * @return $this */ - public function setValidationOptions(ValidationOptions $validationOptions = null): self + public function setValidationOptions(?ValidationOptions $validationOptions = null): self { $this->validationOptions = $validationOptions; return $this; diff --git a/src/Exception/ApiException.php b/src/Exception/ApiException.php index ae8d096..010c1ad 100644 --- a/src/Exception/ApiException.php +++ b/src/Exception/ApiException.php @@ -60,7 +60,7 @@ public function __construct( $errorCode, $message = null, $statusCode = null, - Exception $previous = null, + ?Exception $previous = null, $properties = null, $data = null, array $violations = [] diff --git a/src/Listener/RestExceptionListener.php b/src/Listener/RestExceptionListener.php index 3d71a9b..6b3e9e8 100644 --- a/src/Listener/RestExceptionListener.php +++ b/src/Listener/RestExceptionListener.php @@ -32,7 +32,7 @@ public function __construct( ErrorBuilderInterface $errorBuilder, CoreNormalizer $coreNormalizer, ResponseBuilder $responseBuilder, - LoggerInterface $logger = null + ?LoggerInterface $logger = null ) { $this->requestHelper = $requestHelper; $this->errorBuilder = $errorBuilder; diff --git a/src/Service/Validation/EntityValidator.php b/src/Service/Validation/EntityValidator.php index 830d215..4335bae 100644 --- a/src/Service/Validation/EntityValidator.php +++ b/src/Service/Validation/EntityValidator.php @@ -26,8 +26,8 @@ class EntityValidator protected $propertyPathConverter; public function __construct( - ValidatorInterface $validator = null, - PropertyPathConverterInterface $propertyPathConverter = null + ?ValidatorInterface $validator = null, + ?PropertyPathConverterInterface $propertyPathConverter = null ) { $this->validator = $validator; $this->propertyPathConverter = $propertyPathConverter; From 7490f2f5573e93474402eb329227267ca4af1a0e Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:54:14 +0530 Subject: [PATCH 08/42] EE-283 Add the 1.9.0 changelog entry Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67337b9..9aa8033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,28 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.9.0] +### Added +- Support for Symfony 7.4 +- Support for `psr/log` 3 + +### Changed +- The Symfony components the bundle uses directly are required explicitly: `symfony/config`, `symfony/dependency-injection`, + `symfony/http-foundation`, `symfony/http-kernel`, `symfony/property-access`, `symfony/routing` and `symfony/security-core` +- `Configuration::getConfigTreeBuilder()` declares its `TreeBuilder` return type and `PayseraApiExtension::load()` declares + `void`. Breaking for subclasses that override either method without the return type: add `: TreeBuilder` or `: void` to + the override +- On Symfony 7, loading a route whose controller configures it with the bundle's docblock annotations (`@Body`, `@Query`, + `@PathAttribute`, `@ResponseNormalization`, `@RequiredPermissions`, `@Validation`, `@BodyContentType`) fails with a + `ConfigurationException` that names the attributes to use instead. Symfony 7 does not read docblock annotations, so these + options were ignored without an error. Symfony 4.4 to 6.4 are unchanged +- Optional parameters are declared nullable explicitly (`?Type $parameter = null`), as PHP 8.4 expects +- CI runs the tests on Symfony 7 with PHP 8.2 and 8.3 + +### Fixed +- On Symfony 7.1 and later, `LocaleListener` picks the locale from `Accept-Language` the same way as on older Symfony + versions: a request asking for German (`de`) kept the default locale there + ## [1.8.2] ### Changed - CI allows packages with security advisories, so Symfony 3.4 and 4.4 jobs can install dependencies with Composer 2.10 From ed648fff46433bc6e062dcddc9b379391f447de4 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 20:57:09 +0530 Subject: [PATCH 09/42] EE-283 Pass the permissions to the attribute by name in the loader test The attribute's first parameter is the options array, so a positional list made the test fixture fail with a TypeError on Symfony 7.4. The bundle's own fixtures and the one application using the attribute pass it as permissions: [...]. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Service/RoutingLoader/Fixtures/AttributeOnlyController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php index 2089841..0a2b87d 100644 --- a/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php +++ b/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php @@ -18,7 +18,7 @@ class AttributeOnlyController */ #[Route('/attribute-only', methods: ['POST'])] #[Body(parameterName: 'item')] - #[RequiredPermissions(['ROLE_ADMIN'])] + #[RequiredPermissions(permissions: ['ROLE_ADMIN'])] public function create($item) { } From aabffbeace256673db3d3d93018bbb9e95a796cc Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:00:55 +0530 Subject: [PATCH 10/42] EE-283 Cover the error mapping and the untested branches Tests only. The suite executed 910 of the 1,027 executable lines in src/ (88.6 %); the largest gap was ErrorBuilder, which builds every error response and had no test (30 of 84 lines). - ErrorBuilderTest: the error code, status and message for each kind of exception the builder maps, with the codes the bundle configures, and the API exception's details. - The guards that were never reached: reading an option before it is set, an unknown path attribute resolver type, validation without the validator service, a path attribute or query whose type cannot be guessed, explicit arguments winning over the signature. - RestResponseListener on a request that is not a REST request and on a controller returning nothing; RestRequestHelper for a controller with no identifier. - Two request-body errors in the functional suite: a body without Content-Type where the endpoint restricts content types, and a JSON body that does not decode. - RoutingAttributeLoader on Symfony 6.4 when the application disabled annotations: the docblock options stay ignored there, as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Functional/FunctionalAnnotationsTest.php | 23 ++ .../AttributeParameterResolutionTest.php | 79 +++++++ tests/Unit/Entity/UnsetOptionsTest.php | 67 ++++++ .../Listener/RestResponseListenerTest.php | 62 +++++ tests/Unit/Service/ErrorBuilderTest.php | 216 ++++++++++++++++++ .../PathAttributeResolverRegistryTest.php | 33 +++ tests/Unit/Service/RestRequestHelperTest.php | 12 + .../RoutingAttributeLoaderTest.php | 16 ++ .../Validation/EntityValidatorTest.php | 10 + 9 files changed, 518 insertions(+) create mode 100644 tests/Unit/Attribute/AttributeParameterResolutionTest.php create mode 100644 tests/Unit/Entity/UnsetOptionsTest.php create mode 100644 tests/Unit/Listener/RestResponseListenerTest.php create mode 100644 tests/Unit/Service/ErrorBuilderTest.php create mode 100644 tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php diff --git a/tests/Functional/FunctionalAnnotationsTest.php b/tests/Functional/FunctionalAnnotationsTest.php index 5781488..0c2ea54 100644 --- a/tests/Functional/FunctionalAnnotationsTest.php +++ b/tests/Functional/FunctionalAnnotationsTest.php @@ -180,6 +180,29 @@ public function restRequestsConfigurationProvider(): array ['Content-Type' => 'text/plain'] ), ], + 'testBodyNormalizationWithCustomContentType and no content type' => [ + new Response( + '{"error":"invalid_request","error_description":"Content-Type must be provided"}', + 400 + ), + $this->createRequest( + 'POST', + '/testBodyNormalizationWithCustomContentType', + 'my_text' + ), + ], + 'testBodyNormalizationWithExtractedKeyValue and a body that is not JSON' => [ + new Response( + '{"error":"invalid_request","error_description":"Cannot decode request body to JSON"}', + 400 + ), + $this->createRequest( + 'POST', + '/testBodyNormalizationWithExtractedKeyValue', + '{"key": ', + ['Content-Type' => 'application/json'] + ), + ], 'testBodyNormalizationWithCustomContentTypeAndJsonDecode and JSON content-type' => [ new Response( '{"error":"invalid_request","error_description":"This Content-Type (application/json) is not supported"}', diff --git a/tests/Unit/Attribute/AttributeParameterResolutionTest.php b/tests/Unit/Attribute/AttributeParameterResolutionTest.php new file mode 100644 index 0000000..a8b7193 --- /dev/null +++ b/tests/Unit/Attribute/AttributeParameterResolutionTest.php @@ -0,0 +1,79 @@ + 'item', 'pathPartName' => 'id']); + + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage('Denormalization type could not be guessed for $item in '); + + $attribute->apply(new RestRequestOptions(), $this->wrapUntypedAction()); + } + + public function testQueryCannotGuessTheTypeOfAnUntypedParameter() + { + $attribute = new Query(['parameterName' => 'item']); + + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage('Denormalization type could not be guessed for $item in '); + + $attribute->apply(new RestRequestOptions(), $this->wrapUntypedAction()); + } + + public function testExplicitArgumentsWinOverTheSignature() + { + $options = new RestRequestOptions(); + + (new PathAttribute([ + 'parameterName' => 'item', + 'pathPartName' => 'id', + 'resolverType' => 'custom_resolver', + 'resolutionMandatory' => false, + ]))->apply($options, $this->wrapUntypedAction()); + (new Body([ + 'parameterName' => 'item', + 'denormalizationType' => 'custom_type', + 'optional' => true, + ]))->apply($options, $this->wrapUntypedAction()); + + $pathAttributeOptions = $options->getPathAttributeResolverOptionsList()[0]; + $this->assertSame( + ['custom_resolver', false, true], + [ + $pathAttributeOptions->getPathAttributeResolverType(), + $pathAttributeOptions->isResolutionMandatory(), + $options->isBodyOptional(), + ] + ); + } + + /** + * @param mixed $item + */ + public function untypedAction($item) + { + } + + private function wrapUntypedAction(): ReflectionMethodWrapper + { + return new ReflectionMethodWrapper(new ReflectionMethod(self::class, 'untypedAction')); + } +} diff --git a/tests/Unit/Entity/UnsetOptionsTest.php b/tests/Unit/Entity/UnsetOptionsTest.php new file mode 100644 index 0000000..ebd9300 --- /dev/null +++ b/tests/Unit/Entity/UnsetOptionsTest.php @@ -0,0 +1,67 @@ +expectException(RuntimeException::class); + $this->expectExceptionMessage($expectedMessage); + + $options->$getter(); + } + + public static function unsetOptionDataProvider(): array + { + return [ + [new PathAttributeResolverOptions(), 'getParameterName', 'parameterName was not set'], + [new PathAttributeResolverOptions(), 'getPathPartName', 'pathPartName was not set'], + [new PathAttributeResolverOptions(), 'getPathAttributeResolverType', 'pathAttributeResolverType was not set'], + [new QueryResolverOptions(), 'getParameterName', 'parameterName was not set'], + [new QueryResolverOptions(), 'getDenormalizationType', 'denormalizationType was not set'], + [ + new QueryResolverOptions(), + 'getValidationOptions', + 'No validationOptions available, call isValidationNeeded beforehand', + ], + [ + new RestRequestOptions(), + 'getBodyDenormalizationType', + 'No bodyDenormalizationType available, call hasBodyDenormalization beforehand', + ], + [ + new RestRequestOptions(), + 'getBodyParameterName', + 'No bodyParameterName available, call hasBodyDenormalization beforehand', + ], + [ + (new RestRequestOptions())->disableBodyValidation(), + 'getBodyValidationOptions', + 'No bodyValidationOptions available, call isBodyValidationNeeded beforehand', + ], + ]; + } + + public function testErrorKeepsItsUri() + { + $error = (new Error())->setUri('https://example.com/errors/not_found'); + + $this->assertSame('https://example.com/errors/not_found', $error->getUri()); + } +} diff --git a/tests/Unit/Listener/RestResponseListenerTest.php b/tests/Unit/Listener/RestResponseListenerTest.php new file mode 100644 index 0000000..849f312 --- /dev/null +++ b/tests/Unit/Listener/RestResponseListenerTest.php @@ -0,0 +1,62 @@ +shouldReceive('isRestRequest')->andReturn(false); + $event = $this->createViewEvent(['a' => 'result']); + + $this->createListener($requestHelper, Mockery::mock(ResponseBuilder::class))->onKernelView($event); + + $this->assertNull($event->getResponse()); + } + + public function testAnswersAControllerThatReturnsNothingWithAnEmptyResponse() + { + $requestHelper = Mockery::mock(RestRequestHelper::class); + $requestHelper->shouldReceive('isRestRequest')->andReturn(true); + $emptyResponse = new Response('', Response::HTTP_NO_CONTENT); + $responseBuilder = Mockery::mock(ResponseBuilder::class); + $responseBuilder->shouldReceive('buildEmptyResponse')->once()->andReturn($emptyResponse); + $event = $this->createViewEvent(null); + + $this->createListener($requestHelper, $responseBuilder)->onKernelView($event); + + $this->assertSame($emptyResponse, $event->getResponse()); + } + + private function createListener($requestHelper, $responseBuilder): RestResponseListener + { + return new RestResponseListener(Mockery::mock(CoreNormalizer::class), $requestHelper, $responseBuilder); + } + + private function createViewEvent($controllerResult) + { + $kernel = Mockery::mock(HttpKernelInterface::class); + $requestType = HttpKernelHelper::getMainRequestConstValue(); + if (class_exists(ViewEvent::class)) { + return new ViewEvent($kernel, new Request(), $requestType, $controllerResult); + } + + return new GetResponseForControllerResultEvent($kernel, new Request(), $requestType, $controllerResult); + } +} diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php new file mode 100644 index 0000000..bcf186c --- /dev/null +++ b/tests/Unit/Service/ErrorBuilderTest.php @@ -0,0 +1,216 @@ +createConfiguredErrorBuilder()->createErrorFromException($exception); + + $this->assertSame( + [$expectedCode, $expectedStatusCode, $expectedMessage], + [$error->getCode(), $error->getStatusCode(), $error->getMessage()] + ); + } + + public static function exceptionDataProvider(): array + { + $tooLargeOffset = new TooLargeOffsetException(1000, 1001); + $invalidOrderBy = new InvalidOrderByException('bogus_field'); + + return [ + 'API exception with its own code and status' => [ + new ApiException('custom_code', 'Custom message', 418), + 'custom_code', + 418, + 'Custom message', + ], + 'API exception with a configured code' => [ + new ApiException(ApiException::NOT_FOUND), + 'not_found', + 404, + 'Resource was not found', + ], + 'API exception with an unconfigured code' => [ + new ApiException('unconfigured_code'), + 'unconfigured_code', + 400, + null, + ], + 'invalid data' => [new InvalidDataException('Bad data'), 'invalid_parameters', 400, 'Bad data'], + 'invalid item' => [new InvalidItemException('amount'), 'invalid_parameters', 400, 'Invalid key "amount"'], + 'offset over the maximum' => [ + $tooLargeOffset, + 'offset_too_large', + 400, + $tooLargeOffset->getMessage(), + ], + 'invalid cursor without a message' => [ + new InvalidCursorException(), + 'invalid_cursor', + 400, + 'Provided cursor is invalid', + ], + 'invalid cursor with a message' => [ + new InvalidCursorException('Bad cursor'), + 'invalid_cursor', + 400, + 'Bad cursor', + ], + 'unsupported order-by field' => [ + $invalidOrderBy, + 'invalid_parameters', + 400, + $invalidOrderBy->getMessage(), + ], + 'no credentials' => [ + new AuthenticationCredentialsNotFoundException(), + 'unauthorized', + 401, + 'No authorization data found', + ], + 'authentication failure' => [ + new AuthenticationException('Internal detail'), + 'unauthorized', + 401, + 'You have not provided any credentials or they are invalid', + ], + 'authentication failure meant for the client (code 999)' => [ + new AuthenticationException('Token expired', 999), + 'unauthorized', + 401, + 'Token expired', + ], + 'access denied by security' => [ + new AccessDeniedException('Access Denied.'), + 'forbidden', + 403, + 'Access Denied.', + ], + 'access denied over HTTP' => [new AccessDeniedHttpException('Denied'), 'forbidden', 403, 'Denied'], + 'no route' => [new ResourceNotFoundException(), 'not_found', 404, 'Provided url not found'], + 'not found over HTTP' => [new NotFoundHttpException(), 'not_found', 404, 'Provided url not found'], + 'method not allowed by routing' => [ + new MethodNotAllowedException(['GET']), + 'not_found', + 405, + 'Provided method not allowed for this url', + ], + 'HTTP 404' => [new HttpException(404), 'not_found', 404, 'Resource was not found'], + 'HTTP 405' => [new HttpException(405), 'not_found', 405, 'Provided method not allowed for this url'], + 'HTTP 401' => [ + new HttpException(401), + 'unauthorized', + 401, + 'You have not provided any credentials or they are invalid', + ], + 'HTTP 403' => [ + new HttpException(403), + 'forbidden', + 403, + 'You have no rights to access requested resource or make requested action', + ], + 'HTTP 400' => [new HttpException(400), 'invalid_request', 400, 'Request content is invalid'], + 'other HTTP client error' => [ + new HttpException(418), + 'internal_server_error', + 500, + 'Unexpected internal system error', + ], + 'HTTP server error' => [ + new HttpException(503), + 'internal_server_error', + 500, + 'Unexpected internal system error', + ], + 'any other exception' => [ + new RuntimeException('Secret detail'), + 'internal_server_error', + 500, + 'Unexpected internal system error', + ], + ]; + } + + public function testCarriesTheApiExceptionDetails() + { + $violation = (new Violation())->setField('amount')->setMessage('Too large'); + $exception = (new ApiException('custom_code')) + ->setProperties(['amount' => 'Too large']) + ->setData(['limit' => 100]) + ->setViolations([$violation]) + ; + + $error = $this->createConfiguredErrorBuilder()->createErrorFromException($exception); + + $this->assertSame( + [['amount' => 'Too large'], ['limit' => 100], [$violation]], + [$error->getProperties(), $error->getData(), $error->getViolations()] + ); + } + + private function createConfiguredErrorBuilder(): ErrorBuilder + { + $errorBuilder = new ErrorBuilder(); + $errorBuilder->configureError('invalid_request', 400, 'Request content is invalid'); + $errorBuilder->configureError( + 'invalid_parameters', + 400, + 'Some required parameter is missing or it\'s format is invalid' + ); + $errorBuilder->configureError( + 'invalid_state', + 409, + 'Requested action cannot be made to the current state of resource' + ); + $errorBuilder->configureError( + 'unauthorized', + 401, + 'You have not provided any credentials or they are invalid' + ); + $errorBuilder->configureError( + 'forbidden', + 403, + 'You have no rights to access requested resource or make requested action' + ); + $errorBuilder->configureError('not_found', 404, 'Resource was not found'); + $errorBuilder->configureError('internal_server_error', 500, 'Unexpected internal system error'); + $errorBuilder->configureError('not_acceptable', 406, 'Unknown request or response format'); + + return $errorBuilder; + } +} diff --git a/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php b/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php new file mode 100644 index 0000000..924024d --- /dev/null +++ b/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php @@ -0,0 +1,33 @@ +registerPathAttributeResolver($resolver, 'user'); + + $this->assertSame($resolver, $registry->getResolverByType('user')); + } + + public function testFailsForAnUnregisteredType() + { + $registry = new PathAttributeResolverRegistry(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('No such path attribute resolver registered: "user"'); + + $registry->getResolverByType('user'); + } +} diff --git a/tests/Unit/Service/RestRequestHelperTest.php b/tests/Unit/Service/RestRequestHelperTest.php index 6da258c..7c97af2 100644 --- a/tests/Unit/Service/RestRequestHelperTest.php +++ b/tests/Unit/Service/RestRequestHelperTest.php @@ -63,6 +63,18 @@ public function testResolveRestRequestOptionsWithNoRegisteredOptions() $this->assertNull($helper->resolveRestRequestOptionsForRequest($request)); } + public function testResolvesNoOptionsForAControllerWithoutIdentifierThatIsNotAClassMethodPair() + { + $registry = Mockery::mock(RestRequestOptionsRegistry::class); + $registry->shouldNotReceive('getRestRequestOptionsForController'); + $helper = new RestRequestHelper($registry); + + $options = $helper->resolveRestRequestOptionsForController(new Request(), function () { + }); + + $this->assertNull($options); + } + public function testResolveRestRequestOptionsWithRegisteredOptionsAndCustomController() { $registry = Mockery::mock(RestRequestOptionsRegistry::class); diff --git a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php index 8f03dd5..9606fc1 100644 --- a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php +++ b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php @@ -103,6 +103,22 @@ public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSym $this->assertCount(1, $routes); } + public function testIgnoresTheBundleDocblockAnnotationsWhereTheApplicationDisabledAnnotationsBeforeSymfony7() + { + if (!class_exists(AttributeRouteControllerLoader::class) + || !property_exists(AttributeRouteControllerLoader::class, 'reader') + ) { + $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader property'); + } + $loader = $this->createLoader(); + $this->annotationOptionsBuilder->shouldNotReceive('buildOptions'); + $this->requestHelper->shouldNotReceive('setOptionsForRoute'); + + $routes = $loader->load(DocblockOptionsOnAttributeRouteController::class); + + $this->assertCount(1, $routes); + } + private function skipUnlessTheRouteLoaderHasNoAnnotationReader() { if (!class_exists(AttributeRouteControllerLoader::class) diff --git a/tests/Unit/Service/Validation/EntityValidatorTest.php b/tests/Unit/Service/Validation/EntityValidatorTest.php index 493bc57..cb1fc28 100644 --- a/tests/Unit/Service/Validation/EntityValidatorTest.php +++ b/tests/Unit/Service/Validation/EntityValidatorTest.php @@ -19,6 +19,16 @@ class EntityValidatorTest extends MockeryTestCase { + public function testValidateRequiresTheValidator() + { + $entityValidator = new EntityValidator(null, Mockery::mock(PropertyPathConverterInterface::class)); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('To use validation in RestBundle you must configure framework.validation'); + + $entityValidator->validate(new stdClass(), new ValidationOptions()); + } + public function testValidateDoesNotFailWithNonObject() { $validator = Mockery::mock(ValidatorInterface::class); From 5cd009cbaee77c2821ea34ad1e151696c0aedaa3 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:02:25 +0530 Subject: [PATCH 11/42] EE-283 Clear the query validation options before reading them in the test QueryResolverOptions starts with validation options set, so the guard is only reached once they are removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Entity/UnsetOptionsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Entity/UnsetOptionsTest.php b/tests/Unit/Entity/UnsetOptionsTest.php index ebd9300..3c6212e 100644 --- a/tests/Unit/Entity/UnsetOptionsTest.php +++ b/tests/Unit/Entity/UnsetOptionsTest.php @@ -36,7 +36,7 @@ public static function unsetOptionDataProvider(): array [new QueryResolverOptions(), 'getParameterName', 'parameterName was not set'], [new QueryResolverOptions(), 'getDenormalizationType', 'denormalizationType was not set'], [ - new QueryResolverOptions(), + (new QueryResolverOptions())->setValidationOptions(null), 'getValidationOptions', 'No validationOptions available, call isValidationNeeded beforehand', ], From 5927f2ac81cd6cf7d941643177435981203c4d87 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:08:00 +0530 Subject: [PATCH 12/42] EE-283 Drop the finder's unreachable branch for classes without a file Only internal classes have no file, and they carry no docblocks, so readImports() never met one; the guard was a line no test could reach. The file name check stays, inside the expression. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Service/RoutingLoader/DocblockAnnotationFinder.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index d5f3f47..ec8d99a 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -58,14 +58,11 @@ private function findInDocblock($docComment, ReflectionClass $declaringClass): a */ private function readImports(ReflectionClass $class): array { + // only internal classes have no file, and they carry no docblocks, so this method never sees one $fileName = $class->getFileName(); - if ($fileName === false) { - return []; - } - preg_match_all( '/^\s*use\s+(\\\\?[A-Za-z_][A-Za-z0-9_\\\\]*)(?:\s+as\s+([A-Za-z_][A-Za-z0-9_]*))?\s*;/mi', - (string)file_get_contents($fileName), + $fileName === false ? '' : (string)file_get_contents($fileName), $matches, PREG_SET_ORDER ); From 75c78b95fa18c4ab83d8a5d8f370b915a97f7d6f Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:50:00 +0530 Subject: [PATCH 13/42] EE-283 Find the bundle's docblock annotations wherever Doctrine's reader found them The Symfony 7 guard missed five forms that Doctrine's reader applies on Symfony 4.4 to 6.4, so on Symfony 7 such a route still loaded with its options silently dropped: a group import (use ...\Annotation\{Body, RequiredPermissions}), a comma-separated import, an import on the namespace line, a full class name without the leading backslash, and an action that comes from a trait whose file imports the annotation. The finder now reads the use statements in all these forms, merges the imports of the file that declares the method with the declaring class's (as Doctrine does for traits), and resolves a name through an import, then the namespace, then as a fully qualified name. It also counts an @ only where Doctrine's lexer counts a top-level annotation (at the start of the docblock text, after whitespace or *), so {@inheritdoc}-style inline tags, e-mail addresses and annotations nested in another annotation's arguments are no longer reported. Each file's imports are read once per route loading. The error message no longer claims the endpoint would run without options when an attribute is also present, writes the attribute class with its leading backslash, and names the attribute interface for an application's own annotation class instead of an attribute that does not exist. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 119 +++++++++++++----- .../RoutingLoader/RoutingAttributeLoader.php | 34 +++-- .../DocblockAnnotationFinderTest.php | 40 ++++++ .../Fixtures/CommaImportController.php | 19 +++ .../Fixtures/ControllerUsingTheTrait.php | 10 ++ ...omAnnotationOnAttributeRouteController.php | 18 +++ .../Fixtures/CustomRestAnnotation.php | 27 ++++ .../Fixtures/GroupImportController.php | 18 +++ .../NotTopLevelAnnotationsController.php | 21 ++++ .../Fixtures/SameLineImportController.php | 16 +++ .../Fixtures/TraitWithDocblockOptions.php | 17 +++ .../RoutingAttributeLoaderTest.php | 29 +++-- 12 files changed, 319 insertions(+), 49 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CommaImportController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ControllerUsingTheTrait.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CustomAnnotationOnAttributeRouteController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CustomRestAnnotation.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/GroupImportController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NotTopLevelAnnotationsController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/SameLineImportController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TraitWithDocblockOptions.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index ec8d99a..afebc56 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -9,21 +9,33 @@ use ReflectionMethod; /** - * Finds this bundle's annotations in the docblocks of a controller method and its class without an annotation reader: - * each docblock tag is resolved through the `use` imports of the file that declares it. + * Finds the annotations for this bundle in the docblocks of a controller method and its class without an annotation + * reader. Each docblock tag is resolved the way Doctrine's reader resolves it on Symfony 4.4 to 6.4: through the `use` + * imports of the declaring class's file and of the file that declares the method (a trait's), then relative to the + * class's namespace, then as a fully qualified name. * * @internal */ class DocblockAnnotationFinder { /** - * @return string[] class names of the bundle's annotations the docblocks use, in order, each once + * @var array> imports by file name + */ + private $importsByFile = []; + + /** + * @return string[] class names of the bundle annotations the docblocks use, in order, each once */ public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $method): array { + $declaringClass = $method->getDeclaringClass(); $found = array_merge( - $this->findInDocblock($class->getDocComment(), $class), - $this->findInDocblock($method->getDocComment(), $method->getDeclaringClass()) + $this->findInDocblock($class->getDocComment(), [$class->getFileName()], $class->getNamespaceName()), + $this->findInDocblock( + $method->getDocComment(), + [$declaringClass->getFileName(), $method->getFileName()], + $declaringClass->getNamespaceName() + ) ); return array_values(array_unique($found)); @@ -31,22 +43,32 @@ public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $ /** * @param string|false $docComment + * @param array $fileNames the files whose imports apply; a later file's import wins * @return string[] */ - private function findInDocblock($docComment, ReflectionClass $declaringClass): array + private function findInDocblock($docComment, array $fileNames, string $namespace): array { if ($docComment === false || strpos($docComment, '@') === false) { return []; } - preg_match_all('/@(\\\\?[A-Za-z_][A-Za-z0-9_]*(?:\\\\[A-Za-z_][A-Za-z0-9_]*)*)/', $docComment, $matches); - $imports = $this->readImports($declaringClass); + // a top-level annotation, as Doctrine's lexer reads it: "@" at the start or after whitespace or "*", so + // "{@inheritdoc}", "mail@host" and an annotation nested in another annotation's arguments do not count + preg_match_all( + '/(?:^|[\s*])@(\\\\?[A-Za-z_][A-Za-z0-9_]*(?:\\\\[A-Za-z_][A-Za-z0-9_]*)*)/', + $docComment, + $matches + ); + $imports = []; + foreach (array_unique($fileNames) as $fileName) { + $imports = array_merge($imports, $this->readImports($fileName)); + } $found = []; foreach ($matches[1] as $name) { - $className = $this->resolveClassName($name, $imports, $declaringClass->getNamespaceName()); - if (is_subclass_of($className, RestAnnotationInterface::class)) { - $found[] = ltrim((new ReflectionClass($className))->getName(), '\\'); + $className = $this->resolveClassName($name, $imports, $namespace); + if ($className !== null) { + $found[] = $className; } } @@ -54,25 +76,49 @@ private function findInDocblock($docComment, ReflectionClass $declaringClass): a } /** + * @param string|false $fileName false only for an internal class, which carries no docblocks * @return array imported class or namespace name by its lower-case alias */ - private function readImports(ReflectionClass $class): array + private function readImports($fileName): array { - // only internal classes have no file, and they carry no docblocks, so this method never sees one - $fileName = $class->getFileName(); - preg_match_all( - '/^\s*use\s+(\\\\?[A-Za-z_][A-Za-z0-9_\\\\]*)(?:\s+as\s+([A-Za-z_][A-Za-z0-9_]*))?\s*;/mi', - $fileName === false ? '' : (string)file_get_contents($fileName), - $matches, - PREG_SET_ORDER - ); + $key = (string)$fileName; + if (!isset($this->importsByFile[$key])) { + $this->importsByFile[$key] = $this->parseImports( + $fileName === false ? '' : (string)file_get_contents($fileName) + ); + } + + return $this->importsByFile[$key]; + } + + /** + * The `use` statements of a file: one name, "as" aliases, several names separated by commas, and a group + * ("use A\{B, C as D};"); function and constant imports are left out. A trait's `use` inside a class body can add a + * harmless alias of the trait's own name. + * + * @return array + */ + private function parseImports(string $source): array + { + preg_match_all('/(?:^|;)\s*use\s+(?!function\s|const\s)([\\\\A-Za-z_][^;]*);/mi', $source, $statements); $imports = []; - foreach ($matches as $match) { - $importedName = ltrim($match[1], '\\'); - $parts = explode('\\', $importedName); - $alias = isset($match[2]) && $match[2] !== '' ? $match[2] : end($parts); - $imports[strtolower($alias)] = $importedName; + foreach ($statements[1] as $statement) { + $prefix = ''; + if (preg_match('/^([^{]*)\{([^}]*)\}\s*$/', trim($statement), $group) === 1) { + $prefix = trim($group[1]); + $statement = $group[2]; + } + foreach (explode(',', $statement) as $clause) { + $pattern = '/^\s*\\\\?([A-Za-z_][A-Za-z0-9_\\\\]*)(?:\s+as\s+([A-Za-z_][A-Za-z0-9_]*))?\s*$/i'; + if (preg_match($pattern, $clause, $match) !== 1) { + continue; + } + $importedName = ltrim($prefix . $match[1], '\\'); + $parts = explode('\\', $importedName); + $alias = isset($match[2]) && $match[2] !== '' ? $match[2] : end($parts); + $imports[strtolower($alias)] = $importedName; + } } return $imports; @@ -80,19 +126,28 @@ private function readImports(ReflectionClass $class): array /** * @param array $imports + * @return string|null the bundle annotation class the tag names, or null when it names none */ - private function resolveClassName(string $name, array $imports, string $namespace): string + private function resolveClassName(string $name, array $imports, string $namespace): ?string { if ($name[0] === '\\') { - return ltrim($name, '\\'); + $candidates = [substr($name, 1)]; + } else { + $parts = explode('\\', $name, 2); + $alias = strtolower($parts[0]); + $candidates = isset($imports[$alias]) + ? [$imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : '')] + : [($namespace === '' ? '' : $namespace . '\\') . $name, $name]; } - $parts = explode('\\', $name, 2); - $alias = strtolower($parts[0]); - if (isset($imports[$alias])) { - return $imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : ''); + foreach ($candidates as $candidate) { + if (class_exists($candidate) || interface_exists($candidate)) { + return is_subclass_of($candidate, RestAnnotationInterface::class) + ? (new ReflectionClass($candidate))->getName() + : null; + } } - return $namespace === '' ? $name : $namespace . '\\' . $name; + return null; } } diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index 43ca860..ca64406 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -18,6 +18,9 @@ */ class RoutingAttributeLoader extends AttributeRouteControllerLoader { + private const ANNOTATION_NAMESPACE = 'Paysera\\Bundle\\ApiBundle\\Annotation\\'; + private const ATTRIBUTE_NAMESPACE = 'Paysera\\Bundle\\ApiBundle\\Attribute\\'; + /** * @var RestRequestHelper */ @@ -33,6 +36,11 @@ class RoutingAttributeLoader extends AttributeRouteControllerLoader */ private $attributeOptionsBuilder; + /** + * @var DocblockAnnotationFinder|null + */ + private $docblockAnnotationFinder; + public function setRequestHelper(RestRequestHelper $restRequestHelper) { $this->restRequestHelper = $restRequestHelper; @@ -105,26 +113,32 @@ private function loadAnnotations(Route $route, ReflectionClass $class, Reflectio */ private function refuseDocblockAnnotations(ReflectionClass $class, ReflectionMethod $method): void { - $annotations = (new DocblockAnnotationFinder())->findBundleAnnotations($class, $method); + if ($this->docblockAnnotationFinder === null) { + $this->docblockAnnotationFinder = new DocblockAnnotationFinder(); + } + $annotations = $this->docblockAnnotationFinder->findBundleAnnotations($class, $method); if ($annotations === []) { return; } - $attributes = []; + $replacements = []; foreach ($annotations as $annotation) { - $attributes[] = sprintf( - '#[%s]', - str_replace('\\Annotation\\', '\\Attribute\\', $annotation) - ); + $replacements[] = strpos($annotation, self::ANNOTATION_NAMESPACE) === 0 + ? '#[\\' . self::ATTRIBUTE_NAMESPACE . substr($annotation, strlen(self::ANNOTATION_NAMESPACE)) . ']' + : sprintf( + 'an attribute implementing \\%s in place of \\%s', + RestAttributeInterface::class, + $annotation + ); } throw new ConfigurationException(sprintf( - '%s::%s() configures its REST endpoint with docblock annotations (%s), which Symfony 7 does not read, ' - . 'so the endpoint would run without those options. Use the attributes instead: %s.', + '%s::%s() uses docblock annotations of paysera/lib-api-bundle (\\%s). Symfony 7 does not read docblock ' + . 'annotations, so they would have no effect. Use the PHP attributes instead: %s.', $class->getName(), $method->getName(), - implode(', ', $annotations), - implode(', ', $attributes) + implode(', \\', $annotations), + implode(', ', $replacements) )); } diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 9aee6e6..0ef7499 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -14,7 +14,14 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AliasedImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AttributeOnlyController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildWithoutImports; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommaImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ControllerUsingTheTrait; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomAnnotationOnAttributeRouteController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DirectImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionMethod; @@ -39,6 +46,9 @@ public function testFindsTheBundleAnnotationsOfAMethodAndItsClass( ); } + /** + * @return array + */ public static function controllerDataProvider(): array { return [ @@ -62,6 +72,36 @@ public static function controllerDataProvider(): array 'inherited', [ResponseNormalization::class], ], + 'a method from a trait resolves through the trait file imports' => [ + ControllerUsingTheTrait::class, + 'fromTrait', + [Query::class], + ], + 'a group import with an alias' => [ + GroupImportController::class, + 'create', + [RequiredPermissions::class, Body::class], + ], + 'a comma-separated import over two lines' => [ + CommaImportController::class, + 'create', + [RequiredPermissions::class, Body::class], + ], + 'an import on the namespace line, and a full name without the leading backslash' => [ + SameLineImportController::class, + 'create', + [Body::class, RequiredPermissions::class], + ], + 'only top-level annotations count, as Doctrine reads them' => [ + NotTopLevelAnnotationsController::class, + 'find', + [Query::class], + ], + 'an application\'s own annotation class in the same namespace' => [ + CustomAnnotationOnAttributeRouteController::class, + 'show', + [CustomRestAnnotation::class], + ], ]; } } diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/CommaImportController.php b/tests/Unit/Service/RoutingLoader/Fixtures/CommaImportController.php new file mode 100644 index 0000000..91bdca1 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/CommaImportController.php @@ -0,0 +1,19 @@ +load(DocblockOptionsOnAttributeRouteController::class); $this->fail('The route with the bundle\'s docblock annotations was loaded without them'); } catch (ConfigurationException $exception) { - $this->assertStringContainsString( - DocblockOptionsOnAttributeRouteController::class . '::show() configures its REST endpoint with ' - . 'docblock annotations (' . RequiredPermissions::class . ')', - $exception->getMessage() - ); - $this->assertStringContainsString( - 'Use the attributes instead: #[Paysera\Bundle\ApiBundle\Attribute\RequiredPermissions].', + $this->assertSame( + DocblockOptionsOnAttributeRouteController::class . '::show() uses docblock annotations of ' + . 'paysera/lib-api-bundle (\\' . RequiredPermissions::class . '). Symfony 7 does not read docblock ' + . 'annotations, so they would have no effect. Use the PHP attributes instead: ' + . '#[\\Paysera\\Bundle\\ApiBundle\\Attribute\\RequiredPermissions].', $exception->getMessage() ); } } + public function testNamesTheAttributeInterfaceForAnApplicationsOwnAnnotation() + { + $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); + $loader = $this->createLoader(); + + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage( + 'Use the PHP attributes instead: an attribute implementing ' + . '\\Paysera\\Bundle\\ApiBundle\\Attribute\\RestAttributeInterface in place of \\' + . CustomRestAnnotation::class . '.' + ); + + $loader->load(CustomAnnotationOnAttributeRouteController::class); + } + public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() { $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); From 1be2b39e38ed50d85427a9c2841412b107ebd8e4 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:51:20 +0530 Subject: [PATCH 14/42] EE-283 Read Accept-Language the way Symfony 3.4 to 7.0 did The locale listener still took the header's languages from Request::getLanguages(). Symfony 7.1 rewrote that method too (it lowercases and reformats the tags), so on 7.1 and later a client sending "DE" got German where every earlier line kept the default, and "EN,de" got English instead of German. On http-foundation releases that build that list from the header's array keys (3.4, 4.4.0 to 4.4.45, 5.4.0 to 5.4.12, 6.0.0 to 6.0.12, 6.1.0 to 6.1.4) a numeric tag came back as an int and the listener's strpos() threw a TypeError under strict types, where the old listener returned a locale. The listener now reads the header through AcceptHeader, whose order is the same on every line, and writes the tags the way getLanguages() did up to 7.0 ("de-CH" becomes "de_CH", a tag without a region keeps its case). Five cases are added: "DE" and "EN,de" fail on Symfony 7.4 with the previous listener, "1" and "de, 1" throw on http-foundation 4.4.0 with it, and "i-cherokee" runs the i- branch. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 35 ++++++++++++++++++++-- tests/Unit/Listener/LocaleListenerTest.php | 30 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index 2e4863e..5e211ea 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -4,6 +4,7 @@ namespace Paysera\Bundle\ApiBundle\Listener; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; +use Symfony\Component\HttpFoundation\AcceptHeader; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; @@ -48,11 +49,11 @@ public function onKernelRequest($event) * * This is the rule Request::getPreferredLanguage() applied up to Symfony 7.0. Symfony 7.1 changed it, and passing a * placeholder for "no match" stopped working there ("default" starts with "de"), so the listener matches itself and - * picks the same locale on every Symfony line. + * picks the same locale on every Symfony line, from a header it reads itself (readLanguages()). */ private function resolveFromHeaders(Request $request): ?string { - $languages = $request->getLanguages(); + $languages = $this->readLanguages($request); $candidates = []; foreach ($languages as $language) { $candidates[] = $language; @@ -74,4 +75,34 @@ private function resolveFromHeaders(Request $request): ?string return null; } + + /** + * The Accept-Language tags in the client's order of preference, written the way Request::getLanguages() wrote them up + * to Symfony 7.0: "de-CH" becomes "de_CH", and a tag without a region keeps its case. Symfony 7.1 changed that + * formatting too, so reading the header here keeps the result the same on every Symfony line. + * + * @return string[] + */ + private function readLanguages(Request $request): array + { + $languages = []; + foreach (AcceptHeader::fromString($request->headers->get('Accept-Language'))->all() as $item) { + $language = $item->getValue(); + if (strpos($language, '-') !== false) { + $codes = explode('-', $language); + if ($codes[0] === 'i') { + // a language registered with the i- prefix, such as i-cherokee + $language = $codes[1]; + } else { + $language = strtolower($codes[0]); + for ($i = 1, $count = count($codes); $i < $count; $i++) { + $language .= '_' . strtoupper($codes[$i]); + } + } + } + $languages[] = $language; + } + + return $languages; + } } diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 9946670..728591d 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -126,6 +126,36 @@ public function provider() '', true, ], + 'a tag without a region keeps its case, as Symfony 3.4 to 7.0 read it' => [ + 'unchanged', + ['de'], + 'DE', + true, + ], + 'a wrong-case tag does not match before a matching one' => [ + 'de', + ['en', 'lt', 'de'], + 'EN,de', + true, + ], + 'a numeric tag is not a language' => [ + 'unchanged', + ['de'], + '1', + true, + ], + 'a numeric tag next to a language' => [ + 'de', + ['de'], + 'de, 1', + true, + ], + 'a language registered with the i- prefix' => [ + 'cherokee', + ['en', 'cherokee'], + 'i-cherokee', + true, + ], ]; } } From 07e8bd57a18c8b4d7e5cb9f0b7f6c1717b6e0043 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:55:49 +0530 Subject: [PATCH 15/42] EE-283 Choose the test application's Symfony 7 setup by what routing supports The test application picked its Symfony 7 configuration by the http-kernel major, while the bundle decides by what symfony/routing supports. Composer can install routing 7.4 next to framework-bundle 6.4 (framework-bundle 6.4 allows it), and there the application kept importing its controllers with the "annotation" route type that routing 7 no longer loads: 144 tests failed, on master as well. It now asks routing whether its attribute loader still has an annotation reader, the question the bundle's own loader asks. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Fixtures/FixtureTestBundle/Service/TestHelper.php | 8 +++++--- tests/Functional/FunctionalTestCase.php | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php index abaf8f6..b6933da 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php @@ -5,7 +5,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; -use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\Routing\Loader\AttributeClassLoader; class TestHelper { @@ -15,10 +15,12 @@ public static function phpAttributeSupportExists(): bool } /** - * Symfony 7 removed the Doctrine annotation reader from routing: @Route docblocks are no longer read. + * Whether the installed symfony/routing still reads docblock annotations: its attribute loader lost the annotation + * reader in 7.0. The bundle decides the same way (RoutingAttributeLoader), so a 6.4 framework next to routing 7 is + * tested as the Symfony 7 case it is. */ public static function docblockRoutingSupportExists(): bool { - return Kernel::MAJOR_VERSION < 7; + return !class_exists(AttributeClassLoader::class) || property_exists(AttributeClassLoader::class, 'reader'); } } diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index dbe494a..79be1b6 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -5,6 +5,7 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Tools\SchemaTool; +use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service\TestHelper; use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\TestKernel; use PHPUnit\Framework\TestCase; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; @@ -34,7 +35,7 @@ protected function setUpContainer($testCase, $commonFile = 'common.yml') $prefix = ''; if (Kernel::MAJOR_VERSION <= 4) { $prefix = 'legacy_'; - } elseif (Kernel::MAJOR_VERSION >= 7) { + } elseif (!TestHelper::docblockRoutingSupportExists()) { $prefix = 'sf7_'; } $this->kernel = new TestKernel($testCase, $prefix . $commonFile); From 4d886f156c7f70b4f8421b5a95bddd62dc2b08e0 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:58:11 +0530 Subject: [PATCH 16/42] EE-283 Set the email validation mode Symfony 7 uses in the Symfony 7 test setup framework-bundle 6.4 installs next to validator 7.4, and it still configures the Email validator with the "loose" mode validator 7 removed, so every request validating an e-mail failed with "The defaultMode parameter value is not valid". The Symfony 7 test configuration now names html5, validator 7's own default: the five validation tests pass on that mix, and nothing changes on a pure Symfony 7 install. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Functional/Fixtures/config/sf7_common.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Functional/Fixtures/config/sf7_common.yml b/tests/Functional/Fixtures/config/sf7_common.yml index 8cf8d69..5a42048 100644 --- a/tests/Functional/Fixtures/config/sf7_common.yml +++ b/tests/Functional/Fixtures/config/sf7_common.yml @@ -1,6 +1,9 @@ framework: router: resource: '%kernel.project_dir%/tests/Functional/Fixtures/config/sf7_routing.yml' + validation: + # Symfony 7's default; framework-bundle 6.4 still defaults to "loose", which validator 7 no longer has + email_validation_mode: html5 security: firewalls: From 8340227954cb4f65fad9fe9d20fa73db7f7362be Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 21:59:44 +0530 Subject: [PATCH 17/42] EE-283 Make the new tests check the bundle's own configuration and reach their guards - ErrorBuilderTest builds paysera_api.error_builder from the bundle's own services.xml instead of repeating its eight configureError() calls, so a change to the configured error codes shows up. - The duplicate-annotation test passed on an earlier exception (its first annotation could not guess a type) and never reached the guard it is named after; both annotations now name their type, and the test expects the guard's message. - Element types on the new data providers, an imported RuntimeException, and the eight implicitly nullable test parameters PHP 8.4 reports written ?Type. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Functional/FunctionalAnnotationsTest.php | 6 +-- tests/Functional/FunctionalTestCase.php | 4 +- tests/Unit/Entity/UnsetOptionsTest.php | 3 ++ .../Listener/RestExceptionListenerTest.php | 2 +- .../Normalizer/PagedQueryNormalizerTest.php | 2 +- tests/Unit/Service/ErrorBuilderTest.php | 39 ++++++------------- .../ReflectionMethodWrapperTest.php | 2 +- ...estRequestAnnotationOptionsBuilderTest.php | 6 ++- .../Validation/EntityValidatorTest.php | 3 +- 9 files changed, 28 insertions(+), 39 deletions(-) diff --git a/tests/Functional/FunctionalAnnotationsTest.php b/tests/Functional/FunctionalAnnotationsTest.php index 0c2ea54..bd04693 100644 --- a/tests/Functional/FunctionalAnnotationsTest.php +++ b/tests/Functional/FunctionalAnnotationsTest.php @@ -24,7 +24,7 @@ protected function setUp(): void public function testAnnotatedRestRequestConfiguration( Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ) { $this->makeTest('annotated', $expectedResponse, $request, $extraResponseVersion); } @@ -38,7 +38,7 @@ public function testAnnotatedRestRequestConfiguration( public function testAttributedRestRequestConfiguration( Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ) { $this->makeTest('attributed', $expectedResponse, $request, $extraResponseVersion); } @@ -47,7 +47,7 @@ private function makeTest( string $pathPrefix, Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ): void { if ($pathPrefix === 'attributed' && !TestHelper::phpAttributeSupportExists()) { $this->markTestSkipped('Unsupported environment'); diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index 79be1b6..58f14e2 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -63,9 +63,9 @@ protected function makeGetRequest(string $uri): Response protected function createRequest( string $method, string $uri, - string $content = null, + ?string $content = null, array $headers = [], - string $username = null + ?string $username = null ): Request { $parts = parse_url($uri); parse_str($parts['query'] ?? '', $query); diff --git a/tests/Unit/Entity/UnsetOptionsTest.php b/tests/Unit/Entity/UnsetOptionsTest.php index 3c6212e..441ed4d 100644 --- a/tests/Unit/Entity/UnsetOptionsTest.php +++ b/tests/Unit/Entity/UnsetOptionsTest.php @@ -27,6 +27,9 @@ public function testReadingAnUnsetOptionFails($options, string $getter, string $ $options->$getter(); } + /** + * @return array + */ public static function unsetOptionDataProvider(): array { return [ diff --git a/tests/Unit/Listener/RestExceptionListenerTest.php b/tests/Unit/Listener/RestExceptionListenerTest.php index 3bdc879..f042855 100644 --- a/tests/Unit/Listener/RestExceptionListenerTest.php +++ b/tests/Unit/Listener/RestExceptionListenerTest.php @@ -29,7 +29,7 @@ class RestExceptionListenerTest extends MockeryTestCase * @param int $statusCode * @param string|null $logLevel */ - public function testOnKernelException(bool $restRequest, int $statusCode = 400, string $logLevel = null) + public function testOnKernelException(bool $restRequest, int $statusCode = 400, ?string $logLevel = null) { $helper = Mockery::mock(RestRequestHelper::class); $errorBuilder = Mockery::mock(ErrorBuilderInterface::class); diff --git a/tests/Unit/Normalizer/PagedQueryNormalizerTest.php b/tests/Unit/Normalizer/PagedQueryNormalizerTest.php index 9cf702a..a912137 100644 --- a/tests/Unit/Normalizer/PagedQueryNormalizerTest.php +++ b/tests/Unit/Normalizer/PagedQueryNormalizerTest.php @@ -85,7 +85,7 @@ public function testNormalize( array $explicitlyIncluded, string $defaultStrategy, string $queryStrategy, - int $maximumOffset = null + ?int $maximumOffset = null ) { $normalizationContext = Mockery::mock(NormalizationContext::class); diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php index bcf186c..aa8b386 100644 --- a/tests/Unit/Service/ErrorBuilderTest.php +++ b/tests/Unit/Service/ErrorBuilderTest.php @@ -14,6 +14,9 @@ use Paysera\Pagination\Exception\TooLargeOffsetException; use PHPUnit\Framework\TestCase; use RuntimeException; +use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -25,8 +28,8 @@ use Throwable; /** - * The error every REST endpoint answers with, for each kind of exception, with the error codes the bundle configures in - * Resources/config/services.xml. + * The error every REST endpoint answers with, for each kind of exception, built by the bundle's own + * paysera_api.error_builder service definition (Resources/config/services.xml) with the error codes it configures. */ class ErrorBuilderTest extends TestCase { @@ -47,6 +50,9 @@ public function testBuildsTheErrorForEachKindOfException( ); } + /** + * @return array + */ public static function exceptionDataProvider(): array { $tooLargeOffset = new TooLargeOffsetException(1000, 1001); @@ -185,32 +191,9 @@ public function testCarriesTheApiExceptionDetails() private function createConfiguredErrorBuilder(): ErrorBuilder { - $errorBuilder = new ErrorBuilder(); - $errorBuilder->configureError('invalid_request', 400, 'Request content is invalid'); - $errorBuilder->configureError( - 'invalid_parameters', - 400, - 'Some required parameter is missing or it\'s format is invalid' - ); - $errorBuilder->configureError( - 'invalid_state', - 409, - 'Requested action cannot be made to the current state of resource' - ); - $errorBuilder->configureError( - 'unauthorized', - 401, - 'You have not provided any credentials or they are invalid' - ); - $errorBuilder->configureError( - 'forbidden', - 403, - 'You have no rights to access requested resource or make requested action' - ); - $errorBuilder->configureError('not_found', 404, 'Resource was not found'); - $errorBuilder->configureError('internal_server_error', 500, 'Unexpected internal system error'); - $errorBuilder->configureError('not_acceptable', 406, 'Unknown request or response format'); + $container = new ContainerBuilder(); + (new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../src/Resources/config')))->load('services.xml'); - return $errorBuilder; + return $container->get('paysera_api.error_builder'); } } diff --git a/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php b/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php index 030e782..dce8753 100644 --- a/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php +++ b/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php @@ -58,7 +58,7 @@ public function testGetFriendlyName(): void ); } - public function fixtureMethod(string $param1, DateTime $param2 = null, $param3 = null): string + public function fixtureMethod(string $param1, ?DateTime $param2 = null, $param3 = null): string { return $param1; } diff --git a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php index a36d451..4429736 100644 --- a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php +++ b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php @@ -68,10 +68,12 @@ public function testBuildOptionsWithSeveralUnsupportedAnnotations(): void $builder = new RestRequestAnnotationOptionsBuilder($optionsValidator); $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage('Only one annotation of type ' . Body::class . ' is supported'); + // each Body names its type and optionality, so applying the first one succeeds and the second meets the guard $builder->buildOptions([ - new Body(['parameterName' => 'a']), - new Body(['parameterName' => 'b']), + new Body(['parameterName' => 'a', 'denormalizationType' => 'type_a', 'optional' => false]), + new Body(['parameterName' => 'b', 'denormalizationType' => 'type_b', 'optional' => false]), ], new ReflectionMethod(self::class, 'fixtureMethod')); } diff --git a/tests/Unit/Service/Validation/EntityValidatorTest.php b/tests/Unit/Service/Validation/EntityValidatorTest.php index cb1fc28..6574634 100644 --- a/tests/Unit/Service/Validation/EntityValidatorTest.php +++ b/tests/Unit/Service/Validation/EntityValidatorTest.php @@ -9,6 +9,7 @@ use Paysera\Bundle\ApiBundle\Exception\ApiException; use Paysera\Bundle\ApiBundle\Service\Validation\EntityValidator; use Paysera\Bundle\ApiBundle\Service\Validation\PropertyPathConverterInterface; +use RuntimeException; use Mockery\Adapter\Phpunit\MockeryTestCase; use stdClass; use Symfony\Component\Validator\Constraints\Type; @@ -23,7 +24,7 @@ public function testValidateRequiresTheValidator() { $entityValidator = new EntityValidator(null, Mockery::mock(PropertyPathConverterInterface::class)); - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); $this->expectExceptionMessage('To use validation in RestBundle you must configure framework.validation'); $entityValidator->validate(new stdClass(), new ValidationOptions()); From 21590eb8cfd2df94dc33b83a26a42677bf3f88c1 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 22:00:06 +0530 Subject: [PATCH 18/42] EE-283 Say in the README how endpoints are configured on Symfony 7 Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index eec01c8..20da09c 100644 --- a/README.md +++ b/README.md @@ -645,6 +645,13 @@ cursor and iterating this way until we have `"has_previous": false` is a reliabl ## Annotations/Attributes reference +Every option below exists as a docblock annotation (`Paysera\Bundle\ApiBundle\Annotation\*`) and, since 1.8.0, as a PHP +attribute of the same name (`Paysera\Bundle\ApiBundle\Attribute\*`, read on Symfony 6.4 and later). On Symfony 7 use the +attributes: Symfony 7 reads no docblock annotations, so loading a route whose controller still uses the bundle's docblock +annotations fails with an error naming the attributes to use, and the routes themselves need `#[Route]` with +`type: attribute` imports. The attributes take the same options, passed by name: +`@RequiredPermissions(permissions={"ROLE_ADMIN"})` becomes `#[RequiredPermissions(permissions: ['ROLE_ADMIN'])]`. + ### `Body` Instructs to convert request body into an object and pass to the controller as an argument. From cbfb8e33549d65a54699331840ddb7133d26a381 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 23:08:39 +0530 Subject: [PATCH 19/42] EE-283 Find docblock annotations on Symfony 7 where Doctrine's reader found them The Symfony 7 check still missed annotations that Doctrine's reader applies on Symfony 4.4 to 6.4, so those routes loaded on Symfony 7 with their options dropped, and it reported text that Doctrine never read as an annotation. - Imports: the use statements are read with the PHP tokenizer, as Doctrine's TokenParser does, from the class's file up to the class and from the class's namespace declaration on. Comments inside use statements, several statements on one line and braced or repeated namespace blocks now count as Doctrine counts them; a commented-out import, a trait used in the class body and another namespace's import no longer do. A method from a trait also gets the imports of the trait's file, as in Doctrine's getMethodImports(). - Docblocks: reading starts at the first "@" after a space, a tab or "*"; after that an annotation starts at an "@" after whitespace (a no-break space too) or "*"; a quoted string is text; a name followed by "-" is not an annotation; the arguments of an annotation are skipped, so an annotation nested in them is not reported as the method's own. On 19 sample controllers the check now reports what Doctrine's AnnotationReader reports, with doctrine/lexer 2 and 3; with lexer 1 the no-break space case differs, because lexer 1 does not read it as whitespace. On 207 controller files of Paysera applications (688 docblocks) the imports and the annotations found are the same as Doctrine's. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 248 ++++++++++++------ .../DocblockAnnotationFinderTest.php | 66 ++++- .../Fixtures/CommentedImportsController.php | 25 ++ .../ImportsBeforeTheClassController.php | 24 ++ .../Fixtures/NoBreakSpaceController.php | 20 ++ .../NotTopLevelAnnotationsController.php | 33 ++- .../RoutingLoader/Fixtures/Traits/Query.php | 12 + .../TwoImportsOnOneLineController.php | 18 ++ .../Fixtures/TwoNamespacesController.php | 19 ++ 9 files changed, 385 insertions(+), 80 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CommentedImportsController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportsBeforeTheClassController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NoBreakSpaceController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Traits/Query.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TwoImportsOnOneLineController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TwoNamespacesController.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index afebc56..8f2bab5 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -10,18 +10,37 @@ /** * Finds the annotations for this bundle in the docblocks of a controller method and its class without an annotation - * reader. Each docblock tag is resolved the way Doctrine's reader resolves it on Symfony 4.4 to 6.4: through the `use` - * imports of the declaring class's file and of the file that declares the method (a trait's), then relative to the - * class's namespace, then as a fully qualified name. + * reader. It follows the rules of Doctrine's reader, which applied them on Symfony 4.4 to 6.4: + * - the imports are the `use` statements of the class's file above the class, in the class's namespace; a method + * declared in a trait also gets those of the trait's file (Doctrine's PhpParser, TokenParser, getMethodImports()); + * - reading starts at the first "@" after a space, a tab or "*"; from there an annotation starts at an "@" after + * whitespace or "*", a quoted string is text, a name followed by "-" is not an annotation, and the arguments of an + * annotation are skipped (Doctrine's DocParser and DocLexer); + * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name. + * Unlike Doctrine, it does not pass over the tag names Doctrine ignores (param, return and so on) when a class has + * that name. * * @internal */ class DocblockAnnotationFinder { + private const NAME = '[a-z_\\\\][a-z0-9_:\\\\]*[a-z_][a-z0-9_]*|[a-z_]'; + + /** + * A quoted string, which is one token, or an "@" at the start or after whitespace or "*" with the name right after + * it (group 1) and, when the name is followed by "-", that "-" (group 2). + */ + private const TOKEN_PATTERN = '/"(?:""|[^"])*+"|(?> imports by file name + * @var array> imports by class name */ - private $importsByFile = []; + private $importsByClass = []; /** * @return string[] class names of the bundle annotations the docblocks use, in order, each once @@ -29,13 +48,16 @@ class DocblockAnnotationFinder public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $method): array { $declaringClass = $method->getDeclaringClass(); + $methodImports = $this->readImports($declaringClass); + foreach ($declaringClass->getTraits() as $trait) { + if ($trait->hasMethod($method->getName()) && $trait->getFileName() === $method->getFileName()) { + $methodImports = array_merge($methodImports, $this->readImports($trait)); + } + } + $found = array_merge( - $this->findInDocblock($class->getDocComment(), [$class->getFileName()], $class->getNamespaceName()), - $this->findInDocblock( - $method->getDocComment(), - [$declaringClass->getFileName(), $method->getFileName()], - $declaringClass->getNamespaceName() - ) + $this->findInDocblock($class->getDocComment(), $this->readImports($class), $class->getNamespaceName()), + $this->findInDocblock($method->getDocComment(), $methodImports, $declaringClass->getNamespaceName()) ); return array_values(array_unique($found)); @@ -43,81 +65,161 @@ public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $ /** * @param string|false $docComment - * @param array $fileNames the files whose imports apply; a later file's import wins + * @param array $imports * @return string[] */ - private function findInDocblock($docComment, array $fileNames, string $namespace): array + private function findInDocblock($docComment, array $imports, string $namespace): array { - if ($docComment === false || strpos($docComment, '@') === false) { + if ($docComment === false || preg_match('/[ \t*]@/', $docComment, $start, PREG_OFFSET_CAPTURE) !== 1) { return []; } - // a top-level annotation, as Doctrine's lexer reads it: "@" at the start or after whitespace or "*", so - // "{@inheritdoc}", "mail@host" and an annotation nested in another annotation's arguments do not count - preg_match_all( - '/(?:^|[\s*])@(\\\\?[A-Za-z_][A-Za-z0-9_]*(?:\\\\[A-Za-z_][A-Za-z0-9_]*)*)/', - $docComment, - $matches - ); - $imports = []; - foreach (array_unique($fileNames) as $fileName) { - $imports = array_merge($imports, $this->readImports($fileName)); - } - + $text = substr($docComment, $start[0][1] + 1); $found = []; - foreach ($matches[1] as $name) { - $className = $this->resolveClassName($name, $imports, $namespace); - if ($className !== null) { + $offset = 0; + while (preg_match(self::TOKEN_PATTERN, $text, $token, PREG_OFFSET_CAPTURE, $offset) === 1) { + $offset = $token[0][1] + strlen($token[0][0]); + $isString = !isset($token[1]); + $isFollowedByDash = isset($token[2]); + if ($isString || $isFollowedByDash) { + continue; + } + + $className = $this->resolveClassName($token[1][0], $imports, $namespace); + if ($className === null) { + continue; + } + if (is_subclass_of($className, RestAnnotationInterface::class)) { $found[] = $className; } + if ($this->isAnnotationClass($className) + && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 + ) { + // Doctrine reads an annotation's arguments, so an "@" in them is a nested annotation or text + $offset += strlen($arguments[0]); + } } return $found; } /** - * @param string|false $fileName false only for an internal class, which carries no docblocks + * @param array $imports + * @return string|null the class the name refers to, or null when there is none + */ + private function resolveClassName(string $name, array $imports, string $namespace): ?string + { + if ($name[0] === '\\') { + $candidates = [$name]; + } else { + $parts = explode('\\', $name, 2); + $alias = strtolower($parts[0]); + $candidates = isset($imports[$alias]) + ? [$imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : '')] + : [$namespace . '\\' . $name, $name]; + } + + foreach ($candidates as $candidate) { + if (class_exists($candidate)) { + return (new ReflectionClass($candidate))->getName(); + } + } + + return null; + } + + private function isAnnotationClass(string $className): bool + { + return strpos((string)(new ReflectionClass($className))->getDocComment(), '@Annotation') !== false; + } + + /** * @return array imported class or namespace name by its lower-case alias */ - private function readImports($fileName): array + private function readImports(ReflectionClass $class): array { - $key = (string)$fileName; - if (!isset($this->importsByFile[$key])) { - $this->importsByFile[$key] = $this->parseImports( - $fileName === false ? '' : (string)file_get_contents($fileName) - ); + $className = $class->getName(); + if (!isset($this->importsByClass[$className])) { + $this->importsByClass[$className] = $this->parseImports($class); } - return $this->importsByFile[$key]; + return $this->importsByClass[$className]; } /** - * The `use` statements of a file: one name, "as" aliases, several names separated by commas, and a group - * ("use A\{B, C as D};"); function and constant imports are left out. A trait's `use` inside a class body can add a - * harmless alias of the trait's own name. + * The `use` statements of the class's file up to the class, from the class's namespace declaration on. So a + * trait's `use` in a class body does not count, nor does a commented-out import or another namespace's import. * * @return array */ - private function parseImports(string $source): array + private function parseImports(ReflectionClass $class): array { - preg_match_all('/(?:^|;)\s*use\s+(?!function\s|const\s)([\\\\A-Za-z_][^;]*);/mi', $source, $statements); + $fileName = $class->getFileName(); + if ($fileName === false || !is_file($fileName)) { + return []; + } + + $source = implode('', array_slice(file($fileName), 0, $class->getStartLine())); + $tokens = []; + foreach (token_get_all($source) as $token) { + if (!is_array($token) || !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { + $tokens[] = $token; + } + } $imports = []; - foreach ($statements[1] as $statement) { - $prefix = ''; - if (preg_match('/^([^{]*)\{([^}]*)\}\s*$/', trim($statement), $group) === 1) { - $prefix = trim($group[1]); - $statement = $group[2]; + foreach ($tokens as $index => $token) { + if ($token[0] === T_USE) { + $imports = array_merge($imports, $this->parseUseStatement($tokens, $index + 1)); + } elseif ($token[0] === T_NAMESPACE + && $this->readName($tokens, $index + 1) === $class->getNamespaceName() + ) { + $imports = []; } - foreach (explode(',', $statement) as $clause) { - $pattern = '/^\s*\\\\?([A-Za-z_][A-Za-z0-9_\\\\]*)(?:\s+as\s+([A-Za-z_][A-Za-z0-9_]*))?\s*$/i'; - if (preg_match($pattern, $clause, $match) !== 1) { - continue; + } + + return $imports; + } + + /** + * One `use` statement: a name, a name with "as", names separated by commas, or a group ("use A\{B, C as D};"). + * `use function`, `use const` and a closure's `use (...)` import no class, so they end at their first token. + * + * @param array $tokens + * @return array + */ + private function parseUseStatement(array $tokens, int $index): array + { + $imports = []; + $groupPrefix = ''; + $name = ''; + $alias = ''; + $isAliasNext = false; + for (; isset($tokens[$index]); $index++) { + $token = $tokens[$index]; + if ($this->isNameToken($token)) { + if ($isAliasNext) { + $alias = $token[1]; + } else { + $name .= $token[1]; + $parts = explode('\\', $token[1]); + $alias = end($parts); + } + } elseif ($token[0] === T_AS) { + $isAliasNext = true; + } elseif ($token === ',' || $token === ';') { + $imports[strtolower($alias)] = $groupPrefix . $name; + if ($token === ';') { + break; } - $importedName = ltrim($prefix . $match[1], '\\'); - $parts = explode('\\', $importedName); - $alias = isset($match[2]) && $match[2] !== '' ? $match[2] : end($parts); - $imports[strtolower($alias)] = $importedName; + $name = ''; + $alias = ''; + $isAliasNext = false; + } elseif ($token === '{') { + $groupPrefix = $name; + $name = ''; + } elseif ($token !== '}') { + break; } } @@ -125,29 +227,27 @@ private function parseImports(string $source): array } /** - * @param array $imports - * @return string|null the bundle annotation class the tag names, or null when it names none + * @param array $tokens */ - private function resolveClassName(string $name, array $imports, string $namespace): ?string + private function readName(array $tokens, int $index): string { - if ($name[0] === '\\') { - $candidates = [substr($name, 1)]; - } else { - $parts = explode('\\', $name, 2); - $alias = strtolower($parts[0]); - $candidates = isset($imports[$alias]) - ? [$imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : '')] - : [($namespace === '' ? '' : $namespace . '\\') . $name, $name]; + $name = ''; + for (; isset($tokens[$index]) && $this->isNameToken($tokens[$index]); $index++) { + $name .= $tokens[$index][1]; } - foreach ($candidates as $candidate) { - if (class_exists($candidate) || interface_exists($candidate)) { - return is_subclass_of($candidate, RestAnnotationInterface::class) - ? (new ReflectionClass($candidate))->getName() - : null; - } - } + return $name; + } - return null; + /** + * @param string|array{0: int, 1: string, 2: int} $token + */ + private function isNameToken($token): bool + { + return is_array($token) && ( + in_array($token[0], [T_STRING, T_NS_SEPARATOR], true) + // PHP 8 reads a qualified name as one token + || (PHP_VERSION_ID >= 80000 && in_array($token[0], [T_NAME_QUALIFIED, T_NAME_FULLY_QUALIFIED], true)) + ); } } diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 0ef7499..c026ae0 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -4,6 +4,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader; +use ArrayObject; use Paysera\Bundle\ApiBundle\Annotation\Body; use Paysera\Bundle\ApiBundle\Annotation\PathAttribute; use Paysera\Bundle\ApiBundle\Annotation\Query; @@ -15,13 +16,18 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AttributeOnlyController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildWithoutImports; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommaImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommentedImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ControllerUsingTheTrait; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomAnnotationOnAttributeRouteController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DirectImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportsBeforeTheClassController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NoBreakSpaceController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoImportsOnOneLineController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoNamespacesController; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionMethod; @@ -40,10 +46,12 @@ public function testFindsTheBundleAnnotationsOfAMethodAndItsClass( ) { $finder = new DocblockAnnotationFinder(); - $this->assertSame( - $expectedAnnotations, - $finder->findBundleAnnotations(new ReflectionClass($className), new ReflectionMethod($className, $methodName)) + $annotations = $finder->findBundleAnnotations( + new ReflectionClass($className), + new ReflectionMethod($className, $methodName) ); + + $this->assertSame($expectedAnnotations, $annotations); } /** @@ -92,11 +100,61 @@ public static function controllerDataProvider(): array 'create', [Body::class, RequiredPermissions::class], ], - 'only top-level annotations count, as Doctrine reads them' => [ + 'comments inside use statements' => [ + CommentedImportsController::class, + 'find', + [Query::class, RequiredPermissions::class, Validation::class], + ], + 'two use statements on one line' => [ + TwoImportsOnOneLineController::class, + 'create', + [Body::class, RequiredPermissions::class], + ], + 'only the imports above the class count: not a commented-out one, not a trait in the class body' => [ + ImportsBeforeTheClassController::class, + 'find', + [Query::class, RequiredPermissions::class], + ], + 'an import in another namespace block of the file does not count' => [ + TwoNamespacesController::class, + 'show', + [CustomRestAnnotation::class], + ], + 'an "@" right after a word or "{" does not start an annotation, and a nested one is part of its parent' => [ NotTopLevelAnnotationsController::class, 'find', [Query::class], ], + 'a nested annotation after a space is part of its parent too' => [ + NotTopLevelAnnotationsController::class, + 'findWithASpaceBeforeTheNestedAnnotation', + [Query::class], + ], + 'reading starts at the first "@" after a space, a tab or "*", as in Doctrine' => [ + NotTopLevelAnnotationsController::class, + 'annotationAtTheStartOfALine', + [], + ], + 'an "@" inside a quoted string is text' => [ + NotTopLevelAnnotationsController::class, + 'annotationInAString', + [], + ], + 'a name followed by "-" is not an annotation' => [ + NotTopLevelAnnotationsController::class, + 'annotationFollowedByADash', + [], + ], + 'a no-break space before an annotation is whitespace, as in Doctrine\'s lexer' => [ + NoBreakSpaceController::class, + 'show', + [RequiredPermissions::class], + ], + 'a class of PHP itself has no docblocks' => [ + ArrayObject::class, + 'count', + [], + ], 'an application\'s own annotation class in the same namespace' => [ CustomAnnotationOnAttributeRouteController::class, 'show', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/CommentedImportsController.php b/tests/Unit/Service/RoutingLoader/Fixtures/CommentedImportsController.php new file mode 100644 index 0000000..d7501fd --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/CommentedImportsController.php @@ -0,0 +1,25 @@ + Date: Wed, 23 Sep 2026 23:08:39 +0530 Subject: [PATCH 20/42] EE-283 Keep the locale listener from failing on a malformed Accept-Language item http-foundation 3.4 gives null for an item that is only ";" and false for a lone quote, and the listener passed that to strpos() under strict types: "Accept-Language: '" ended in a TypeError where 1.8.2 kept the locale. The value is read as a string again, as it was before the listener read the header itself. Two test cases cover a malformed item alone and next to a language. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 3 ++- tests/Unit/Listener/LocaleListenerTest.php | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index 5e211ea..6635a02 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -87,7 +87,8 @@ private function readLanguages(Request $request): array { $languages = []; foreach (AcceptHeader::fromString($request->headers->get('Accept-Language'))->all() as $item) { - $language = $item->getValue(); + // http-foundation 3.4 gives null or false for a malformed item, such as ";" or a lone quote + $language = (string)$item->getValue(); if (strpos($language, '-') !== false) { $codes = explode('-', $language); if ($codes[0] === 'i') { diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 728591d..c64ef1b 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -156,6 +156,18 @@ public function provider() 'i-cherokee', true, ], + 'a malformed item is not a language' => [ + 'unchanged', + ['de'], + "'", + true, + ], + 'a malformed item next to a language' => [ + 'de', + ['de'], + "de, '", + true, + ], ]; } } From 42b40090908bd0e45dbc4f4c529808d43cd30f31 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 23:08:39 +0530 Subject: [PATCH 21/42] EE-283 Check every error code the bundle configures ErrorBuilderTest covered six of the eight codes in services.xml. It now also checks invalid_parameters (with its configured message), invalid_state and not_acceptable, so a change to any configured status or message fails a test. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Service/ErrorBuilderTest.php | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php index aa8b386..d273ed3 100644 --- a/tests/Unit/Service/ErrorBuilderTest.php +++ b/tests/Unit/Service/ErrorBuilderTest.php @@ -71,6 +71,24 @@ public static function exceptionDataProvider(): array 404, 'Resource was not found', ], + 'API exception with the invalid parameters code' => [ + new ApiException(ApiException::INVALID_PARAMETERS), + 'invalid_parameters', + 400, + 'Some required parameter is missing or it\'s format is invalid', + ], + 'API exception with the invalid state code' => [ + new ApiException(ApiException::INVALID_STATE), + 'invalid_state', + 409, + 'Requested action cannot be made to the current state of resource', + ], + 'API exception with the not acceptable code' => [ + new ApiException(ApiException::NOT_ACCEPTABLE), + 'not_acceptable', + 406, + 'Unknown request or response format', + ], 'API exception with an unconfigured code' => [ new ApiException('unconfigured_code'), 'unconfigured_code', @@ -192,7 +210,8 @@ public function testCarriesTheApiExceptionDetails() private function createConfiguredErrorBuilder(): ErrorBuilder { $container = new ContainerBuilder(); - (new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../src/Resources/config')))->load('services.xml'); + $loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../src/Resources/config')); + $loader->load('services.xml'); return $container->get('paysera_api.error_builder'); } From b51b6642fd1d0d9bb2b991458d3c7396b352ab55 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Wed, 23 Sep 2026 23:11:07 +0530 Subject: [PATCH 22/42] EE-283 Say that the locale listener keeps the rule, not every result, across Symfony lines The comments said the listener picks the same locale on every Symfony line. It applies the same rule and tag format on every line; for some malformed headers the releases' own parsing of Accept-Language still differs (for example "de;q=, en"), as it did before this change. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index 6635a02..e039158 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -48,8 +48,8 @@ public function onKernelRequest($event) * regional variant (de_CH) also offers its primary language (de) unless the header lists that language itself. * * This is the rule Request::getPreferredLanguage() applied up to Symfony 7.0. Symfony 7.1 changed it, and passing a - * placeholder for "no match" stopped working there ("default" starts with "de"), so the listener matches itself and - * picks the same locale on every Symfony line, from a header it reads itself (readLanguages()). + * placeholder for "no match" stopped working there ("default" starts with "de"), so the listener applies the rule + * itself on every Symfony line, to a header it reads itself (readLanguages()). */ private function resolveFromHeaders(Request $request): ?string { @@ -79,7 +79,7 @@ private function resolveFromHeaders(Request $request): ?string /** * The Accept-Language tags in the client's order of preference, written the way Request::getLanguages() wrote them up * to Symfony 7.0: "de-CH" becomes "de_CH", and a tag without a region keeps its case. Symfony 7.1 changed that - * formatting too, so reading the header here keeps the result the same on every Symfony line. + * formatting too, so the listener writes the tags this way itself, on every Symfony line. * * @return string[] */ From c2263a9418e16c98c7bc5ca32546e56b3396da14 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 10:40:45 +0530 Subject: [PATCH 23/42] EE-283 Test the rest of the Symfony 7 check's rules against Doctrine's reader Six rules of the check ran in the suite without a test that fails when they break, or did not run at all. Each new case gives the result Doctrine's AnnotationReader gives (checked with doctrine/lexer 2 and 3), and each fails when its rule is taken out: - a `use function` or `use const` import with an annotation's short name does not replace the class import (the statement ends at `function` / `const`); - a method a class declares over its trait's method resolves through the class file's imports only, not the trait file's; - the parentheses after a class that is not an annotation are read, so an annotation in them counts, as in Doctrine; - reading starts at the first "@" after a space, a tab or "*": an annotation at the start of a line before it is skipped, and one after a tab starts the reading; - a class declared in evaluated code, such as a test double, has no file and gives no imports. The no-break space fixture moves into WhitespaceBeforeAnnotationController with the tab case. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinderTest.php | 47 +++++++++++++++++-- .../ControllerOverridingTheTraitMethod.php | 19 ++++++++ .../FunctionAndConstantImportsController.php | 19 ++++++++ .../NonAnnotationClassTagController.php | 17 +++++++ .../NotTopLevelAnnotationsController.php | 8 ++++ .../Fixtures/TraitWithAnotherQueryImport.php | 17 +++++++ ... WhitespaceBeforeAnnotationController.php} | 11 ++++- 7 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ControllerOverridingTheTraitMethod.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/FunctionAndConstantImportsController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NonAnnotationClassTagController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TraitWithAnotherQueryImport.php rename tests/Unit/Service/RoutingLoader/Fixtures/{NoBreakSpaceController.php => WhitespaceBeforeAnnotationController.php} (66%) diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index c026ae0..dc67d94 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -17,17 +17,20 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildWithoutImports; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommaImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommentedImportsController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ControllerOverridingTheTraitMethod; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ControllerUsingTheTrait; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomAnnotationOnAttributeRouteController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DirectImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\FunctionAndConstantImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportsBeforeTheClassController; -use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NoBreakSpaceController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NonAnnotationClassTagController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoImportsOnOneLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoNamespacesController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WhitespaceBeforeAnnotationController; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionMethod; @@ -54,6 +57,19 @@ public function testFindsTheBundleAnnotationsOfAMethodAndItsClass( $this->assertSame($expectedAnnotations, $annotations); } + public function testReadsAClassDeclaredInEvaluatedCode() + { + $testDouble = get_class($this->createMock(DirectImportController::class)); + $finder = new DocblockAnnotationFinder(); + + $annotations = $finder->findBundleAnnotations( + new ReflectionClass($testDouble), + new ReflectionMethod($testDouble, 'create') + ); + + $this->assertSame([], $annotations); + } + /** * @return array */ @@ -80,6 +96,11 @@ public static function controllerDataProvider(): array 'inherited', [ResponseNormalization::class], ], + 'a method the class declares over its trait\'s resolves through the class file imports only' => [ + ControllerOverridingTheTraitMethod::class, + 'find', + [Query::class], + ], 'a method from a trait resolves through the trait file imports' => [ ControllerUsingTheTrait::class, 'fromTrait', @@ -115,6 +136,11 @@ public static function controllerDataProvider(): array 'find', [Query::class, RequiredPermissions::class], ], + 'a function or constant import of the same name does not replace a class import' => [ + FunctionAndConstantImportsController::class, + 'show', + [RequiredPermissions::class], + ], 'an import in another namespace block of the file does not count' => [ TwoNamespacesController::class, 'show', @@ -135,6 +161,11 @@ public static function controllerDataProvider(): array 'annotationAtTheStartOfALine', [], ], + 'an annotation at the start of a line is skipped before the first one Doctrine reads' => [ + NotTopLevelAnnotationsController::class, + 'annotationAfterOneAtTheStartOfALine', + [Query::class], + ], 'an "@" inside a quoted string is text' => [ NotTopLevelAnnotationsController::class, 'annotationInAString', @@ -145,11 +176,21 @@ public static function controllerDataProvider(): array 'annotationFollowedByADash', [], ], - 'a no-break space before an annotation is whitespace, as in Doctrine\'s lexer' => [ - NoBreakSpaceController::class, + 'the parentheses after a class that is not an annotation are read, as in Doctrine' => [ + NonAnnotationClassTagController::class, 'show', [RequiredPermissions::class], ], + 'a no-break space before an annotation is whitespace, as in Doctrine\'s lexer' => [ + WhitespaceBeforeAnnotationController::class, + 'afterANoBreakSpace', + [RequiredPermissions::class], + ], + 'reading can start at an "@" after a tab' => [ + WhitespaceBeforeAnnotationController::class, + 'afterATab', + [RequiredPermissions::class], + ], 'a class of PHP itself has no docblocks' => [ ArrayObject::class, 'count', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/ControllerOverridingTheTraitMethod.php b/tests/Unit/Service/RoutingLoader/Fixtures/ControllerOverridingTheTraitMethod.php new file mode 100644 index 0000000..536e52b --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/ControllerOverridingTheTraitMethod.php @@ -0,0 +1,19 @@ + Date: Thu, 24 Sep 2026 10:40:45 +0530 Subject: [PATCH 24/42] EE-283 Name the finder's quoted-string pattern once and say when "-" keeps a name The quoted-string pattern was written in two regular expressions; it is now one constant. The class comment said a name followed by "-" is never an annotation, but a "-" that starts a number keeps it, in Doctrine and in the code. A comment line in the locale listener is rewrapped to the line length. No behaviour changes. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 4 ++-- .../RoutingLoader/DocblockAnnotationFinder.php | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index e039158..aaf051a 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -77,8 +77,8 @@ private function resolveFromHeaders(Request $request): ?string } /** - * The Accept-Language tags in the client's order of preference, written the way Request::getLanguages() wrote them up - * to Symfony 7.0: "de-CH" becomes "de_CH", and a tag without a region keeps its case. Symfony 7.1 changed that + * The Accept-Language tags in the client's order of preference, written the way Request::getLanguages() wrote them + * up to Symfony 7.0: "de-CH" becomes "de_CH", and a tag without a region keeps its case. Symfony 7.1 changed that * formatting too, so the listener writes the tags this way itself, on every Symfony line. * * @return string[] diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index 8f2bab5..b840ebc 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -14,8 +14,8 @@ * - the imports are the `use` statements of the class's file above the class, in the class's namespace; a method * declared in a trait also gets those of the trait's file (Doctrine's PhpParser, TokenParser, getMethodImports()); * - reading starts at the first "@" after a space, a tab or "*"; from there an annotation starts at an "@" after - * whitespace or "*", a quoted string is text, a name followed by "-" is not an annotation, and the arguments of an - * annotation are skipped (Doctrine's DocParser and DocLexer); + * whitespace or "*", a quoted string is text, a name followed by "-" is not an annotation (unless the "-" starts a + * number), and the arguments of an annotation are skipped (Doctrine's DocParser and DocLexer); * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name. * Unlike Doctrine, it does not pass over the tag names Doctrine ignores (param, return and so on) when a class has * that name. @@ -24,18 +24,26 @@ */ class DocblockAnnotationFinder { + /** + * A name as Doctrine's lexer reads it: letters, digits, "_", ":" and "\". + */ private const NAME = '[a-z_\\\\][a-z0-9_:\\\\]*[a-z_][a-z0-9_]*|[a-z_]'; + /** + * A quoted string, in which "" stands for a quote. + */ + private const STRING = '"(?:""|[^"])*+"'; + /** * A quoted string, which is one token, or an "@" at the start or after whitespace or "*" with the name right after - * it (group 1) and, when the name is followed by "-", that "-" (group 2). + * it (group 1) and, when the name is followed by "-" that does not start a number, that "-" (group 2). */ - private const TOKEN_PATTERN = '/"(?:""|[^"])*+"|(?> imports by class name From 30b797207eb29df67367eba23fa84a23e11e3c97 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 10:40:45 +0530 Subject: [PATCH 25/42] EE-283 Test an Accept-Language item that is only a semicolon On http-foundation 3.4 such an item gives null, which the listener must read as no language; the existing lone-quote cases cover only false. http-foundation 4.4 fails on an empty item itself, as with 1.8.2, so the test is skipped where Symfony's own parser throws. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Listener/LocaleListenerTest.php | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index c64ef1b..68bd154 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -8,10 +8,12 @@ use Paysera\Bundle\ApiBundle\Listener\LocaleListener; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Paysera\Bundle\ApiBundle\Tests\Unit\Helper\HttpKernelHelper; +use Symfony\Component\HttpFoundation\AcceptHeader; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; +use TypeError; class LocaleListenerTest extends MockeryTestCase { @@ -24,6 +26,25 @@ class LocaleListenerTest extends MockeryTestCase * @param bool $rest */ public function testOnKernelRequest(string $expectedLocale, array $locales, string $acceptLanguage, bool $rest) + { + $this->assertSame($expectedLocale, $this->resolveLocale($locales, $acceptLanguage, $rest)); + } + + public function testAnItemOfOnlyASemicolonIsNotALanguage() + { + try { + AcceptHeader::fromString(';'); + } catch (TypeError $error) { + $this->markTestSkipped('This http-foundation release fails on an empty Accept-Language item itself'); + } + + $this->assertSame('unchanged', $this->resolveLocale(['de'], ';', true)); + } + + /** + * @param string[] $locales + */ + private function resolveLocale(array $locales, string $acceptLanguage, bool $rest): string { $helper = Mockery::mock(RestRequestHelper::class); $kernel = Mockery::mock(HttpKernelInterface::class); @@ -42,7 +63,7 @@ public function testOnKernelRequest(string $expectedLocale, array $locales, stri $listener->onKernelRequest($event); - $this->assertSame($expectedLocale, $request->getLocale()); + return $request->getLocale(); } public function provider() From df9b48fbb67486d0b8ee7864dca87f60763f2d21 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 12:24:13 +0530 Subject: [PATCH 26/42] EE-283 Skip the semicolon-item test wherever Symfony's parser fails on it On http-foundation 4.4 the parser reads a value that is not there before it throws its TypeError, and PHP 7.4 reports that as a notice, PHP 8 as a warning. PHPUnit turns either into an error first, so the test errored on Symfony 4.4 with PHP 7.4 and later instead of being skipped. It now skips on any failure of Symfony's own parser. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Listener/LocaleListenerTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 68bd154..3d08248 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -13,7 +13,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; -use TypeError; +use Throwable; class LocaleListenerTest extends MockeryTestCase { @@ -34,7 +34,8 @@ public function testAnItemOfOnlyASemicolonIsNotALanguage() { try { AcceptHeader::fromString(';'); - } catch (TypeError $error) { + } catch (Throwable $error) { + // http-foundation 4.4 reads a missing value from the empty item: a notice or warning, then a TypeError $this->markTestSkipped('This http-foundation release fails on an empty Accept-Language item itself'); } From 442868865a14fd3bb5e5014b0a25b8e7b1ffd34e Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 12:24:13 +0530 Subject: [PATCH 27/42] EE-283 Keep reading where Doctrine's reader keeps reading, and test the rest of its rules Two inputs made the Symfony 7 check miss an annotation that Doctrine's reader applies on Symfony 4.4 to 6.4: - `@Target( @RequiredPermissions(...) )` where `Target` is an application's annotation class found through the namespace: Doctrine ignores tag names such as Target and Required unless they are imported or written in full, and then reads what the parentheses hold as top-level annotations. The check now skips an annotation's arguments only when its name was imported or written in full, where Doctrine always reads them. - an "@" right after a quote (`see "the notes"@RequiredPermissions(...)`): Doctrine's lexer measures a string without its quotes, so the "@" is not glued to it. New cases pin rules no test protected before, each checked against Doctrine's reader and each failing when its rule is taken out: an "@" right after a star, a one-letter alias, aliases in a comma list, an import written with a leading backslash, an alias two namespace levels up, an import on the class's own line, a docblock comment inside an import, arguments after a space, a parenthesis inside a quoted argument, an annotation without arguments before another, a ":" or a negative number after a name, a class docblock resolved in its own namespace and file, two classes in one file with their own namespace blocks, and a trait whose import wins over the class's for the trait's method. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 51 +++++++---- .../DocblockAnnotationFinderTest.php | 91 ++++++++++++++++++- .../Fixtures/AliasedImportController.php | 2 + .../Fixtures/ArgumentBoundariesController.php | 50 ++++++++++ .../ChildOfAParentInAnotherNamespace.php | 34 +++++++ .../Fixtures/ChildWithOwnImports.php | 14 +++ .../ClassLevelCustomAnnotationController.php | 15 +++ .../Fixtures/CommentedImportsController.php | 2 +- .../Fixtures/ControllerUsingTheTrait.php | 3 + .../Fixtures/CustomRestAnnotation.php | 2 +- .../Fixtures/IgnoredTagNameController.php | 17 ++++ .../ImportListWithAliasesController.php | 22 +++++ .../ImportOnTheClassLineController.php | 15 +++ .../NonAnnotationClassTagController.php | 8 ++ .../OtherNamespace/LocalRestAnnotation.php | 27 ++++++ .../RoutingLoader/Fixtures/Required.php | 12 +++ ...StarAndQuoteBeforeAnnotationController.php | 39 ++++++++ .../Service/RoutingLoader/Fixtures/Target.php | 18 ++++ 18 files changed, 402 insertions(+), 20 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ArgumentBoundariesController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildWithOwnImports.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/IgnoredTagNameController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportListWithAliasesController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportOnTheClassLineController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/OtherNamespace/LocalRestAnnotation.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Required.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/StarAndQuoteBeforeAnnotationController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Target.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index b840ebc..4d4a9eb 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -17,8 +17,8 @@ * whitespace or "*", a quoted string is text, a name followed by "-" is not an annotation (unless the "-" starts a * number), and the arguments of an annotation are skipped (Doctrine's DocParser and DocLexer); * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name. - * Unlike Doctrine, it does not pass over the tag names Doctrine ignores (param, return and so on) when a class has - * that name. + * Doctrine also passes over the tag names it ignores (Target, Required, param and so on) unless they are imported or + * written in full. This finder keeps no such list, so where it cannot tell, it reads on and reports more, never less. * * @internal */ @@ -35,10 +35,12 @@ class DocblockAnnotationFinder private const STRING = '"(?:""|[^"])*+"'; /** - * A quoted string, which is one token, or an "@" at the start or after whitespace or "*" with the name right after - * it (group 1) and, when the name is followed by "-" that does not start a number, that "-" (group 2). + * A quoted string, which is one token, or an "@" at the start or after whitespace, "*" or a quote, with the name + * right after it (group 1) and, when the name is followed by "-" that does not start a number, that "-" (group 2). + * Doctrine's lexer measures a token that starts with a quote without its quotes, so an "@" right after a quote is + * never glued to it. */ - private const TOKEN_PATTERN = '/' . self::STRING . '|(?resolveClassName($token[1][0], $imports, $namespace); + $name = $token[1][0]; + $importedName = $this->resolveImportedName($name, $imports); + $candidates = $importedName !== null ? [$importedName] : [$namespace . '\\' . $name, $name]; + $className = $this->findClass($candidates); if ($className === null) { continue; } if (is_subclass_of($className, RestAnnotationInterface::class)) { $found[] = $className; } - if ($this->isAnnotationClass($className) + // Doctrine reads the arguments of an annotation class named through an import or in full, so an "@" in them + // is a nested annotation or text. Found another way, the class may carry a name Doctrine ignores, and then + // Doctrine reads what follows as top-level annotations: so do not skip. + if ($importedName !== null + && $this->isAnnotationClass($className) && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 ) { - // Doctrine reads an annotation's arguments, so an "@" in them is a nested annotation or text $offset += strlen($arguments[0]); } } @@ -113,20 +121,29 @@ private function findInDocblock($docComment, array $imports, string $namespace): /** * @param array $imports - * @return string|null the class the name refers to, or null when there is none + * @return string|null the class name a fully qualified or imported name stands for, or null for any other name */ - private function resolveClassName(string $name, array $imports, string $namespace): ?string + private function resolveImportedName(string $name, array $imports): ?string { if ($name[0] === '\\') { - $candidates = [$name]; - } else { - $parts = explode('\\', $name, 2); - $alias = strtolower($parts[0]); - $candidates = isset($imports[$alias]) - ? [$imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : '')] - : [$namespace . '\\' . $name, $name]; + return $name; } + $parts = explode('\\', $name, 2); + $alias = strtolower($parts[0]); + if (!isset($imports[$alias])) { + return null; + } + + return $imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : ''); + } + + /** + * @param string[] $candidates class names in the order Doctrine tries them + * @return string|null the first that exists, as the class declares its name + */ + private function findClass(array $candidates): ?string + { foreach ($candidates as $candidate) { if (class_exists($candidate)) { return (new ReflectionClass($candidate))->getName(); diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index dc67d94..8f55562 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -13,8 +13,12 @@ use Paysera\Bundle\ApiBundle\Annotation\Validation; use Paysera\Bundle\ApiBundle\Service\RoutingLoader\DocblockAnnotationFinder; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AliasedImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ArgumentBoundariesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AttributeOnlyController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildOfAParentInAnotherNamespace; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildWithoutImports; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ChildWithOwnImports; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ClassLevelCustomAnnotationController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommaImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CommentedImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ControllerOverridingTheTraitMethod; @@ -24,10 +28,15 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DirectImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\FunctionAndConstantImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\IgnoredTagNameController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportListWithAliasesController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportOnTheClassLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportsBeforeTheClassController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NonAnnotationClassTagController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\OtherNamespace\LocalRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\StarAndQuoteBeforeAnnotationController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoImportsOnOneLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoNamespacesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WhitespaceBeforeAnnotationController; @@ -84,7 +93,7 @@ public static function controllerDataProvider(): array 'namespace alias, class alias and a fully qualified name' => [ AliasedImportController::class, 'find', - [Query::class, Validation::class, PathAttribute::class], + [Query::class, Validation::class, PathAttribute::class, ResponseNormalization::class], ], 'attributes, Symfony tags and attribute class names are not the annotations' => [ AttributeOnlyController::class, @@ -101,6 +110,26 @@ public static function controllerDataProvider(): array 'find', [Query::class], ], + 'the class docblock resolves through its own file, the inherited method through the parent\'s' => [ + ChildWithOwnImports::class, + 'inherited', + [RequiredPermissions::class, ResponseNormalization::class], + ], + 'a class docblock resolves relative to its namespace' => [ + ClassLevelCustomAnnotationController::class, + 'show', + [CustomRestAnnotation::class], + ], + 'each of two classes in one file reads its own namespace block' => [ + ChildOfAParentInAnotherNamespace::class, + 'create', + [RequiredPermissions::class, Body::class, LocalRestAnnotation::class], + ], + 'an import on the class\'s own line counts' => [ + ImportOnTheClassLineController::class, + 'find', + [Query::class], + ], 'a method from a trait resolves through the trait file imports' => [ ControllerUsingTheTrait::class, 'fromTrait', @@ -181,6 +210,66 @@ public static function controllerDataProvider(): array 'show', [RequiredPermissions::class], ], + 'a class named like a tag Doctrine ignores does not hide what its parentheses hold' => [ + IgnoredTagNameController::class, + 'show', + [RequiredPermissions::class], + ], + 'an annotation without arguments does not take the next one\'s' => [ + ArgumentBoundariesController::class, + 'afterAnAnnotationWithoutArguments', + [ResponseNormalization::class, RequiredPermissions::class], + ], + 'a parenthesis in a quoted argument does not end the arguments' => [ + ArgumentBoundariesController::class, + 'afterAParenthesisInAString', + [Query::class, RequiredPermissions::class], + ], + 'arguments after a space are still the annotation\'s' => [ + ArgumentBoundariesController::class, + 'argumentsAfterASpace', + [Query::class], + ], + 'a one-letter alias, aliases in a comma list and an import written with a leading backslash' => [ + ImportListWithAliasesController::class, + 'create', + [RequiredPermissions::class, Body::class, Validation::class, Query::class], + ], + 'reading can start at an "@" right after a star' => [ + StarAndQuoteBeforeAnnotationController::class, + 'firstRightAfterAStar', + [RequiredPermissions::class], + ], + 'an "@" right after a star starts an annotation' => [ + StarAndQuoteBeforeAnnotationController::class, + 'laterAfterAStar', + [ResponseNormalization::class, RequiredPermissions::class], + ], + 'an "@" right after a closing quote starts an annotation, as in Doctrine' => [ + StarAndQuoteBeforeAnnotationController::class, + 'afterAClosingQuote', + [RequiredPermissions::class], + ], + 'an "@" right after a lone quote starts one too' => [ + StarAndQuoteBeforeAnnotationController::class, + 'afterALoneQuote', + [ResponseNormalization::class], + ], + 'a ":" after a name leaves it an annotation' => [ + ArgumentBoundariesController::class, + 'colonAfterAName', + [ResponseNormalization::class], + ], + 'the parentheses after an imported class that is not an annotation are read' => [ + NonAnnotationClassTagController::class, + 'afterAnImportedNonAnnotationClass', + [RequiredPermissions::class], + ], + 'a name followed by a negative number is an annotation, as in Doctrine' => [ + ArgumentBoundariesController::class, + 'nameFollowedByANegativeNumber', + [ResponseNormalization::class], + ], 'a no-break space before an annotation is whitespace, as in Doctrine\'s lexer' => [ WhitespaceBeforeAnnotationController::class, 'afterANoBreakSpace', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php index 1684d31..3e8699f 100644 --- a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php +++ b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php @@ -4,6 +4,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures; +use Paysera\Bundle\ApiBundle as Api; use Paysera\Bundle\ApiBundle\Annotation as REST; use Paysera\Bundle\ApiBundle\Annotation\Validation as ApiValidation; @@ -13,6 +14,7 @@ class AliasedImportController * @REST\Query(parameterName="filter") * @ApiValidation(groups={"internal"}) * @\Paysera\Bundle\ApiBundle\Annotation\PathAttribute(parameterName="item", pathPartName="id") + * @Api\Annotation\ResponseNormalization */ public function find() { diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/ArgumentBoundariesController.php b/tests/Unit/Service/RoutingLoader/Fixtures/ArgumentBoundariesController.php new file mode 100644 index 0000000..4b16422 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/ArgumentBoundariesController.php @@ -0,0 +1,50 @@ + Date: Thu, 24 Sep 2026 12:24:13 +0530 Subject: [PATCH 28/42] EE-283 Check the configured status of internal_server_error ErrorBuilder falls back to a hard-coded 500 for other exceptions, so no test read the status configured for internal_server_error. A data set with that code now does. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Service/ErrorBuilderTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php index d273ed3..ef7f18b 100644 --- a/tests/Unit/Service/ErrorBuilderTest.php +++ b/tests/Unit/Service/ErrorBuilderTest.php @@ -89,6 +89,12 @@ public static function exceptionDataProvider(): array 406, 'Unknown request or response format', ], + 'API exception with the internal server error code' => [ + new ApiException(ApiException::INTERNAL_SERVER_ERROR), + 'internal_server_error', + 500, + 'Unexpected internal system error', + ], 'API exception with an unconfigured code' => [ new ApiException('unconfigured_code'), 'unconfigured_code', From 8eda65e9a23944a52159e3df95759a27ad316868 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 13:53:05 +0530 Subject: [PATCH 29/42] EE-283 Skip the semicolon-item test on Symfony 4.4 only The guard skipped on any failure of Symfony's parser, so a later release that broke on ";" would have skipped the test instead of failing it. Only http-foundation 4.4 fails on an empty item (3.4, 5.4, 6.4 and 7.4 parse it), so the test now skips on Symfony 4 and runs everywhere else. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Listener/LocaleListenerTest.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 3d08248..0397cf4 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -8,12 +8,11 @@ use Paysera\Bundle\ApiBundle\Listener\LocaleListener; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Paysera\Bundle\ApiBundle\Tests\Unit\Helper\HttpKernelHelper; -use Symfony\Component\HttpFoundation\AcceptHeader; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; -use Throwable; +use Symfony\Component\HttpKernel\Kernel; class LocaleListenerTest extends MockeryTestCase { @@ -32,11 +31,10 @@ public function testOnKernelRequest(string $expectedLocale, array $locales, stri public function testAnItemOfOnlyASemicolonIsNotALanguage() { - try { - AcceptHeader::fromString(';'); - } catch (Throwable $error) { - // http-foundation 4.4 reads a missing value from the empty item: a notice or warning, then a TypeError - $this->markTestSkipped('This http-foundation release fails on an empty Accept-Language item itself'); + if (Kernel::MAJOR_VERSION === 4) { + // http-foundation 4.4 fails on an empty Accept-Language item itself (a notice or warning, then a + // TypeError), with 1.8.2 as well + $this->markTestSkipped('Symfony 4.4 fails on an empty Accept-Language item itself'); } $this->assertSame('unchanged', $this->resolveLocale(['de'], ';', true)); From 7bf10abf492213806824c42b91363763724157ac Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 13:53:05 +0530 Subject: [PATCH 30/42] EE-283 Read annotation names the way Doctrine's parser joins and resolves them Four more inputs where the Symfony 7 check and Doctrine's reader (Symfony 4.4 to 6.4) differed: - a name written on both sides of a "\" with whitespace, a line break or "*" between them (`@REST\ RequiredPermissions`): Doctrine joins the parts, so the check now does too; - a name with two leading backslashes: Doctrine strips them all; - a bundle annotation written in another case after an alias (`@REST\requiredPermissions`): Doctrine found it once an earlier route had loaded the class, but on Symfony 7 nothing may have loaded it, and the autoloader looks for a file in the written case. The check now loads the bundle's own annotation classes before resolving names; - a name with a namespace that is not imported (`@Paysera\...\Query(... @Validation ...)`): Doctrine never ignores such a name, so its arguments are skipped again, as for imported names. The first three made the check miss an annotation Doctrine applies; the fourth made it report a nested one. New cases also pin that reading does not start at an "@" glued to a quote, and that a class docblock resolves in the route class's own namespace when the method is inherited. Each case gives Doctrine's result and fails when its rule is taken out. The class comment now says which arguments are skipped and in which direction the check can still differ from Doctrine. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 51 +++++++++++++---- .../DocblockAnnotationFinderTest.php | 56 ++++++++++++++++++- .../ChildOfAParentInAnotherNamespace.php | 1 + .../NameAcrossASeparatorController.php | 33 +++++++++++ .../Fixtures/QualifiedNameController.php | 31 ++++++++++ ...StarAndQuoteBeforeAnnotationController.php | 5 ++ .../Fixtures/WrongCaseNameController.php | 17 ++++++ 7 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NameAcrossASeparatorController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/QualifiedNameController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/WrongCaseNameController.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index 4d4a9eb..0fffd93 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -14,11 +14,15 @@ * - the imports are the `use` statements of the class's file above the class, in the class's namespace; a method * declared in a trait also gets those of the trait's file (Doctrine's PhpParser, TokenParser, getMethodImports()); * - reading starts at the first "@" after a space, a tab or "*"; from there an annotation starts at an "@" after - * whitespace or "*", a quoted string is text, a name followed by "-" is not an annotation (unless the "-" starts a - * number), and the arguments of an annotation are skipped (Doctrine's DocParser and DocLexer); - * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name. - * Doctrine also passes over the tag names it ignores (Target, Required, param and so on) unless they are imported or - * written in full. This finder keeps no such list, so where it cannot tell, it reads on and reports more, never less. + * whitespace, "*" or a quote, a quoted string is text, a name may continue after a "\" over whitespace and "*", a + * name followed by "-" is not an annotation (unless the "-" starts a number), and the arguments of an annotation + * class named through an import or with a namespace are skipped (Doctrine's DocParser and DocLexer); + * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name; the bundle's + * own annotation classes are loaded first, so a name written in another case resolves as it did once Doctrine had + * loaded them. + * Doctrine also passes over the tag names it ignores (Target, Required, param and so on, all without a namespace) + * unless they are imported annotation classes. This finder keeps no such list: after such a name it reads on, so it + * can report a nested annotation Doctrine did not apply, which refuses the route rather than dropping options. * * @internal */ @@ -38,9 +42,10 @@ class DocblockAnnotationFinder * A quoted string, which is one token, or an "@" at the start or after whitespace, "*" or a quote, with the name * right after it (group 1) and, when the name is followed by "-" that does not start a number, that "-" (group 2). * Doctrine's lexer measures a token that starts with a quote without its quotes, so an "@" right after a quote is - * never glued to it. + * never glued to it; and its parser joins a name ending in "\" to the next one over whitespace and "*". */ - private const TOKEN_PATTERN = '/' . self::STRING . '|(?resolveImportedName($name, $imports); $candidates = $importedName !== null ? [$importedName] : [$namespace . '\\' . $name, $name]; $className = $this->findClass($candidates); @@ -105,10 +115,11 @@ private function findInDocblock($docComment, array $imports, string $namespace): if (is_subclass_of($className, RestAnnotationInterface::class)) { $found[] = $className; } - // Doctrine reads the arguments of an annotation class named through an import or in full, so an "@" in them - // is a nested annotation or text. Found another way, the class may carry a name Doctrine ignores, and then + // Doctrine reads the arguments of an annotation class named through an import or with a namespace, so an + // "@" in them is a nested annotation or text. A name without either may be one Doctrine ignores, and then // Doctrine reads what follows as top-level annotations: so do not skip. - if ($importedName !== null + $isNeverIgnored = $importedName !== null || strpos($name, '\\') !== false; + if ($isNeverIgnored && $this->isAnnotationClass($className) && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 ) { @@ -126,7 +137,7 @@ private function findInDocblock($docComment, array $imports, string $namespace): private function resolveImportedName(string $name, array $imports): ?string { if ($name[0] === '\\') { - return $name; + return ltrim($name, '\\'); } $parts = explode('\\', $name, 2); @@ -144,6 +155,7 @@ private function resolveImportedName(string $name, array $imports): ?string */ private function findClass(array $candidates): ?string { + $this->loadBundleAnnotations(); foreach ($candidates as $candidate) { if (class_exists($candidate)) { return (new ReflectionClass($candidate))->getName(); @@ -153,6 +165,21 @@ private function findClass(array $candidates): ?string return null; } + /** + * PHP finds a loaded class by any case of its name; an autoloader, reading a file name, may not. + */ + private function loadBundleAnnotations(): void + { + if ($this->bundleAnnotationsLoaded) { + return; + } + $this->bundleAnnotationsLoaded = true; + $namespace = (new ReflectionClass(RestAnnotationInterface::class))->getNamespaceName(); + foreach (glob(dirname(__DIR__, 2) . '/Annotation/*.php') ?: [] as $file) { + class_exists($namespace . '\\' . basename($file, '.php')); + } + } + private function isAnnotationClass(string $className): bool { return strpos((string)(new ReflectionClass($className))->getDocComment(), '@Annotation') !== false; diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 8f55562..84d528c 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -32,14 +32,17 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportListWithAliasesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportOnTheClassLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportsBeforeTheClassController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NameAcrossASeparatorController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NonAnnotationClassTagController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\OtherNamespace\LocalRestAnnotation; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\QualifiedNameController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\StarAndQuoteBeforeAnnotationController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoImportsOnOneLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\TwoNamespacesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WhitespaceBeforeAnnotationController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WrongCaseNameController; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionMethod; @@ -66,6 +69,22 @@ public function testFindsTheBundleAnnotationsOfAMethodAndItsClass( $this->assertSame($expectedAnnotations, $annotations); } + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function testResolvesABundleAnnotationWrittenInAnotherCaseBeforeAnythingLoadedIt() + { + $finder = new DocblockAnnotationFinder(); + + $annotations = $finder->findBundleAnnotations( + new ReflectionClass(WrongCaseNameController::class), + new ReflectionMethod(WrongCaseNameController::class, 'show') + ); + + $this->assertSame([RequiredPermissions::class], $annotations); + } + public function testReadsAClassDeclaredInEvaluatedCode() { $testDouble = get_class($this->createMock(DirectImportController::class)); @@ -123,7 +142,7 @@ public static function controllerDataProvider(): array 'each of two classes in one file reads its own namespace block' => [ ChildOfAParentInAnotherNamespace::class, 'create', - [RequiredPermissions::class, Body::class, LocalRestAnnotation::class], + [RequiredPermissions::class, CustomRestAnnotation::class, Body::class, LocalRestAnnotation::class], ], 'an import on the class\'s own line counts' => [ ImportOnTheClassLineController::class, @@ -210,6 +229,36 @@ public static function controllerDataProvider(): array 'show', [RequiredPermissions::class], ], + 'a name with a namespace keeps its arguments, as Doctrine never ignores it' => [ + QualifiedNameController::class, + 'withoutTheLeadingBackslash', + [Query::class], + ], + 'a fully qualified name keeps its arguments' => [ + QualifiedNameController::class, + 'withTheLeadingBackslash', + [Query::class], + ], + 'two leading backslashes, as Doctrine strips them all' => [ + QualifiedNameController::class, + 'withTwoLeadingBackslashes', + [RequiredPermissions::class], + ], + 'a name continues after a separator and a space, as Doctrine joins it' => [ + NameAcrossASeparatorController::class, + 'spaceAfterTheSeparator', + [RequiredPermissions::class], + ], + 'a name continues after a separator and a line break' => [ + NameAcrossASeparatorController::class, + 'lineBreakAfterTheSeparator', + [RequiredPermissions::class], + ], + 'a name continues after a separator and a star' => [ + NameAcrossASeparatorController::class, + 'starAfterTheSeparator', + [RequiredPermissions::class], + ], 'a class named like a tag Doctrine ignores does not hide what its parentheses hold' => [ IgnoredTagNameController::class, 'show', @@ -245,6 +294,11 @@ public static function controllerDataProvider(): array 'laterAfterAStar', [ResponseNormalization::class, RequiredPermissions::class], ], + 'reading does not start at an "@" glued to a quote' => [ + StarAndQuoteBeforeAnnotationController::class, + 'firstGluedToAQuote', + [], + ], 'an "@" right after a closing quote starts an annotation, as in Doctrine' => [ StarAndQuoteBeforeAnnotationController::class, 'afterAClosingQuote', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php b/tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php index f03a46c..1c52800 100644 --- a/tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php +++ b/tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php @@ -27,6 +27,7 @@ public function create() /** * @Shared(permissions={"ROLE_CLASS"}) + * @CustomRestAnnotation */ class ChildOfAParentInAnotherNamespace extends OtherNamespace\ParentInAnotherNamespace { diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/NameAcrossASeparatorController.php b/tests/Unit/Service/RoutingLoader/Fixtures/NameAcrossASeparatorController.php new file mode 100644 index 0000000..11bca5a --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/NameAcrossASeparatorController.php @@ -0,0 +1,33 @@ + Date: Thu, 24 Sep 2026 13:54:48 +0530 Subject: [PATCH 31/42] EE-283 Test that a name resolves in the namespace before a PHP class of the same name Doctrine tries the controller's namespace before the global name, and so does the check; no test held that order. An application annotation named Directory, like PHP's own \Directory class, now pins it: tried in the other order, the check finds PHP's class and drops the annotation. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinderTest.php | 6 +++++ .../ClassLevelCustomAnnotationController.php | 7 +++++ .../RoutingLoader/Fixtures/Directory.php | 27 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Directory.php diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 84d528c..d4c8bb8 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -26,6 +26,7 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomAnnotationOnAttributeRouteController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DirectImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\Directory; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\FunctionAndConstantImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\IgnoredTagNameController; @@ -139,6 +140,11 @@ public static function controllerDataProvider(): array 'show', [CustomRestAnnotation::class], ], + 'a name resolves in the namespace before a PHP class of the same name' => [ + ClassLevelCustomAnnotationController::class, + 'list', + [CustomRestAnnotation::class, Directory::class], + ], 'each of two classes in one file reads its own namespace block' => [ ChildOfAParentInAnotherNamespace::class, 'create', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php b/tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php index a739162..ce82ae0 100644 --- a/tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php +++ b/tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php @@ -12,4 +12,11 @@ class ClassLevelCustomAnnotationController public function show() { } + + /** + * @Directory + */ + public function list() + { + } } diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/Directory.php b/tests/Unit/Service/RoutingLoader/Fixtures/Directory.php new file mode 100644 index 0000000..ed3cd4f --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/Directory.php @@ -0,0 +1,27 @@ + Date: Thu, 24 Sep 2026 16:22:36 +0530 Subject: [PATCH 32/42] EE-283 Read on after any name Doctrine may ignore, and test the name-joining rules 7bf10ab skipped the arguments of every name with a namespace, on the grounds that Doctrine never ignores one. It does when a class lists the name with @IgnoreAnnotation, and then reads what the parentheses hold as top-level annotations, so the check missed an annotation Doctrine applies. Arguments are again skipped only after an imported name or one written in full, where Doctrine reads them in every case; after any other name the check reads on and can only report more. The bundle's annotation classes are listed with scandir() instead of glob(), which reads brackets in the install path as a pattern. New cases pin a name joined across two separators, a no-break space after a separator, a name that does not take the next line's words, three leading backslashes, and a name the class tells Doctrine to ignore. Each gives Doctrine's result and fails when its rule is taken out. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 24 ++++++++------- .../DocblockAnnotationFinderTest.php | 30 +++++++++++++++++-- .../IgnoredQualifiedNameController.php | 22 ++++++++++++++ .../NameAcrossASeparatorController.php | 22 ++++++++++++++ .../Fixtures/QualifiedNameController.php | 7 +++++ 5 files changed, 92 insertions(+), 13 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/IgnoredQualifiedNameController.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index 0fffd93..f5f2841 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -16,13 +16,14 @@ * - reading starts at the first "@" after a space, a tab or "*"; from there an annotation starts at an "@" after * whitespace, "*" or a quote, a quoted string is text, a name may continue after a "\" over whitespace and "*", a * name followed by "-" is not an annotation (unless the "-" starts a number), and the arguments of an annotation - * class named through an import or with a namespace are skipped (Doctrine's DocParser and DocLexer); + * class named through an import or in full are skipped (Doctrine's DocParser and DocLexer); * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name; the bundle's * own annotation classes are loaded first, so a name written in another case resolves as it did once Doctrine had * loaded them. - * Doctrine also passes over the tag names it ignores (Target, Required, param and so on, all without a namespace) - * unless they are imported annotation classes. This finder keeps no such list: after such a name it reads on, so it - * can report a nested annotation Doctrine did not apply, which refuses the route rather than dropping options. + * Doctrine also passes over the tag names it ignores (Target, Required, param and so on, and any a class lists with + * @IgnoreAnnotation) unless they are imported or written in full. This finder keeps no such list: after any other + * name it reads on, so it can report a nested annotation Doctrine did not apply, which refuses the route rather than + * dropping options. * * @internal */ @@ -115,11 +116,10 @@ private function findInDocblock($docComment, array $imports, string $namespace): if (is_subclass_of($className, RestAnnotationInterface::class)) { $found[] = $className; } - // Doctrine reads the arguments of an annotation class named through an import or with a namespace, so an - // "@" in them is a nested annotation or text. A name without either may be one Doctrine ignores, and then - // Doctrine reads what follows as top-level annotations: so do not skip. - $isNeverIgnored = $importedName !== null || strpos($name, '\\') !== false; - if ($isNeverIgnored + // Doctrine reads the arguments of an annotation class named through an import or in full, so an "@" in them + // is a nested annotation or text. Any other name may be one Doctrine ignores, and then Doctrine reads what + // follows as top-level annotations: so do not skip. + if ($importedName !== null && $this->isAnnotationClass($className) && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 ) { @@ -175,8 +175,10 @@ private function loadBundleAnnotations(): void } $this->bundleAnnotationsLoaded = true; $namespace = (new ReflectionClass(RestAnnotationInterface::class))->getNamespaceName(); - foreach (glob(dirname(__DIR__, 2) . '/Annotation/*.php') ?: [] as $file) { - class_exists($namespace . '\\' . basename($file, '.php')); + foreach (scandir(dirname(__DIR__, 2) . '/Annotation') ?: [] as $file) { + if (substr($file, -4) === '.php') { + class_exists($namespace . '\\' . substr($file, 0, -4)); + } } } diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index d4c8bb8..742799b 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -29,6 +29,7 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\Directory; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\FunctionAndConstantImportsController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\GroupImportController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\IgnoredQualifiedNameController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\IgnoredTagNameController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportListWithAliasesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportOnTheClassLineController; @@ -235,10 +236,15 @@ public static function controllerDataProvider(): array 'show', [RequiredPermissions::class], ], - 'a name with a namespace keeps its arguments, as Doctrine never ignores it' => [ + 'after a name with a namespace that is not imported the arguments are read, as Doctrine may ignore it' => [ QualifiedNameController::class, 'withoutTheLeadingBackslash', - [Query::class], + [Query::class, Validation::class], + ], + 'a name with a namespace that the class tells Doctrine to ignore hides nothing' => [ + IgnoredQualifiedNameController::class, + 'show', + [RequiredPermissions::class], ], 'a fully qualified name keeps its arguments' => [ QualifiedNameController::class, @@ -250,6 +256,11 @@ public static function controllerDataProvider(): array 'withTwoLeadingBackslashes', [RequiredPermissions::class], ], + 'three leading backslashes' => [ + QualifiedNameController::class, + 'withThreeLeadingBackslashes', + [RequiredPermissions::class], + ], 'a name continues after a separator and a space, as Doctrine joins it' => [ NameAcrossASeparatorController::class, 'spaceAfterTheSeparator', @@ -265,6 +276,21 @@ public static function controllerDataProvider(): array 'starAfterTheSeparator', [RequiredPermissions::class], ], + 'a name continues across two separators with spaces' => [ + NameAcrossASeparatorController::class, + 'twoSeparatorsWithSpaces', + [RequiredPermissions::class], + ], + 'a name continues after a separator and a no-break space' => [ + NameAcrossASeparatorController::class, + 'noBreakSpaceAfterTheSeparator', + [RequiredPermissions::class], + ], + 'a name does not take the next line\'s words without a separator' => [ + NameAcrossASeparatorController::class, + 'nameFollowedByTextOnTheNextLine', + [ResponseNormalization::class], + ], 'a class named like a tag Doctrine ignores does not hide what its parentheses hold' => [ IgnoredTagNameController::class, 'show', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/IgnoredQualifiedNameController.php b/tests/Unit/Service/RoutingLoader/Fixtures/IgnoredQualifiedNameController.php new file mode 100644 index 0000000..37fb022 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/IgnoredQualifiedNameController.php @@ -0,0 +1,22 @@ + Date: Thu, 24 Sep 2026 16:22:36 +0530 Subject: [PATCH 33/42] EE-283 Key the semicolon-item test's skip on the parser, not on a version The skip checked http-kernel's major version, but the failure is http-foundation's, and the two can be installed at different majors. The test now skips exactly when Symfony's own parser fails on ";" (http-foundation 4.4, as with 1.8.2), and runs everywhere else. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Unit/Listener/LocaleListenerTest.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 0397cf4..7a937c4 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -8,11 +8,12 @@ use Paysera\Bundle\ApiBundle\Listener\LocaleListener; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Paysera\Bundle\ApiBundle\Tests\Unit\Helper\HttpKernelHelper; +use Symfony\Component\HttpFoundation\AcceptHeader; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; -use Symfony\Component\HttpKernel\Kernel; +use Throwable; class LocaleListenerTest extends MockeryTestCase { @@ -31,10 +32,12 @@ public function testOnKernelRequest(string $expectedLocale, array $locales, stri public function testAnItemOfOnlyASemicolonIsNotALanguage() { - if (Kernel::MAJOR_VERSION === 4) { - // http-foundation 4.4 fails on an empty Accept-Language item itself (a notice or warning, then a - // TypeError), with 1.8.2 as well - $this->markTestSkipped('Symfony 4.4 fails on an empty Accept-Language item itself'); + try { + AcceptHeader::fromString(';'); + } catch (Throwable $error) { + // http-foundation 4.4 fails on an empty item itself (a notice or warning, then a TypeError), with 1.8.2 + // as well; keyed on the parser, not on a version, because http-kernel and http-foundation can differ + $this->markTestSkipped('This http-foundation fails on an empty Accept-Language item itself'); } $this->assertSame('unchanged', $this->resolveLocale(['de'], ';', true)); From 7de29b6928227958db59676aba13dff17e1bd4f4 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Thu, 24 Sep 2026 17:20:56 +0530 Subject: [PATCH 34/42] EE-283 Read a docblock as doctrine/lexer 1.0 did where it reads more The lowest supported doctrine/lexer, 1.0, differs from 1.2 and later in two ways. It does not read a no-break space as whitespace, so a parenthesis after one is not an annotation's arguments. It reads a docblock that is not valid UTF-8 byte by byte, where the later versions read no annotations in it at all. The finder followed the later versions only. On lexer 1.0 it skipped an annotation Doctrine applied (the one inside a parenthesis after a no-break space), and a route would have loaded without it. It now takes only ASCII whitespace before arguments and reads a docblock that is not UTF-8 byte by byte: on every lexer version, the reading that reports more. The class comment also states the ignored-name rule as it is: Doctrine passes over such a name unless it names an annotation class through an import or in full. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinder.php | 18 +++++++++------- .../DocblockAnnotationFinderTest.php | 12 +++++++++++ .../Fixtures/LexerOneController.php | 21 +++++++++++++++++++ .../Fixtures/NotUtf8DocblockController.php | 20 ++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/LexerOneController.php create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NotUtf8DocblockController.php diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index f5f2841..ef8303a 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -21,9 +21,10 @@ * own annotation classes are loaded first, so a name written in another case resolves as it did once Doctrine had * loaded them. * Doctrine also passes over the tag names it ignores (Target, Required, param and so on, and any a class lists with - * @IgnoreAnnotation) unless they are imported or written in full. This finder keeps no such list: after any other - * name it reads on, so it can report a nested annotation Doctrine did not apply, which refuses the route rather than - * dropping options. + * @IgnoreAnnotation) unless they name an annotation class through an import or in full. This finder keeps no such + * list: after any other name it reads on, so it can report a nested annotation Doctrine did not apply, which refuses + * the route rather than dropping options. Where doctrine/lexer 1.0 read a docblock differently from its later + * versions (a no-break space, a byte that is not UTF-8), it takes the reading that reports more. * * @internal */ @@ -46,12 +47,13 @@ class DocblockAnnotationFinder * never glued to it; and its parser joins a name ending in "\" to the next one over whitespace and "*". */ private const TOKEN_PATTERN = '/' . self::STRING . '|(?> imports by class name @@ -98,7 +100,9 @@ private function findInDocblock($docComment, array $imports, string $namespace): $text = substr($docComment, $start[0][1] + 1); $found = []; $offset = 0; - while (preg_match(self::TOKEN_PATTERN, $text, $token, PREG_OFFSET_CAPTURE, $offset) === 1) { + // doctrine/lexer 1.2 and later read a docblock that is not valid UTF-8 as no annotations, 1.0 byte by byte + $pattern = self::TOKEN_PATTERN . (preg_match('//u', $text) === 1 ? 'iu' : 'i'); + while (preg_match($pattern, $text, $token, PREG_OFFSET_CAPTURE, $offset) === 1) { $offset = $token[0][1] + strlen($token[0][0]); $isString = !isset($token[1]); $isFollowedByDash = isset($token[2]); diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 742799b..8fd12dd 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -34,9 +34,11 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportListWithAliasesController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportOnTheClassLineController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\ImportsBeforeTheClassController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\LexerOneController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NameAcrossASeparatorController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NonAnnotationClassTagController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotTopLevelAnnotationsController; +use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\NotUtf8DocblockController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\OtherNamespace\LocalRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\QualifiedNameController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\SameLineImportController; @@ -291,6 +293,16 @@ public static function controllerDataProvider(): array 'nameFollowedByTextOnTheNextLine', [ResponseNormalization::class], ], + 'a no-break space before the arguments leaves them unread, as doctrine/lexer 1.0 read them' => [ + LexerOneController::class, + 'noBreakSpaceBeforeTheArguments', + [RequiredPermissions::class], + ], + 'a docblock with a byte that is not UTF-8 is still read, as doctrine/lexer 1.0 read it' => [ + NotUtf8DocblockController::class, + 'show', + [RequiredPermissions::class], + ], 'a class named like a tag Doctrine ignores does not hide what its parentheses hold' => [ IgnoredTagNameController::class, 'show', diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/LexerOneController.php b/tests/Unit/Service/RoutingLoader/Fixtures/LexerOneController.php new file mode 100644 index 0000000..c2580b7 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/LexerOneController.php @@ -0,0 +1,21 @@ + Date: Thu, 24 Sep 2026 17:21:44 +0530 Subject: [PATCH 35/42] EE-283 Test the annotation preload under a directory with brackets in its path The finder lists the bundle's Annotation directory with scandir rather than glob, because glob reads "[" and "]" in the path as a pattern and would then find nothing. A test now copies the bundle's source under a directory named with brackets and checks that a name written in another case still resolves from there. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../DocblockAnnotationFinderTest.php | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php index 8fd12dd..1545f68 100644 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php @@ -5,6 +5,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader; use ArrayObject; +use FilesystemIterator; use Paysera\Bundle\ApiBundle\Annotation\Body; use Paysera\Bundle\ApiBundle\Annotation\PathAttribute; use Paysera\Bundle\ApiBundle\Annotation\Query; @@ -48,6 +49,8 @@ use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WhitespaceBeforeAnnotationController; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\WrongCaseNameController; use PHPUnit\Framework\TestCase; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; use ReflectionClass; use ReflectionMethod; @@ -89,6 +92,37 @@ public function testResolvesABundleAnnotationWrittenInAnotherCaseBeforeAnythingL $this->assertSame([RequiredPermissions::class], $annotations); } + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function testResolvesAnotherCaseWhenTheBundleIsInstalledUnderAPathWithBrackets() + { + $directory = sys_get_temp_dir() . '/api-bundle-' . getmypid() . '[1]'; + $this->copyDirectory(dirname(__DIR__, 4) . '/src', $directory . '/src'); + spl_autoload_register(function (string $className) use ($directory) { + $prefix = 'Paysera\\Bundle\\ApiBundle\\'; + $file = $directory . '/src/' . str_replace('\\', '/', substr($className, strlen($prefix))) . '.php'; + if (strpos($className, $prefix) === 0 && is_file($file)) { + require $file; + } + }, true, true); + + try { + $annotations = (new DocblockAnnotationFinder())->findBundleAnnotations( + new ReflectionClass(WrongCaseNameController::class), + new ReflectionMethod(WrongCaseNameController::class, 'show') + ); + } finally { + $this->removeDirectory($directory); + } + + $this->assertSame( + [$directory . '/src/Service/RoutingLoader/DocblockAnnotationFinder.php', [RequiredPermissions::class]], + [(new ReflectionClass(DocblockAnnotationFinder::class))->getFileName(), $annotations] + ); + } + public function testReadsAClassDeclaredInEvaluatedCode() { $testDouble = get_class($this->createMock(DirectImportController::class)); @@ -390,4 +424,29 @@ public static function controllerDataProvider(): array ], ]; } + + private function copyDirectory(string $source, string $target) + { + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($source, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::SELF_FIRST + ); + mkdir($target, 0777, true); + foreach ($files as $file) { + $path = $target . '/' . substr($file->getPathname(), strlen($source) + 1); + $file->isDir() ? mkdir($path) : copy($file->getPathname(), $path); + } + } + + private function removeDirectory(string $directory) + { + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($directory, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($files as $file) { + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + rmdir($directory); + } } From 38a3ad66ebe54bab337b27fc755e7f714831007c Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Fri, 25 Sep 2026 19:11:30 +0530 Subject: [PATCH 36/42] EE-283 Rework the new tests to the maintainer's review standard Repeated tests become one test with a data provider, several getter checks become one assertion per case, and the pull request adds no comments; the reasons they gave are in the pull request description. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Listener/LocaleListener.php | 14 ---- .../DocblockAnnotationFinder.php | 57 +-------------- .../RoutingLoader/RoutingAttributeLoader.php | 5 +- .../AttributedPagedQueryController.php | 4 - .../AttributedPersistedEntityController.php | 4 - .../FixtureTestBundle/Service/TestHelper.php | 5 -- tests/Functional/Fixtures/TestKernel.php | 1 - .../Functional/Fixtures/config/sf7_common.yml | 1 - .../Fixtures/config/sf7_routing.yml | 2 - .../AttributeParameterResolutionTest.php | 50 +++++++------ tests/Unit/Entity/UnsetOptionsTest.php | 72 ++++++++++++------ tests/Unit/Listener/LocaleListenerTest.php | 2 - tests/Unit/Service/ErrorBuilderTest.php | 42 +++++++++-- ...estRequestAnnotationOptionsBuilderTest.php | 1 - .../RoutingAttributeLoaderTest.php | 73 +++++++++++-------- 15 files changed, 158 insertions(+), 175 deletions(-) diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index aaf051a..aebfd9a 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -43,14 +43,6 @@ public function onKernelRequest($event) } } - /** - * The first language of Accept-Language, in the client's order of preference, that is a configured locale; a - * regional variant (de_CH) also offers its primary language (de) unless the header lists that language itself. - * - * This is the rule Request::getPreferredLanguage() applied up to Symfony 7.0. Symfony 7.1 changed it, and passing a - * placeholder for "no match" stopped working there ("default" starts with "de"), so the listener applies the rule - * itself on every Symfony line, to a header it reads itself (readLanguages()). - */ private function resolveFromHeaders(Request $request): ?string { $languages = $this->readLanguages($request); @@ -77,22 +69,16 @@ private function resolveFromHeaders(Request $request): ?string } /** - * The Accept-Language tags in the client's order of preference, written the way Request::getLanguages() wrote them - * up to Symfony 7.0: "de-CH" becomes "de_CH", and a tag without a region keeps its case. Symfony 7.1 changed that - * formatting too, so the listener writes the tags this way itself, on every Symfony line. - * * @return string[] */ private function readLanguages(Request $request): array { $languages = []; foreach (AcceptHeader::fromString($request->headers->get('Accept-Language'))->all() as $item) { - // http-foundation 3.4 gives null or false for a malformed item, such as ";" or a lone quote $language = (string)$item->getValue(); if (strpos($language, '-') !== false) { $codes = explode('-', $language); if ($codes[0] === 'i') { - // a language registered with the i- prefix, such as i-cherokee $language = $codes[1]; } else { $language = strtolower($codes[0]); diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php index ef8303a..a84a74d 100644 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ b/src/Service/RoutingLoader/DocblockAnnotationFinder.php @@ -9,54 +9,21 @@ use ReflectionMethod; /** - * Finds the annotations for this bundle in the docblocks of a controller method and its class without an annotation - * reader. It follows the rules of Doctrine's reader, which applied them on Symfony 4.4 to 6.4: - * - the imports are the `use` statements of the class's file above the class, in the class's namespace; a method - * declared in a trait also gets those of the trait's file (Doctrine's PhpParser, TokenParser, getMethodImports()); - * - reading starts at the first "@" after a space, a tab or "*"; from there an annotation starts at an "@" after - * whitespace, "*" or a quote, a quoted string is text, a name may continue after a "\" over whitespace and "*", a - * name followed by "-" is not an annotation (unless the "-" starts a number), and the arguments of an annotation - * class named through an import or in full are skipped (Doctrine's DocParser and DocLexer); - * - a name resolves through the imports, else relative to the namespace, else as a fully qualified name; the bundle's - * own annotation classes are loaded first, so a name written in another case resolves as it did once Doctrine had - * loaded them. - * Doctrine also passes over the tag names it ignores (Target, Required, param and so on, and any a class lists with - * @IgnoreAnnotation) unless they name an annotation class through an import or in full. This finder keeps no such - * list: after any other name it reads on, so it can report a nested annotation Doctrine did not apply, which refuses - * the route rather than dropping options. Where doctrine/lexer 1.0 read a docblock differently from its later - * versions (a no-break space, a byte that is not UTF-8), it takes the reading that reports more. - * * @internal */ class DocblockAnnotationFinder { - /** - * A name as Doctrine's lexer reads it: letters, digits, "_", ":" and "\". - */ private const NAME = '[a-z_\\\\][a-z0-9_:\\\\]*[a-z_][a-z0-9_]*|[a-z_]'; - /** - * A quoted string, in which "" stands for a quote. - */ private const STRING = '"(?:""|[^"])*+"'; - /** - * A quoted string, which is one token, or an "@" at the start or after whitespace, "*" or a quote, with the name - * right after it (group 1) and, when the name is followed by "-" that does not start a number, that "-" (group 2). - * Doctrine's lexer measures a token that starts with a quote without its quotes, so an "@" right after a quote is - * never glued to it; and its parser joins a name ending in "\" to the next one over whitespace and "*". - */ private const TOKEN_PATTERN = '/' . self::STRING . '|(?> imports by class name + * @var array> */ private $importsByClass = []; @@ -66,7 +33,7 @@ class DocblockAnnotationFinder private $bundleAnnotationsLoaded = false; /** - * @return string[] class names of the bundle annotations the docblocks use, in order, each once + * @return string[] */ public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $method): array { @@ -100,7 +67,6 @@ private function findInDocblock($docComment, array $imports, string $namespace): $text = substr($docComment, $start[0][1] + 1); $found = []; $offset = 0; - // doctrine/lexer 1.2 and later read a docblock that is not valid UTF-8 as no annotations, 1.0 byte by byte $pattern = self::TOKEN_PATTERN . (preg_match('//u', $text) === 1 ? 'iu' : 'i'); while (preg_match($pattern, $text, $token, PREG_OFFSET_CAPTURE, $offset) === 1) { $offset = $token[0][1] + strlen($token[0][0]); @@ -120,9 +86,6 @@ private function findInDocblock($docComment, array $imports, string $namespace): if (is_subclass_of($className, RestAnnotationInterface::class)) { $found[] = $className; } - // Doctrine reads the arguments of an annotation class named through an import or in full, so an "@" in them - // is a nested annotation or text. Any other name may be one Doctrine ignores, and then Doctrine reads what - // follows as top-level annotations: so do not skip. if ($importedName !== null && $this->isAnnotationClass($className) && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 @@ -136,7 +99,6 @@ private function findInDocblock($docComment, array $imports, string $namespace): /** * @param array $imports - * @return string|null the class name a fully qualified or imported name stands for, or null for any other name */ private function resolveImportedName(string $name, array $imports): ?string { @@ -154,8 +116,7 @@ private function resolveImportedName(string $name, array $imports): ?string } /** - * @param string[] $candidates class names in the order Doctrine tries them - * @return string|null the first that exists, as the class declares its name + * @param string[] $candidates */ private function findClass(array $candidates): ?string { @@ -169,9 +130,6 @@ private function findClass(array $candidates): ?string return null; } - /** - * PHP finds a loaded class by any case of its name; an autoloader, reading a file name, may not. - */ private function loadBundleAnnotations(): void { if ($this->bundleAnnotationsLoaded) { @@ -192,7 +150,7 @@ private function isAnnotationClass(string $className): bool } /** - * @return array imported class or namespace name by its lower-case alias + * @return array */ private function readImports(ReflectionClass $class): array { @@ -205,9 +163,6 @@ private function readImports(ReflectionClass $class): array } /** - * The `use` statements of the class's file up to the class, from the class's namespace declaration on. So a - * trait's `use` in a class body does not count, nor does a commented-out import or another namespace's import. - * * @return array */ private function parseImports(ReflectionClass $class): array @@ -240,9 +195,6 @@ private function parseImports(ReflectionClass $class): array } /** - * One `use` statement: a name, a name with "as", names separated by commas, or a group ("use A\{B, C as D};"). - * `use function`, `use const` and a closure's `use (...)` import no class, so they end at their first token. - * * @param array $tokens * @return array */ @@ -304,7 +256,6 @@ private function isNameToken($token): bool { return is_array($token) && ( in_array($token[0], [T_STRING, T_NS_SEPARATOR], true) - // PHP 8 reads a qualified name as one token || (PHP_VERSION_ID >= 80000 && in_array($token[0], [T_NAME_QUALIFIED, T_NAME_FULLY_QUALIFIED], true)) ); } diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index ca64406..31536f9 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -69,7 +69,7 @@ protected function configureRoute( } /** - * @throws ConfigurationException on Symfony 7 and later when the controller uses the bundle's docblock annotations + * @throws ConfigurationException */ private function loadAnnotations(Route $route, ReflectionClass $class, ReflectionMethod $method): void { @@ -106,9 +106,6 @@ private function loadAnnotations(Route $route, ReflectionClass $class, Reflectio } /** - * Symfony 7 gives the route loader no annotation reader, so these options would be ignored without a word — an - * endpoint would lose its required permissions. Fail at route loading instead and name the attributes to use. - * * @throws ConfigurationException */ private function refuseDocblockAnnotations(ReflectionClass $class, ReflectionMethod $method): void diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php index 2e69312..5d5e77b 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php @@ -12,10 +12,6 @@ use Paysera\Pagination\Entity\Pager; use Symfony\Component\Routing\Annotation\Route; -/** - * PagedQueryController with attributes: it serves the same path on Symfony 7, which no longer reads @Route docblocks. - * Below Symfony 7 the docblock controller is imported first and serves the path. - */ class AttributedPagedQueryController { private $entityManager; diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php index ed79485..1fcbc06 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php @@ -9,10 +9,6 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; -/** - * PersistedEntityController with attributes: it serves the same paths on Symfony 7, which no longer reads @Route - * docblocks. Below Symfony 7 the docblock controller is imported first and serves the paths. - */ class AttributedPersistedEntityController { #[Route(path: '/persisted-entities/{identifier}', methods: ['GET'])] diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php index b6933da..e092728 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php @@ -14,11 +14,6 @@ public static function phpAttributeSupportExists(): bool return class_exists(AttributeRouteControllerLoader::class); } - /** - * Whether the installed symfony/routing still reads docblock annotations: its attribute loader lost the annotation - * reader in 7.0. The bundle decides the same way (RoutingAttributeLoader), so a 6.4 framework next to routing 7 is - * tested as the Symfony 7 case it is. - */ public static function docblockRoutingSupportExists(): bool { return !class_exists(AttributeClassLoader::class) || property_exists(AttributeClassLoader::class, 'reader'); diff --git a/tests/Functional/Fixtures/TestKernel.php b/tests/Functional/Fixtures/TestKernel.php index bb7f618..e0810d4 100644 --- a/tests/Functional/Fixtures/TestKernel.php +++ b/tests/Functional/Fixtures/TestKernel.php @@ -43,7 +43,6 @@ public function registerContainerConfiguration(LoaderInterface $loader) $loader->load(__DIR__ . '/config/' . $this->configFile); $loader->load(__DIR__ . '/config/' . $this->commonFile); - // Symfony 7 gets its routes from its own common file (sf7_common.yml): no @Route docblock imports if (TestHelper::phpAttributeSupportExists() && TestHelper::docblockRoutingSupportExists()) { $loader->load(__DIR__ . '/config/attributed_common.yml'); } diff --git a/tests/Functional/Fixtures/config/sf7_common.yml b/tests/Functional/Fixtures/config/sf7_common.yml index 5a42048..9fe4ff7 100644 --- a/tests/Functional/Fixtures/config/sf7_common.yml +++ b/tests/Functional/Fixtures/config/sf7_common.yml @@ -2,7 +2,6 @@ framework: router: resource: '%kernel.project_dir%/tests/Functional/Fixtures/config/sf7_routing.yml' validation: - # Symfony 7's default; framework-bundle 6.4 still defaults to "loose", which validator 7 no longer has email_validation_mode: html5 security: diff --git a/tests/Functional/Fixtures/config/sf7_routing.yml b/tests/Functional/Fixtures/config/sf7_routing.yml index 80ec812..2b876f8 100644 --- a/tests/Functional/Fixtures/config/sf7_routing.yml +++ b/tests/Functional/Fixtures/config/sf7_routing.yml @@ -1,5 +1,3 @@ -# Symfony 7 reads no docblock annotations: the controllers routed by @Route docblocks are not imported here, their -# attribute twins in Controller/Attribute/ are. paysera_fixture_test: resource: "@PayseraFixtureTestBundle/Resources/config/explicit_routing.xml" prefix: / diff --git a/tests/Unit/Attribute/AttributeParameterResolutionTest.php b/tests/Unit/Attribute/AttributeParameterResolutionTest.php index a8b7193..30cfddf 100644 --- a/tests/Unit/Attribute/AttributeParameterResolutionTest.php +++ b/tests/Unit/Attribute/AttributeParameterResolutionTest.php @@ -7,20 +7,23 @@ use Paysera\Bundle\ApiBundle\Attribute\Body; use Paysera\Bundle\ApiBundle\Attribute\PathAttribute; use Paysera\Bundle\ApiBundle\Attribute\Query; +use Paysera\Bundle\ApiBundle\Entity\PathAttributeResolverOptions; use Paysera\Bundle\ApiBundle\Entity\RestRequestOptions; use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; use Paysera\Bundle\ApiBundle\Service\RoutingLoader\ReflectionMethodWrapper; use PHPUnit\Framework\TestCase; use ReflectionMethod; -/** - * What the attributes take from the controller method's signature and what they take from their own arguments. - */ class AttributeParameterResolutionTest extends TestCase { - public function testPathAttributeCannotGuessTheTypeOfAnUntypedParameter() + /** + * @dataProvider untypedParameterAttributeDataProvider + * + * @param array $attributeOptions + */ + public function testCannotGuessTheTypeOfAnUntypedParameter(string $attributeClass, array $attributeOptions) { - $attribute = new PathAttribute(['parameterName' => 'item', 'pathPartName' => 'id']); + $attribute = new $attributeClass($attributeOptions); $this->expectException(ConfigurationException::class); $this->expectExceptionMessage('Denormalization type could not be guessed for $item in '); @@ -28,14 +31,15 @@ public function testPathAttributeCannotGuessTheTypeOfAnUntypedParameter() $attribute->apply(new RestRequestOptions(), $this->wrapUntypedAction()); } - public function testQueryCannotGuessTheTypeOfAnUntypedParameter() + /** + * @return array}> + */ + public static function untypedParameterAttributeDataProvider(): array { - $attribute = new Query(['parameterName' => 'item']); - - $this->expectException(ConfigurationException::class); - $this->expectExceptionMessage('Denormalization type could not be guessed for $item in '); - - $attribute->apply(new RestRequestOptions(), $this->wrapUntypedAction()); + return [ + 'path attribute' => [PathAttribute::class, ['parameterName' => 'item', 'pathPartName' => 'id']], + 'query' => [Query::class, ['parameterName' => 'item']], + ]; } public function testExplicitArgumentsWinOverTheSignature() @@ -54,15 +58,19 @@ public function testExplicitArgumentsWinOverTheSignature() 'optional' => true, ]))->apply($options, $this->wrapUntypedAction()); - $pathAttributeOptions = $options->getPathAttributeResolverOptionsList()[0]; - $this->assertSame( - ['custom_resolver', false, true], - [ - $pathAttributeOptions->getPathAttributeResolverType(), - $pathAttributeOptions->isResolutionMandatory(), - $options->isBodyOptional(), - ] - ); + $expectedOptions = (new RestRequestOptions()) + ->addPathAttributeResolverOptions( + (new PathAttributeResolverOptions()) + ->setParameterName('item') + ->setPathPartName('id') + ->setPathAttributeResolverType('custom_resolver') + ->setResolutionMandatory(false) + ) + ->setBodyParameterName('item') + ->setBodyDenormalizationType('custom_type') + ->setBodyOptional(true) + ; + $this->assertEquals($expectedOptions, $options); } /** diff --git a/tests/Unit/Entity/UnsetOptionsTest.php b/tests/Unit/Entity/UnsetOptionsTest.php index 441ed4d..3f37914 100644 --- a/tests/Unit/Entity/UnsetOptionsTest.php +++ b/tests/Unit/Entity/UnsetOptionsTest.php @@ -11,51 +11,77 @@ use PHPUnit\Framework\TestCase; use RuntimeException; -/** - * Reading an option that was never set fails with a message that says what to check first. - */ class UnsetOptionsTest extends TestCase { /** * @dataProvider unsetOptionDataProvider */ - public function testReadingAnUnsetOptionFails($options, string $getter, string $expectedMessage) + public function testReadingAnUnsetOptionFails(callable $readOption, string $expectedMessage) { $this->expectException(RuntimeException::class); $this->expectExceptionMessage($expectedMessage); - $options->$getter(); + $readOption(); } /** - * @return array + * @return array */ public static function unsetOptionDataProvider(): array { return [ - [new PathAttributeResolverOptions(), 'getParameterName', 'parameterName was not set'], - [new PathAttributeResolverOptions(), 'getPathPartName', 'pathPartName was not set'], - [new PathAttributeResolverOptions(), 'getPathAttributeResolverType', 'pathAttributeResolverType was not set'], - [new QueryResolverOptions(), 'getParameterName', 'parameterName was not set'], - [new QueryResolverOptions(), 'getDenormalizationType', 'denormalizationType was not set'], - [ - (new QueryResolverOptions())->setValidationOptions(null), - 'getValidationOptions', + 'path attribute parameter name' => [ + function () { + (new PathAttributeResolverOptions())->getParameterName(); + }, + 'parameterName was not set', + ], + 'path attribute path part name' => [ + function () { + (new PathAttributeResolverOptions())->getPathPartName(); + }, + 'pathPartName was not set', + ], + 'path attribute resolver type' => [ + function () { + (new PathAttributeResolverOptions())->getPathAttributeResolverType(); + }, + 'pathAttributeResolverType was not set', + ], + 'query parameter name' => [ + function () { + (new QueryResolverOptions())->getParameterName(); + }, + 'parameterName was not set', + ], + 'query denormalization type' => [ + function () { + (new QueryResolverOptions())->getDenormalizationType(); + }, + 'denormalizationType was not set', + ], + 'query validation options after they were set to null' => [ + function () { + (new QueryResolverOptions())->setValidationOptions(null)->getValidationOptions(); + }, 'No validationOptions available, call isValidationNeeded beforehand', ], - [ - new RestRequestOptions(), - 'getBodyDenormalizationType', + 'body denormalization type' => [ + function () { + (new RestRequestOptions())->getBodyDenormalizationType(); + }, 'No bodyDenormalizationType available, call hasBodyDenormalization beforehand', ], - [ - new RestRequestOptions(), - 'getBodyParameterName', + 'body parameter name' => [ + function () { + (new RestRequestOptions())->getBodyParameterName(); + }, 'No bodyParameterName available, call hasBodyDenormalization beforehand', ], - [ - (new RestRequestOptions())->disableBodyValidation(), - 'getBodyValidationOptions', + 'body validation options after validation was disabled' => [ + function () { + (new RestRequestOptions())->disableBodyValidation()->getBodyValidationOptions(); + }, 'No bodyValidationOptions available, call isBodyValidationNeeded beforehand', ], ]; diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index 7a937c4..b56a774 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -35,8 +35,6 @@ public function testAnItemOfOnlyASemicolonIsNotALanguage() try { AcceptHeader::fromString(';'); } catch (Throwable $error) { - // http-foundation 4.4 fails on an empty item itself (a notice or warning, then a TypeError), with 1.8.2 - // as well; keyed on the parser, not on a version, because http-kernel and http-foundation can differ $this->markTestSkipped('This http-foundation fails on an empty Accept-Language item itself'); } diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php index ef7f18b..85f0bdc 100644 --- a/tests/Unit/Service/ErrorBuilderTest.php +++ b/tests/Unit/Service/ErrorBuilderTest.php @@ -4,6 +4,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Unit\Service; +use Paysera\Bundle\ApiBundle\Entity\Error; use Paysera\Bundle\ApiBundle\Entity\Violation; use Paysera\Bundle\ApiBundle\Exception\ApiException; use Paysera\Bundle\ApiBundle\Service\ErrorBuilder; @@ -27,10 +28,6 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Throwable; -/** - * The error every REST endpoint answers with, for each kind of exception, built by the bundle's own - * paysera_api.error_builder service definition (Resources/config/services.xml) with the error codes it configures. - */ class ErrorBuilderTest extends TestCase { /** @@ -45,8 +42,16 @@ public function testBuildsTheErrorForEachKindOfException( $error = $this->createConfiguredErrorBuilder()->createErrorFromException($exception); $this->assertSame( - [$expectedCode, $expectedStatusCode, $expectedMessage], - [$error->getCode(), $error->getStatusCode(), $error->getMessage()] + [ + 'code' => $expectedCode, + 'statusCode' => $expectedStatusCode, + 'uri' => null, + 'message' => $expectedMessage, + 'properties' => null, + 'data' => null, + 'violations' => [], + ], + $this->readError($error) ); } @@ -208,8 +213,16 @@ public function testCarriesTheApiExceptionDetails() $error = $this->createConfiguredErrorBuilder()->createErrorFromException($exception); $this->assertSame( - [['amount' => 'Too large'], ['limit' => 100], [$violation]], - [$error->getProperties(), $error->getData(), $error->getViolations()] + [ + 'code' => 'custom_code', + 'statusCode' => 400, + 'uri' => null, + 'message' => null, + 'properties' => ['amount' => 'Too large'], + 'data' => ['limit' => 100], + 'violations' => [$violation], + ], + $this->readError($error) ); } @@ -221,4 +234,17 @@ private function createConfiguredErrorBuilder(): ErrorBuilder return $container->get('paysera_api.error_builder'); } + + private function readError(Error $error): array + { + return [ + 'code' => $error->getCode(), + 'statusCode' => $error->getStatusCode(), + 'uri' => $error->getUri(), + 'message' => $error->getMessage(), + 'properties' => $error->getProperties(), + 'data' => $error->getData(), + 'violations' => $error->getViolations(), + ]; + } } diff --git a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php index 4429736..b5350bd 100644 --- a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php +++ b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php @@ -70,7 +70,6 @@ public function testBuildOptionsWithSeveralUnsupportedAnnotations(): void $this->expectException(ConfigurationException::class); $this->expectExceptionMessage('Only one annotation of type ' . Body::class . ' is supported'); - // each Body names its type and optionality, so applying the first one succeeds and the second meets the guard $builder->buildOptions([ new Body(['parameterName' => 'a', 'denormalizationType' => 'type_a', 'optional' => false]), new Body(['parameterName' => 'b', 'denormalizationType' => 'type_b', 'optional' => false]), diff --git a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php index 589fcd7..006776c 100644 --- a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php +++ b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php @@ -46,39 +46,47 @@ protected function setUp(): void $this->attributeOptionsBuilder = Mockery::mock(RestRequestAttributeOptionsBuilder::class); } - public function testRefusesTheBundleDocblockAnnotationsWhereSymfonyReadsNone() - { + /** + * @dataProvider refusedControllerDataProvider + */ + public function testRefusesTheBundleDocblockAnnotationsWhereSymfonyReadsNone( + string $controllerClass, + string $expectedMessage + ) { $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); $loader = $this->createLoader(); $this->requestHelper->shouldNotReceive('setOptionsForRoute'); try { - $loader->load(DocblockOptionsOnAttributeRouteController::class); + $loader->load($controllerClass); $this->fail('The route with the bundle\'s docblock annotations was loaded without them'); } catch (ConfigurationException $exception) { - $this->assertSame( - DocblockOptionsOnAttributeRouteController::class . '::show() uses docblock annotations of ' - . 'paysera/lib-api-bundle (\\' . RequiredPermissions::class . '). Symfony 7 does not read docblock ' - . 'annotations, so they would have no effect. Use the PHP attributes instead: ' - . '#[\\Paysera\\Bundle\\ApiBundle\\Attribute\\RequiredPermissions].', - $exception->getMessage() - ); + $this->assertSame($expectedMessage, $exception->getMessage()); } } - public function testNamesTheAttributeInterfaceForAnApplicationsOwnAnnotation() + /** + * @return array + */ + public static function refusedControllerDataProvider(): array { - $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); - $loader = $this->createLoader(); - - $this->expectException(ConfigurationException::class); - $this->expectExceptionMessage( - 'Use the PHP attributes instead: an attribute implementing ' - . '\\Paysera\\Bundle\\ApiBundle\\Attribute\\RestAttributeInterface in place of \\' - . CustomRestAnnotation::class . '.' - ); - - $loader->load(CustomAnnotationOnAttributeRouteController::class); + return [ + 'a bundle annotation, with the attribute to use' => [ + DocblockOptionsOnAttributeRouteController::class, + DocblockOptionsOnAttributeRouteController::class . '::show() uses docblock annotations of ' + . 'paysera/lib-api-bundle (\\' . RequiredPermissions::class . '). Symfony 7 does not read docblock ' + . 'annotations, so they would have no effect. Use the PHP attributes instead: ' + . '#[\\Paysera\\Bundle\\ApiBundle\\Attribute\\RequiredPermissions].', + ], + 'an application\'s own annotation, with the attribute interface to implement' => [ + CustomAnnotationOnAttributeRouteController::class, + CustomAnnotationOnAttributeRouteController::class . '::show() uses docblock annotations of ' + . 'paysera/lib-api-bundle (\\' . CustomRestAnnotation::class . '). Symfony 7 does not read docblock ' + . 'annotations, so they would have no effect. Use the PHP attributes instead: an attribute ' + . 'implementing \\Paysera\\Bundle\\ApiBundle\\Attribute\\RestAttributeInterface in place of \\' + . CustomRestAnnotation::class . '.', + ], + ]; } public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() @@ -96,11 +104,7 @@ public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSymfony7() { - if (!class_exists(AttributeRouteControllerLoader::class) - || !property_exists(AttributeRouteControllerLoader::class, 'reader') - ) { - $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader'); - } + $this->skipUnlessTheRouteLoaderHasAnAnnotationReader(); $loader = $this->createLoader(new AnnotationReader()); $options = new RestRequestOptions(); $this->annotationOptionsBuilder @@ -120,11 +124,7 @@ public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSym public function testIgnoresTheBundleDocblockAnnotationsWhereTheApplicationDisabledAnnotationsBeforeSymfony7() { - if (!class_exists(AttributeRouteControllerLoader::class) - || !property_exists(AttributeRouteControllerLoader::class, 'reader') - ) { - $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader property'); - } + $this->skipUnlessTheRouteLoaderHasAnAnnotationReader(); $loader = $this->createLoader(); $this->annotationOptionsBuilder->shouldNotReceive('buildOptions'); $this->requestHelper->shouldNotReceive('setOptionsForRoute'); @@ -134,6 +134,15 @@ public function testIgnoresTheBundleDocblockAnnotationsWhereTheApplicationDisabl $this->assertCount(1, $routes); } + private function skipUnlessTheRouteLoaderHasAnAnnotationReader() + { + if (!class_exists(AttributeRouteControllerLoader::class) + || !property_exists(AttributeRouteControllerLoader::class, 'reader') + ) { + $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader property'); + } + } + private function skipUnlessTheRouteLoaderHasNoAnnotationReader() { if (!class_exists(AttributeRouteControllerLoader::class) From a11829b54b97490e128f640c859b1e199471bc1e Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Mon, 28 Sep 2026 16:33:47 +0530 Subject: [PATCH 37/42] EE-283 Run the workflow jobs with lowest dependencies too The matrix gains dependency lowest next to highest, and the single PHP 7.1 lowest entry became one of those jobs. Eight exclude entries take out the 10 lowest combinations whose oldest allowed dependencies cannot run together or on PHP 8: Symfony 4's routing annotation loader on PHP 8 (it misreads class names), lib-object-wrapper 0.3.0 on PHP 8.1 and later (ArrayAccess return types), and DoctrineBundle's lowest release against Symfony 6.0's Doctrine bridge. The 14 lowest jobs that remain pass. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 217403a..c1a8ae9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,9 +33,8 @@ jobs: - '6.*' - '7.*' dependency: + - 'lowest' - 'highest' - include: - - { php: '7.1', symfony: '3.*', dependency: 'lowest' } exclude: - { php: '8.0', symfony: '3.*' } - { php: '8.1', symfony: '3.*' } @@ -53,6 +52,14 @@ jobs: - { php: '7.4', symfony: '7.*' } - { php: '8.0', symfony: '7.*' } - { php: '8.1', symfony: '7.*' } + - { php: '8.0', symfony: '4.*', dependency: 'lowest' } + - { php: '8.1', dependency: 'lowest' } + - { php: '8.2', symfony: '4.*', dependency: 'lowest' } + - { php: '8.2', symfony: '5.*', dependency: 'lowest' } + - { php: '8.2', symfony: '6.*', dependency: 'lowest' } + - { php: '8.3', symfony: '4.*', dependency: 'lowest' } + - { php: '8.3', symfony: '5.*', dependency: 'lowest' } + - { php: '8.3', symfony: '6.*', dependency: 'lowest' } steps: - name: Checkout From ee37629023da37d99a90cb8bb9f690e56445d637 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Mon, 28 Sep 2026 17:01:19 +0530 Subject: [PATCH 38/42] EE-283 Expect the bundle's docblock annotations to be ignored without a reader The routing loader test no longer expects a refusal on Symfony 7. Without an annotation reader, on Symfony 7 or where an application disabled annotations before it, the bundle's docblock annotations are ignored and the route loads. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../RoutingAttributeLoaderTest.php | 52 ++----------------- 1 file changed, 4 insertions(+), 48 deletions(-) diff --git a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php index 006776c..2bb4233 100644 --- a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php +++ b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php @@ -10,14 +10,11 @@ use Mockery\MockInterface; use Paysera\Bundle\ApiBundle\Annotation\RequiredPermissions; use Paysera\Bundle\ApiBundle\Entity\RestRequestOptions; -use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RestRequestAnnotationOptionsBuilder; use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RestRequestAttributeOptionsBuilder; use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RoutingAttributeLoader; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\AttributeOnlyController; -use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomAnnotationOnAttributeRouteController; -use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\CustomRestAnnotation; use Paysera\Bundle\ApiBundle\Tests\Unit\Service\RoutingLoader\Fixtures\DocblockOptionsOnAttributeRouteController; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; @@ -46,49 +43,6 @@ protected function setUp(): void $this->attributeOptionsBuilder = Mockery::mock(RestRequestAttributeOptionsBuilder::class); } - /** - * @dataProvider refusedControllerDataProvider - */ - public function testRefusesTheBundleDocblockAnnotationsWhereSymfonyReadsNone( - string $controllerClass, - string $expectedMessage - ) { - $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); - $loader = $this->createLoader(); - $this->requestHelper->shouldNotReceive('setOptionsForRoute'); - - try { - $loader->load($controllerClass); - $this->fail('The route with the bundle\'s docblock annotations was loaded without them'); - } catch (ConfigurationException $exception) { - $this->assertSame($expectedMessage, $exception->getMessage()); - } - } - - /** - * @return array - */ - public static function refusedControllerDataProvider(): array - { - return [ - 'a bundle annotation, with the attribute to use' => [ - DocblockOptionsOnAttributeRouteController::class, - DocblockOptionsOnAttributeRouteController::class . '::show() uses docblock annotations of ' - . 'paysera/lib-api-bundle (\\' . RequiredPermissions::class . '). Symfony 7 does not read docblock ' - . 'annotations, so they would have no effect. Use the PHP attributes instead: ' - . '#[\\Paysera\\Bundle\\ApiBundle\\Attribute\\RequiredPermissions].', - ], - 'an application\'s own annotation, with the attribute interface to implement' => [ - CustomAnnotationOnAttributeRouteController::class, - CustomAnnotationOnAttributeRouteController::class . '::show() uses docblock annotations of ' - . 'paysera/lib-api-bundle (\\' . CustomRestAnnotation::class . '). Symfony 7 does not read docblock ' - . 'annotations, so they would have no effect. Use the PHP attributes instead: an attribute ' - . 'implementing \\Paysera\\Bundle\\ApiBundle\\Attribute\\RestAttributeInterface in place of \\' - . CustomRestAnnotation::class . '.', - ], - ]; - } - public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() { $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); @@ -122,9 +76,11 @@ public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSym $this->assertCount(1, $routes); } - public function testIgnoresTheBundleDocblockAnnotationsWhereTheApplicationDisabledAnnotationsBeforeSymfony7() + public function testIgnoresTheBundleDocblockAnnotationsWithoutAnAnnotationReader() { - $this->skipUnlessTheRouteLoaderHasAnAnnotationReader(); + if (!class_exists(AttributeRouteControllerLoader::class)) { + $this->markTestSkipped('Needs the attribute route loader of Symfony 6.4 and later'); + } $loader = $this->createLoader(); $this->annotationOptionsBuilder->shouldNotReceive('buildOptions'); $this->requestHelper->shouldNotReceive('setOptionsForRoute'); From 6552ec25e4721249235450e72028bbb405b2f5a5 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Mon, 28 Sep 2026 17:03:53 +0530 Subject: [PATCH 39/42] EE-283 Drop the refusal of docblock annotations on Symfony 7 The docblock annotation finder and the refusal built on it are removed: RoutingAttributeLoader is back to master's code, which skips the bundle's docblock annotations when there is no annotation reader, as on Symfony 7. The finder's test and the 40 fixtures only it used go with it. CHANGELOG and README say that the annotations have no effect on Symfony 7 and that the attributes of the same name configure those routes. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 +- README.md | 5 +- .../DocblockAnnotationFinder.php | 262 ---------- .../RoutingLoader/RoutingAttributeLoader.php | 51 -- .../DocblockAnnotationFinderTest.php | 452 ------------------ .../Fixtures/AliasedImportController.php | 22 - .../Fixtures/ArgumentBoundariesController.php | 50 -- .../ChildOfAParentInAnotherNamespace.php | 35 -- .../Fixtures/ChildWithOwnImports.php | 14 - .../Fixtures/ChildWithoutImports.php | 9 - .../ClassLevelCustomAnnotationController.php | 22 - .../Fixtures/CommaImportController.php | 19 - .../Fixtures/CommentedImportsController.php | 25 - .../ControllerOverridingTheTraitMethod.php | 19 - .../Fixtures/ControllerUsingTheTrait.php | 13 - ...omAnnotationOnAttributeRouteController.php | 18 - .../Fixtures/CustomRestAnnotation.php | 27 -- .../Fixtures/DirectImportController.php | 26 - .../RoutingLoader/Fixtures/Directory.php | 27 -- .../FunctionAndConstantImportsController.php | 19 - .../Fixtures/GroupImportController.php | 18 - .../IgnoredQualifiedNameController.php | 22 - .../Fixtures/IgnoredTagNameController.php | 17 - .../ImportListWithAliasesController.php | 22 - .../ImportOnTheClassLineController.php | 15 - .../ImportsBeforeTheClassController.php | 24 - .../Fixtures/LexerOneController.php | 21 - .../NameAcrossASeparatorController.php | 55 --- .../NonAnnotationClassTagController.php | 25 - .../NotTopLevelAnnotationsController.php | 58 --- .../Fixtures/NotUtf8DocblockController.php | 20 - .../OtherNamespace/LocalRestAnnotation.php | 27 -- .../Fixtures/ParentWithDocblockOptions.php | 17 - .../Fixtures/QualifiedNameController.php | 38 -- .../RoutingLoader/Fixtures/Required.php | 12 - .../Fixtures/SameLineImportController.php | 16 - ...StarAndQuoteBeforeAnnotationController.php | 44 -- .../Service/RoutingLoader/Fixtures/Target.php | 18 - .../Fixtures/TraitWithAnotherQueryImport.php | 17 - .../Fixtures/TraitWithDocblockOptions.php | 17 - .../RoutingLoader/Fixtures/Traits/Query.php | 12 - .../TwoImportsOnOneLineController.php | 18 - .../Fixtures/TwoNamespacesController.php | 19 - .../WhitespaceBeforeAnnotationController.php | 27 -- .../Fixtures/WrongCaseNameController.php | 17 - 45 files changed, 5 insertions(+), 1713 deletions(-) delete mode 100644 src/Service/RoutingLoader/DocblockAnnotationFinder.php delete mode 100644 tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ArgumentBoundariesController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildOfAParentInAnotherNamespace.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildWithOwnImports.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ChildWithoutImports.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ClassLevelCustomAnnotationController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CommaImportController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CommentedImportsController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ControllerOverridingTheTraitMethod.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ControllerUsingTheTrait.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CustomAnnotationOnAttributeRouteController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/CustomRestAnnotation.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/DirectImportController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Directory.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/FunctionAndConstantImportsController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/GroupImportController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/IgnoredQualifiedNameController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/IgnoredTagNameController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportListWithAliasesController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportOnTheClassLineController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ImportsBeforeTheClassController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/LexerOneController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NameAcrossASeparatorController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NonAnnotationClassTagController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NotTopLevelAnnotationsController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/NotUtf8DocblockController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/OtherNamespace/LocalRestAnnotation.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/ParentWithDocblockOptions.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/QualifiedNameController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Required.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/SameLineImportController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/StarAndQuoteBeforeAnnotationController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Target.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TraitWithAnotherQueryImport.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TraitWithDocblockOptions.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/Traits/Query.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TwoImportsOnOneLineController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/TwoNamespacesController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/WhitespaceBeforeAnnotationController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/WrongCaseNameController.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aa8033..08c085b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,10 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Configuration::getConfigTreeBuilder()` declares its `TreeBuilder` return type and `PayseraApiExtension::load()` declares `void`. Breaking for subclasses that override either method without the return type: add `: TreeBuilder` or `: void` to the override -- On Symfony 7, loading a route whose controller configures it with the bundle's docblock annotations (`@Body`, `@Query`, - `@PathAttribute`, `@ResponseNormalization`, `@RequiredPermissions`, `@Validation`, `@BodyContentType`) fails with a - `ConfigurationException` that names the attributes to use instead. Symfony 7 does not read docblock annotations, so these - options were ignored without an error. Symfony 4.4 to 6.4 are unchanged +- Symfony 7 reads no docblock annotations, so on Symfony 7 the bundle's docblock annotations (`@Body`, `@Query`, + `@PathAttribute`, `@ResponseNormalization`, `@RequiredPermissions`, `@Validation`, `@BodyContentType`) have no effect: + configure those routes with the attributes of the same name. Symfony 4.4 to 6.4 are unchanged - Optional parameters are declared nullable explicitly (`?Type $parameter = null`), as PHP 8.4 expects - CI runs the tests on Symfony 7 with PHP 8.2 and 8.3 diff --git a/README.md b/README.md index 20da09c..ccf433b 100644 --- a/README.md +++ b/README.md @@ -647,9 +647,8 @@ cursor and iterating this way until we have `"has_previous": false` is a reliabl Every option below exists as a docblock annotation (`Paysera\Bundle\ApiBundle\Annotation\*`) and, since 1.8.0, as a PHP attribute of the same name (`Paysera\Bundle\ApiBundle\Attribute\*`, read on Symfony 6.4 and later). On Symfony 7 use the -attributes: Symfony 7 reads no docblock annotations, so loading a route whose controller still uses the bundle's docblock -annotations fails with an error naming the attributes to use, and the routes themselves need `#[Route]` with -`type: attribute` imports. The attributes take the same options, passed by name: +attributes: Symfony 7 reads no docblock annotations, so the bundle's docblock annotations have no effect there, and the +routes themselves need `#[Route]` with `type: attribute` imports. The attributes take the same options, passed by name: `@RequiredPermissions(permissions={"ROLE_ADMIN"})` becomes `#[RequiredPermissions(permissions: ['ROLE_ADMIN'])]`. ### `Body` diff --git a/src/Service/RoutingLoader/DocblockAnnotationFinder.php b/src/Service/RoutingLoader/DocblockAnnotationFinder.php deleted file mode 100644 index a84a74d..0000000 --- a/src/Service/RoutingLoader/DocblockAnnotationFinder.php +++ /dev/null @@ -1,262 +0,0 @@ -> - */ - private $importsByClass = []; - - /** - * @var bool - */ - private $bundleAnnotationsLoaded = false; - - /** - * @return string[] - */ - public function findBundleAnnotations(ReflectionClass $class, ReflectionMethod $method): array - { - $declaringClass = $method->getDeclaringClass(); - $methodImports = $this->readImports($declaringClass); - foreach ($declaringClass->getTraits() as $trait) { - if ($trait->hasMethod($method->getName()) && $trait->getFileName() === $method->getFileName()) { - $methodImports = array_merge($methodImports, $this->readImports($trait)); - } - } - - $found = array_merge( - $this->findInDocblock($class->getDocComment(), $this->readImports($class), $class->getNamespaceName()), - $this->findInDocblock($method->getDocComment(), $methodImports, $declaringClass->getNamespaceName()) - ); - - return array_values(array_unique($found)); - } - - /** - * @param string|false $docComment - * @param array $imports - * @return string[] - */ - private function findInDocblock($docComment, array $imports, string $namespace): array - { - if ($docComment === false || preg_match('/[ \t*]@/', $docComment, $start, PREG_OFFSET_CAPTURE) !== 1) { - return []; - } - - $text = substr($docComment, $start[0][1] + 1); - $found = []; - $offset = 0; - $pattern = self::TOKEN_PATTERN . (preg_match('//u', $text) === 1 ? 'iu' : 'i'); - while (preg_match($pattern, $text, $token, PREG_OFFSET_CAPTURE, $offset) === 1) { - $offset = $token[0][1] + strlen($token[0][0]); - $isString = !isset($token[1]); - $isFollowedByDash = isset($token[2]); - if ($isString || $isFollowedByDash) { - continue; - } - - $name = (string)preg_replace('/[\s*]++/u', '', $token[1][0]); - $importedName = $this->resolveImportedName($name, $imports); - $candidates = $importedName !== null ? [$importedName] : [$namespace . '\\' . $name, $name]; - $className = $this->findClass($candidates); - if ($className === null) { - continue; - } - if (is_subclass_of($className, RestAnnotationInterface::class)) { - $found[] = $className; - } - if ($importedName !== null - && $this->isAnnotationClass($className) - && preg_match(self::ARGUMENTS_PATTERN, $text, $arguments, 0, $offset) === 1 - ) { - $offset += strlen($arguments[0]); - } - } - - return $found; - } - - /** - * @param array $imports - */ - private function resolveImportedName(string $name, array $imports): ?string - { - if ($name[0] === '\\') { - return ltrim($name, '\\'); - } - - $parts = explode('\\', $name, 2); - $alias = strtolower($parts[0]); - if (!isset($imports[$alias])) { - return null; - } - - return $imports[$alias] . (isset($parts[1]) ? '\\' . $parts[1] : ''); - } - - /** - * @param string[] $candidates - */ - private function findClass(array $candidates): ?string - { - $this->loadBundleAnnotations(); - foreach ($candidates as $candidate) { - if (class_exists($candidate)) { - return (new ReflectionClass($candidate))->getName(); - } - } - - return null; - } - - private function loadBundleAnnotations(): void - { - if ($this->bundleAnnotationsLoaded) { - return; - } - $this->bundleAnnotationsLoaded = true; - $namespace = (new ReflectionClass(RestAnnotationInterface::class))->getNamespaceName(); - foreach (scandir(dirname(__DIR__, 2) . '/Annotation') ?: [] as $file) { - if (substr($file, -4) === '.php') { - class_exists($namespace . '\\' . substr($file, 0, -4)); - } - } - } - - private function isAnnotationClass(string $className): bool - { - return strpos((string)(new ReflectionClass($className))->getDocComment(), '@Annotation') !== false; - } - - /** - * @return array - */ - private function readImports(ReflectionClass $class): array - { - $className = $class->getName(); - if (!isset($this->importsByClass[$className])) { - $this->importsByClass[$className] = $this->parseImports($class); - } - - return $this->importsByClass[$className]; - } - - /** - * @return array - */ - private function parseImports(ReflectionClass $class): array - { - $fileName = $class->getFileName(); - if ($fileName === false || !is_file($fileName)) { - return []; - } - - $source = implode('', array_slice(file($fileName), 0, $class->getStartLine())); - $tokens = []; - foreach (token_get_all($source) as $token) { - if (!is_array($token) || !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { - $tokens[] = $token; - } - } - - $imports = []; - foreach ($tokens as $index => $token) { - if ($token[0] === T_USE) { - $imports = array_merge($imports, $this->parseUseStatement($tokens, $index + 1)); - } elseif ($token[0] === T_NAMESPACE - && $this->readName($tokens, $index + 1) === $class->getNamespaceName() - ) { - $imports = []; - } - } - - return $imports; - } - - /** - * @param array $tokens - * @return array - */ - private function parseUseStatement(array $tokens, int $index): array - { - $imports = []; - $groupPrefix = ''; - $name = ''; - $alias = ''; - $isAliasNext = false; - for (; isset($tokens[$index]); $index++) { - $token = $tokens[$index]; - if ($this->isNameToken($token)) { - if ($isAliasNext) { - $alias = $token[1]; - } else { - $name .= $token[1]; - $parts = explode('\\', $token[1]); - $alias = end($parts); - } - } elseif ($token[0] === T_AS) { - $isAliasNext = true; - } elseif ($token === ',' || $token === ';') { - $imports[strtolower($alias)] = $groupPrefix . $name; - if ($token === ';') { - break; - } - $name = ''; - $alias = ''; - $isAliasNext = false; - } elseif ($token === '{') { - $groupPrefix = $name; - $name = ''; - } elseif ($token !== '}') { - break; - } - } - - return $imports; - } - - /** - * @param array $tokens - */ - private function readName(array $tokens, int $index): string - { - $name = ''; - for (; isset($tokens[$index]) && $this->isNameToken($tokens[$index]); $index++) { - $name .= $tokens[$index][1]; - } - - return $name; - } - - /** - * @param string|array{0: int, 1: string, 2: int} $token - */ - private function isNameToken($token): bool - { - return is_array($token) && ( - in_array($token[0], [T_STRING, T_NS_SEPARATOR], true) - || (PHP_VERSION_ID >= 80000 && in_array($token[0], [T_NAME_QUALIFIED, T_NAME_FULLY_QUALIFIED], true)) - ); - } -} diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index 31536f9..955f27d 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -6,7 +6,6 @@ use Paysera\Bundle\ApiBundle\Annotation\RestAnnotationInterface; use Paysera\Bundle\ApiBundle\Attribute\RestAttributeInterface; -use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use ReflectionClass; use ReflectionMethod; @@ -18,9 +17,6 @@ */ class RoutingAttributeLoader extends AttributeRouteControllerLoader { - private const ANNOTATION_NAMESPACE = 'Paysera\\Bundle\\ApiBundle\\Annotation\\'; - private const ATTRIBUTE_NAMESPACE = 'Paysera\\Bundle\\ApiBundle\\Attribute\\'; - /** * @var RestRequestHelper */ @@ -36,11 +32,6 @@ class RoutingAttributeLoader extends AttributeRouteControllerLoader */ private $attributeOptionsBuilder; - /** - * @var DocblockAnnotationFinder|null - */ - private $docblockAnnotationFinder; - public function setRequestHelper(RestRequestHelper $restRequestHelper) { $this->restRequestHelper = $restRequestHelper; @@ -68,16 +59,8 @@ protected function configureRoute( $this->loadAttributes($route, $class, $method); } - /** - * @throws ConfigurationException - */ private function loadAnnotations(Route $route, ReflectionClass $class, ReflectionMethod $method): void { - if (!property_exists($this, 'reader')) { - $this->refuseDocblockAnnotations($class, $method); - return; - } - if (!isset($this->reader)) { return; } @@ -105,40 +88,6 @@ private function loadAnnotations(Route $route, ReflectionClass $class, Reflectio ); } - /** - * @throws ConfigurationException - */ - private function refuseDocblockAnnotations(ReflectionClass $class, ReflectionMethod $method): void - { - if ($this->docblockAnnotationFinder === null) { - $this->docblockAnnotationFinder = new DocblockAnnotationFinder(); - } - $annotations = $this->docblockAnnotationFinder->findBundleAnnotations($class, $method); - if ($annotations === []) { - return; - } - - $replacements = []; - foreach ($annotations as $annotation) { - $replacements[] = strpos($annotation, self::ANNOTATION_NAMESPACE) === 0 - ? '#[\\' . self::ATTRIBUTE_NAMESPACE . substr($annotation, strlen(self::ANNOTATION_NAMESPACE)) . ']' - : sprintf( - 'an attribute implementing \\%s in place of \\%s', - RestAttributeInterface::class, - $annotation - ); - } - - throw new ConfigurationException(sprintf( - '%s::%s() uses docblock annotations of paysera/lib-api-bundle (\\%s). Symfony 7 does not read docblock ' - . 'annotations, so they would have no effect. Use the PHP attributes instead: %s.', - $class->getName(), - $method->getName(), - implode(', \\', $annotations), - implode(', ', $replacements) - )); - } - private function loadAttributes(Route $route, ReflectionClass $class, ReflectionMethod $method): void { $attributes = array_merge($class->getAttributes(), $method->getAttributes()); diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php deleted file mode 100644 index 1545f68..0000000 --- a/tests/Unit/Service/RoutingLoader/DocblockAnnotationFinderTest.php +++ /dev/null @@ -1,452 +0,0 @@ -findBundleAnnotations( - new ReflectionClass($className), - new ReflectionMethod($className, $methodName) - ); - - $this->assertSame($expectedAnnotations, $annotations); - } - - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testResolvesABundleAnnotationWrittenInAnotherCaseBeforeAnythingLoadedIt() - { - $finder = new DocblockAnnotationFinder(); - - $annotations = $finder->findBundleAnnotations( - new ReflectionClass(WrongCaseNameController::class), - new ReflectionMethod(WrongCaseNameController::class, 'show') - ); - - $this->assertSame([RequiredPermissions::class], $annotations); - } - - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testResolvesAnotherCaseWhenTheBundleIsInstalledUnderAPathWithBrackets() - { - $directory = sys_get_temp_dir() . '/api-bundle-' . getmypid() . '[1]'; - $this->copyDirectory(dirname(__DIR__, 4) . '/src', $directory . '/src'); - spl_autoload_register(function (string $className) use ($directory) { - $prefix = 'Paysera\\Bundle\\ApiBundle\\'; - $file = $directory . '/src/' . str_replace('\\', '/', substr($className, strlen($prefix))) . '.php'; - if (strpos($className, $prefix) === 0 && is_file($file)) { - require $file; - } - }, true, true); - - try { - $annotations = (new DocblockAnnotationFinder())->findBundleAnnotations( - new ReflectionClass(WrongCaseNameController::class), - new ReflectionMethod(WrongCaseNameController::class, 'show') - ); - } finally { - $this->removeDirectory($directory); - } - - $this->assertSame( - [$directory . '/src/Service/RoutingLoader/DocblockAnnotationFinder.php', [RequiredPermissions::class]], - [(new ReflectionClass(DocblockAnnotationFinder::class))->getFileName(), $annotations] - ); - } - - public function testReadsAClassDeclaredInEvaluatedCode() - { - $testDouble = get_class($this->createMock(DirectImportController::class)); - $finder = new DocblockAnnotationFinder(); - - $annotations = $finder->findBundleAnnotations( - new ReflectionClass($testDouble), - new ReflectionMethod($testDouble, 'create') - ); - - $this->assertSame([], $annotations); - } - - /** - * @return array - */ - public static function controllerDataProvider(): array - { - return [ - 'class imports, class and method docblocks, each annotation once' => [ - DirectImportController::class, - 'create', - [RequiredPermissions::class, Body::class], - ], - 'namespace alias, class alias and a fully qualified name' => [ - AliasedImportController::class, - 'find', - [Query::class, Validation::class, PathAttribute::class, ResponseNormalization::class], - ], - 'attributes, Symfony tags and attribute class names are not the annotations' => [ - AttributeOnlyController::class, - 'create', - [], - ], - 'a method declared in a parent resolves through the parent file imports' => [ - ChildWithoutImports::class, - 'inherited', - [ResponseNormalization::class], - ], - 'a method the class declares over its trait\'s resolves through the class file imports only' => [ - ControllerOverridingTheTraitMethod::class, - 'find', - [Query::class], - ], - 'the class docblock resolves through its own file, the inherited method through the parent\'s' => [ - ChildWithOwnImports::class, - 'inherited', - [RequiredPermissions::class, ResponseNormalization::class], - ], - 'a class docblock resolves relative to its namespace' => [ - ClassLevelCustomAnnotationController::class, - 'show', - [CustomRestAnnotation::class], - ], - 'a name resolves in the namespace before a PHP class of the same name' => [ - ClassLevelCustomAnnotationController::class, - 'list', - [CustomRestAnnotation::class, Directory::class], - ], - 'each of two classes in one file reads its own namespace block' => [ - ChildOfAParentInAnotherNamespace::class, - 'create', - [RequiredPermissions::class, CustomRestAnnotation::class, Body::class, LocalRestAnnotation::class], - ], - 'an import on the class\'s own line counts' => [ - ImportOnTheClassLineController::class, - 'find', - [Query::class], - ], - 'a method from a trait resolves through the trait file imports' => [ - ControllerUsingTheTrait::class, - 'fromTrait', - [Query::class], - ], - 'a group import with an alias' => [ - GroupImportController::class, - 'create', - [RequiredPermissions::class, Body::class], - ], - 'a comma-separated import over two lines' => [ - CommaImportController::class, - 'create', - [RequiredPermissions::class, Body::class], - ], - 'an import on the namespace line, and a full name without the leading backslash' => [ - SameLineImportController::class, - 'create', - [Body::class, RequiredPermissions::class], - ], - 'comments inside use statements' => [ - CommentedImportsController::class, - 'find', - [Query::class, RequiredPermissions::class, Validation::class], - ], - 'two use statements on one line' => [ - TwoImportsOnOneLineController::class, - 'create', - [Body::class, RequiredPermissions::class], - ], - 'only the imports above the class count: not a commented-out one, not a trait in the class body' => [ - ImportsBeforeTheClassController::class, - 'find', - [Query::class, RequiredPermissions::class], - ], - 'a function or constant import of the same name does not replace a class import' => [ - FunctionAndConstantImportsController::class, - 'show', - [RequiredPermissions::class], - ], - 'an import in another namespace block of the file does not count' => [ - TwoNamespacesController::class, - 'show', - [CustomRestAnnotation::class], - ], - 'an "@" right after a word or "{" does not start an annotation, and a nested one is part of its parent' => [ - NotTopLevelAnnotationsController::class, - 'find', - [Query::class], - ], - 'a nested annotation after a space is part of its parent too' => [ - NotTopLevelAnnotationsController::class, - 'findWithASpaceBeforeTheNestedAnnotation', - [Query::class], - ], - 'reading starts at the first "@" after a space, a tab or "*", as in Doctrine' => [ - NotTopLevelAnnotationsController::class, - 'annotationAtTheStartOfALine', - [], - ], - 'an annotation at the start of a line is skipped before the first one Doctrine reads' => [ - NotTopLevelAnnotationsController::class, - 'annotationAfterOneAtTheStartOfALine', - [Query::class], - ], - 'an "@" inside a quoted string is text' => [ - NotTopLevelAnnotationsController::class, - 'annotationInAString', - [], - ], - 'a name followed by "-" is not an annotation' => [ - NotTopLevelAnnotationsController::class, - 'annotationFollowedByADash', - [], - ], - 'the parentheses after a class that is not an annotation are read, as in Doctrine' => [ - NonAnnotationClassTagController::class, - 'show', - [RequiredPermissions::class], - ], - 'after a name with a namespace that is not imported the arguments are read, as Doctrine may ignore it' => [ - QualifiedNameController::class, - 'withoutTheLeadingBackslash', - [Query::class, Validation::class], - ], - 'a name with a namespace that the class tells Doctrine to ignore hides nothing' => [ - IgnoredQualifiedNameController::class, - 'show', - [RequiredPermissions::class], - ], - 'a fully qualified name keeps its arguments' => [ - QualifiedNameController::class, - 'withTheLeadingBackslash', - [Query::class], - ], - 'two leading backslashes, as Doctrine strips them all' => [ - QualifiedNameController::class, - 'withTwoLeadingBackslashes', - [RequiredPermissions::class], - ], - 'three leading backslashes' => [ - QualifiedNameController::class, - 'withThreeLeadingBackslashes', - [RequiredPermissions::class], - ], - 'a name continues after a separator and a space, as Doctrine joins it' => [ - NameAcrossASeparatorController::class, - 'spaceAfterTheSeparator', - [RequiredPermissions::class], - ], - 'a name continues after a separator and a line break' => [ - NameAcrossASeparatorController::class, - 'lineBreakAfterTheSeparator', - [RequiredPermissions::class], - ], - 'a name continues after a separator and a star' => [ - NameAcrossASeparatorController::class, - 'starAfterTheSeparator', - [RequiredPermissions::class], - ], - 'a name continues across two separators with spaces' => [ - NameAcrossASeparatorController::class, - 'twoSeparatorsWithSpaces', - [RequiredPermissions::class], - ], - 'a name continues after a separator and a no-break space' => [ - NameAcrossASeparatorController::class, - 'noBreakSpaceAfterTheSeparator', - [RequiredPermissions::class], - ], - 'a name does not take the next line\'s words without a separator' => [ - NameAcrossASeparatorController::class, - 'nameFollowedByTextOnTheNextLine', - [ResponseNormalization::class], - ], - 'a no-break space before the arguments leaves them unread, as doctrine/lexer 1.0 read them' => [ - LexerOneController::class, - 'noBreakSpaceBeforeTheArguments', - [RequiredPermissions::class], - ], - 'a docblock with a byte that is not UTF-8 is still read, as doctrine/lexer 1.0 read it' => [ - NotUtf8DocblockController::class, - 'show', - [RequiredPermissions::class], - ], - 'a class named like a tag Doctrine ignores does not hide what its parentheses hold' => [ - IgnoredTagNameController::class, - 'show', - [RequiredPermissions::class], - ], - 'an annotation without arguments does not take the next one\'s' => [ - ArgumentBoundariesController::class, - 'afterAnAnnotationWithoutArguments', - [ResponseNormalization::class, RequiredPermissions::class], - ], - 'a parenthesis in a quoted argument does not end the arguments' => [ - ArgumentBoundariesController::class, - 'afterAParenthesisInAString', - [Query::class, RequiredPermissions::class], - ], - 'arguments after a space are still the annotation\'s' => [ - ArgumentBoundariesController::class, - 'argumentsAfterASpace', - [Query::class], - ], - 'a one-letter alias, aliases in a comma list and an import written with a leading backslash' => [ - ImportListWithAliasesController::class, - 'create', - [RequiredPermissions::class, Body::class, Validation::class, Query::class], - ], - 'reading can start at an "@" right after a star' => [ - StarAndQuoteBeforeAnnotationController::class, - 'firstRightAfterAStar', - [RequiredPermissions::class], - ], - 'an "@" right after a star starts an annotation' => [ - StarAndQuoteBeforeAnnotationController::class, - 'laterAfterAStar', - [ResponseNormalization::class, RequiredPermissions::class], - ], - 'reading does not start at an "@" glued to a quote' => [ - StarAndQuoteBeforeAnnotationController::class, - 'firstGluedToAQuote', - [], - ], - 'an "@" right after a closing quote starts an annotation, as in Doctrine' => [ - StarAndQuoteBeforeAnnotationController::class, - 'afterAClosingQuote', - [RequiredPermissions::class], - ], - 'an "@" right after a lone quote starts one too' => [ - StarAndQuoteBeforeAnnotationController::class, - 'afterALoneQuote', - [ResponseNormalization::class], - ], - 'a ":" after a name leaves it an annotation' => [ - ArgumentBoundariesController::class, - 'colonAfterAName', - [ResponseNormalization::class], - ], - 'the parentheses after an imported class that is not an annotation are read' => [ - NonAnnotationClassTagController::class, - 'afterAnImportedNonAnnotationClass', - [RequiredPermissions::class], - ], - 'a name followed by a negative number is an annotation, as in Doctrine' => [ - ArgumentBoundariesController::class, - 'nameFollowedByANegativeNumber', - [ResponseNormalization::class], - ], - 'a no-break space before an annotation is whitespace, as in Doctrine\'s lexer' => [ - WhitespaceBeforeAnnotationController::class, - 'afterANoBreakSpace', - [RequiredPermissions::class], - ], - 'reading can start at an "@" after a tab' => [ - WhitespaceBeforeAnnotationController::class, - 'afterATab', - [RequiredPermissions::class], - ], - 'a class of PHP itself has no docblocks' => [ - ArrayObject::class, - 'count', - [], - ], - 'an application\'s own annotation class in the same namespace' => [ - CustomAnnotationOnAttributeRouteController::class, - 'show', - [CustomRestAnnotation::class], - ], - ]; - } - - private function copyDirectory(string $source, string $target) - { - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($source, FilesystemIterator::SKIP_DOTS), - RecursiveIteratorIterator::SELF_FIRST - ); - mkdir($target, 0777, true); - foreach ($files as $file) { - $path = $target . '/' . substr($file->getPathname(), strlen($source) + 1); - $file->isDir() ? mkdir($path) : copy($file->getPathname(), $path); - } - } - - private function removeDirectory(string $directory) - { - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($directory, FilesystemIterator::SKIP_DOTS), - RecursiveIteratorIterator::CHILD_FIRST - ); - foreach ($files as $file) { - $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); - } - rmdir($directory); - } -} diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php deleted file mode 100644 index 3e8699f..0000000 --- a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedImportController.php +++ /dev/null @@ -1,22 +0,0 @@ - Date: Mon, 28 Sep 2026 17:17:19 +0530 Subject: [PATCH 40/42] EE-283 Keep only the tests for the code this pull request changes The new test files for classes the pull request does not change are removed (ErrorBuilder, RestResponseListener, PathAttributeResolverRegistry, the option entities, attribute parameter resolution, RoutingAttributeLoader and its two fixtures), and the RestRequestHelper and EntityValidator tests are back to master's. They were added to raise line coverage, not to test this change. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../AttributeParameterResolutionTest.php | 87 ------ tests/Unit/Entity/UnsetOptionsTest.php | 96 ------- .../Listener/RestResponseListenerTest.php | 62 ----- tests/Unit/Service/ErrorBuilderTest.php | 250 ------------------ .../PathAttributeResolverRegistryTest.php | 33 --- tests/Unit/Service/RestRequestHelperTest.php | 12 - .../Fixtures/AttributeOnlyController.php | 25 -- ...blockOptionsOnAttributeRouteController.php | 19 -- .../RoutingAttributeLoaderTest.php | 120 --------- .../Validation/EntityValidatorTest.php | 13 +- 10 files changed, 1 insertion(+), 716 deletions(-) delete mode 100644 tests/Unit/Attribute/AttributeParameterResolutionTest.php delete mode 100644 tests/Unit/Entity/UnsetOptionsTest.php delete mode 100644 tests/Unit/Listener/RestResponseListenerTest.php delete mode 100644 tests/Unit/Service/ErrorBuilderTest.php delete mode 100644 tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php delete mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php delete mode 100644 tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php diff --git a/tests/Unit/Attribute/AttributeParameterResolutionTest.php b/tests/Unit/Attribute/AttributeParameterResolutionTest.php deleted file mode 100644 index 30cfddf..0000000 --- a/tests/Unit/Attribute/AttributeParameterResolutionTest.php +++ /dev/null @@ -1,87 +0,0 @@ - $attributeOptions - */ - public function testCannotGuessTheTypeOfAnUntypedParameter(string $attributeClass, array $attributeOptions) - { - $attribute = new $attributeClass($attributeOptions); - - $this->expectException(ConfigurationException::class); - $this->expectExceptionMessage('Denormalization type could not be guessed for $item in '); - - $attribute->apply(new RestRequestOptions(), $this->wrapUntypedAction()); - } - - /** - * @return array}> - */ - public static function untypedParameterAttributeDataProvider(): array - { - return [ - 'path attribute' => [PathAttribute::class, ['parameterName' => 'item', 'pathPartName' => 'id']], - 'query' => [Query::class, ['parameterName' => 'item']], - ]; - } - - public function testExplicitArgumentsWinOverTheSignature() - { - $options = new RestRequestOptions(); - - (new PathAttribute([ - 'parameterName' => 'item', - 'pathPartName' => 'id', - 'resolverType' => 'custom_resolver', - 'resolutionMandatory' => false, - ]))->apply($options, $this->wrapUntypedAction()); - (new Body([ - 'parameterName' => 'item', - 'denormalizationType' => 'custom_type', - 'optional' => true, - ]))->apply($options, $this->wrapUntypedAction()); - - $expectedOptions = (new RestRequestOptions()) - ->addPathAttributeResolverOptions( - (new PathAttributeResolverOptions()) - ->setParameterName('item') - ->setPathPartName('id') - ->setPathAttributeResolverType('custom_resolver') - ->setResolutionMandatory(false) - ) - ->setBodyParameterName('item') - ->setBodyDenormalizationType('custom_type') - ->setBodyOptional(true) - ; - $this->assertEquals($expectedOptions, $options); - } - - /** - * @param mixed $item - */ - public function untypedAction($item) - { - } - - private function wrapUntypedAction(): ReflectionMethodWrapper - { - return new ReflectionMethodWrapper(new ReflectionMethod(self::class, 'untypedAction')); - } -} diff --git a/tests/Unit/Entity/UnsetOptionsTest.php b/tests/Unit/Entity/UnsetOptionsTest.php deleted file mode 100644 index 3f37914..0000000 --- a/tests/Unit/Entity/UnsetOptionsTest.php +++ /dev/null @@ -1,96 +0,0 @@ -expectException(RuntimeException::class); - $this->expectExceptionMessage($expectedMessage); - - $readOption(); - } - - /** - * @return array - */ - public static function unsetOptionDataProvider(): array - { - return [ - 'path attribute parameter name' => [ - function () { - (new PathAttributeResolverOptions())->getParameterName(); - }, - 'parameterName was not set', - ], - 'path attribute path part name' => [ - function () { - (new PathAttributeResolverOptions())->getPathPartName(); - }, - 'pathPartName was not set', - ], - 'path attribute resolver type' => [ - function () { - (new PathAttributeResolverOptions())->getPathAttributeResolverType(); - }, - 'pathAttributeResolverType was not set', - ], - 'query parameter name' => [ - function () { - (new QueryResolverOptions())->getParameterName(); - }, - 'parameterName was not set', - ], - 'query denormalization type' => [ - function () { - (new QueryResolverOptions())->getDenormalizationType(); - }, - 'denormalizationType was not set', - ], - 'query validation options after they were set to null' => [ - function () { - (new QueryResolverOptions())->setValidationOptions(null)->getValidationOptions(); - }, - 'No validationOptions available, call isValidationNeeded beforehand', - ], - 'body denormalization type' => [ - function () { - (new RestRequestOptions())->getBodyDenormalizationType(); - }, - 'No bodyDenormalizationType available, call hasBodyDenormalization beforehand', - ], - 'body parameter name' => [ - function () { - (new RestRequestOptions())->getBodyParameterName(); - }, - 'No bodyParameterName available, call hasBodyDenormalization beforehand', - ], - 'body validation options after validation was disabled' => [ - function () { - (new RestRequestOptions())->disableBodyValidation()->getBodyValidationOptions(); - }, - 'No bodyValidationOptions available, call isBodyValidationNeeded beforehand', - ], - ]; - } - - public function testErrorKeepsItsUri() - { - $error = (new Error())->setUri('https://example.com/errors/not_found'); - - $this->assertSame('https://example.com/errors/not_found', $error->getUri()); - } -} diff --git a/tests/Unit/Listener/RestResponseListenerTest.php b/tests/Unit/Listener/RestResponseListenerTest.php deleted file mode 100644 index 849f312..0000000 --- a/tests/Unit/Listener/RestResponseListenerTest.php +++ /dev/null @@ -1,62 +0,0 @@ -shouldReceive('isRestRequest')->andReturn(false); - $event = $this->createViewEvent(['a' => 'result']); - - $this->createListener($requestHelper, Mockery::mock(ResponseBuilder::class))->onKernelView($event); - - $this->assertNull($event->getResponse()); - } - - public function testAnswersAControllerThatReturnsNothingWithAnEmptyResponse() - { - $requestHelper = Mockery::mock(RestRequestHelper::class); - $requestHelper->shouldReceive('isRestRequest')->andReturn(true); - $emptyResponse = new Response('', Response::HTTP_NO_CONTENT); - $responseBuilder = Mockery::mock(ResponseBuilder::class); - $responseBuilder->shouldReceive('buildEmptyResponse')->once()->andReturn($emptyResponse); - $event = $this->createViewEvent(null); - - $this->createListener($requestHelper, $responseBuilder)->onKernelView($event); - - $this->assertSame($emptyResponse, $event->getResponse()); - } - - private function createListener($requestHelper, $responseBuilder): RestResponseListener - { - return new RestResponseListener(Mockery::mock(CoreNormalizer::class), $requestHelper, $responseBuilder); - } - - private function createViewEvent($controllerResult) - { - $kernel = Mockery::mock(HttpKernelInterface::class); - $requestType = HttpKernelHelper::getMainRequestConstValue(); - if (class_exists(ViewEvent::class)) { - return new ViewEvent($kernel, new Request(), $requestType, $controllerResult); - } - - return new GetResponseForControllerResultEvent($kernel, new Request(), $requestType, $controllerResult); - } -} diff --git a/tests/Unit/Service/ErrorBuilderTest.php b/tests/Unit/Service/ErrorBuilderTest.php deleted file mode 100644 index 85f0bdc..0000000 --- a/tests/Unit/Service/ErrorBuilderTest.php +++ /dev/null @@ -1,250 +0,0 @@ -createConfiguredErrorBuilder()->createErrorFromException($exception); - - $this->assertSame( - [ - 'code' => $expectedCode, - 'statusCode' => $expectedStatusCode, - 'uri' => null, - 'message' => $expectedMessage, - 'properties' => null, - 'data' => null, - 'violations' => [], - ], - $this->readError($error) - ); - } - - /** - * @return array - */ - public static function exceptionDataProvider(): array - { - $tooLargeOffset = new TooLargeOffsetException(1000, 1001); - $invalidOrderBy = new InvalidOrderByException('bogus_field'); - - return [ - 'API exception with its own code and status' => [ - new ApiException('custom_code', 'Custom message', 418), - 'custom_code', - 418, - 'Custom message', - ], - 'API exception with a configured code' => [ - new ApiException(ApiException::NOT_FOUND), - 'not_found', - 404, - 'Resource was not found', - ], - 'API exception with the invalid parameters code' => [ - new ApiException(ApiException::INVALID_PARAMETERS), - 'invalid_parameters', - 400, - 'Some required parameter is missing or it\'s format is invalid', - ], - 'API exception with the invalid state code' => [ - new ApiException(ApiException::INVALID_STATE), - 'invalid_state', - 409, - 'Requested action cannot be made to the current state of resource', - ], - 'API exception with the not acceptable code' => [ - new ApiException(ApiException::NOT_ACCEPTABLE), - 'not_acceptable', - 406, - 'Unknown request or response format', - ], - 'API exception with the internal server error code' => [ - new ApiException(ApiException::INTERNAL_SERVER_ERROR), - 'internal_server_error', - 500, - 'Unexpected internal system error', - ], - 'API exception with an unconfigured code' => [ - new ApiException('unconfigured_code'), - 'unconfigured_code', - 400, - null, - ], - 'invalid data' => [new InvalidDataException('Bad data'), 'invalid_parameters', 400, 'Bad data'], - 'invalid item' => [new InvalidItemException('amount'), 'invalid_parameters', 400, 'Invalid key "amount"'], - 'offset over the maximum' => [ - $tooLargeOffset, - 'offset_too_large', - 400, - $tooLargeOffset->getMessage(), - ], - 'invalid cursor without a message' => [ - new InvalidCursorException(), - 'invalid_cursor', - 400, - 'Provided cursor is invalid', - ], - 'invalid cursor with a message' => [ - new InvalidCursorException('Bad cursor'), - 'invalid_cursor', - 400, - 'Bad cursor', - ], - 'unsupported order-by field' => [ - $invalidOrderBy, - 'invalid_parameters', - 400, - $invalidOrderBy->getMessage(), - ], - 'no credentials' => [ - new AuthenticationCredentialsNotFoundException(), - 'unauthorized', - 401, - 'No authorization data found', - ], - 'authentication failure' => [ - new AuthenticationException('Internal detail'), - 'unauthorized', - 401, - 'You have not provided any credentials or they are invalid', - ], - 'authentication failure meant for the client (code 999)' => [ - new AuthenticationException('Token expired', 999), - 'unauthorized', - 401, - 'Token expired', - ], - 'access denied by security' => [ - new AccessDeniedException('Access Denied.'), - 'forbidden', - 403, - 'Access Denied.', - ], - 'access denied over HTTP' => [new AccessDeniedHttpException('Denied'), 'forbidden', 403, 'Denied'], - 'no route' => [new ResourceNotFoundException(), 'not_found', 404, 'Provided url not found'], - 'not found over HTTP' => [new NotFoundHttpException(), 'not_found', 404, 'Provided url not found'], - 'method not allowed by routing' => [ - new MethodNotAllowedException(['GET']), - 'not_found', - 405, - 'Provided method not allowed for this url', - ], - 'HTTP 404' => [new HttpException(404), 'not_found', 404, 'Resource was not found'], - 'HTTP 405' => [new HttpException(405), 'not_found', 405, 'Provided method not allowed for this url'], - 'HTTP 401' => [ - new HttpException(401), - 'unauthorized', - 401, - 'You have not provided any credentials or they are invalid', - ], - 'HTTP 403' => [ - new HttpException(403), - 'forbidden', - 403, - 'You have no rights to access requested resource or make requested action', - ], - 'HTTP 400' => [new HttpException(400), 'invalid_request', 400, 'Request content is invalid'], - 'other HTTP client error' => [ - new HttpException(418), - 'internal_server_error', - 500, - 'Unexpected internal system error', - ], - 'HTTP server error' => [ - new HttpException(503), - 'internal_server_error', - 500, - 'Unexpected internal system error', - ], - 'any other exception' => [ - new RuntimeException('Secret detail'), - 'internal_server_error', - 500, - 'Unexpected internal system error', - ], - ]; - } - - public function testCarriesTheApiExceptionDetails() - { - $violation = (new Violation())->setField('amount')->setMessage('Too large'); - $exception = (new ApiException('custom_code')) - ->setProperties(['amount' => 'Too large']) - ->setData(['limit' => 100]) - ->setViolations([$violation]) - ; - - $error = $this->createConfiguredErrorBuilder()->createErrorFromException($exception); - - $this->assertSame( - [ - 'code' => 'custom_code', - 'statusCode' => 400, - 'uri' => null, - 'message' => null, - 'properties' => ['amount' => 'Too large'], - 'data' => ['limit' => 100], - 'violations' => [$violation], - ], - $this->readError($error) - ); - } - - private function createConfiguredErrorBuilder(): ErrorBuilder - { - $container = new ContainerBuilder(); - $loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../src/Resources/config')); - $loader->load('services.xml'); - - return $container->get('paysera_api.error_builder'); - } - - private function readError(Error $error): array - { - return [ - 'code' => $error->getCode(), - 'statusCode' => $error->getStatusCode(), - 'uri' => $error->getUri(), - 'message' => $error->getMessage(), - 'properties' => $error->getProperties(), - 'data' => $error->getData(), - 'violations' => $error->getViolations(), - ]; - } -} diff --git a/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php b/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php deleted file mode 100644 index 924024d..0000000 --- a/tests/Unit/Service/PathAttributeResolver/PathAttributeResolverRegistryTest.php +++ /dev/null @@ -1,33 +0,0 @@ -registerPathAttributeResolver($resolver, 'user'); - - $this->assertSame($resolver, $registry->getResolverByType('user')); - } - - public function testFailsForAnUnregisteredType() - { - $registry = new PathAttributeResolverRegistry(); - - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('No such path attribute resolver registered: "user"'); - - $registry->getResolverByType('user'); - } -} diff --git a/tests/Unit/Service/RestRequestHelperTest.php b/tests/Unit/Service/RestRequestHelperTest.php index 7c97af2..6da258c 100644 --- a/tests/Unit/Service/RestRequestHelperTest.php +++ b/tests/Unit/Service/RestRequestHelperTest.php @@ -63,18 +63,6 @@ public function testResolveRestRequestOptionsWithNoRegisteredOptions() $this->assertNull($helper->resolveRestRequestOptionsForRequest($request)); } - public function testResolvesNoOptionsForAControllerWithoutIdentifierThatIsNotAClassMethodPair() - { - $registry = Mockery::mock(RestRequestOptionsRegistry::class); - $registry->shouldNotReceive('getRestRequestOptionsForController'); - $helper = new RestRequestHelper($registry); - - $options = $helper->resolveRestRequestOptionsForController(new Request(), function () { - }); - - $this->assertNull($options); - } - public function testResolveRestRequestOptionsWithRegisteredOptionsAndCustomController() { $registry = Mockery::mock(RestRequestOptionsRegistry::class); diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php deleted file mode 100644 index 0a2b87d..0000000 --- a/tests/Unit/Service/RoutingLoader/Fixtures/AttributeOnlyController.php +++ /dev/null @@ -1,25 +0,0 @@ -requestHelper = Mockery::mock(RestRequestHelper::class); - $this->annotationOptionsBuilder = Mockery::mock(RestRequestAnnotationOptionsBuilder::class); - $this->attributeOptionsBuilder = Mockery::mock(RestRequestAttributeOptionsBuilder::class); - } - - public function testLoadsTheBundleAttributesWhereSymfonyReadsNoDocblocks() - { - $this->skipUnlessTheRouteLoaderHasNoAnnotationReader(); - $loader = $this->createLoader(); - $options = new RestRequestOptions(); - $this->attributeOptionsBuilder->shouldReceive('buildOptions')->once()->andReturn($options); - $this->requestHelper->shouldReceive('setOptionsForRoute')->once()->with(Mockery::any(), $options); - - $routes = $loader->load(AttributeOnlyController::class); - - $this->assertCount(1, $routes); - } - - public function testAppliesTheBundleDocblockAnnotationsThroughTheReaderBeforeSymfony7() - { - $this->skipUnlessTheRouteLoaderHasAnAnnotationReader(); - $loader = $this->createLoader(new AnnotationReader()); - $options = new RestRequestOptions(); - $this->annotationOptionsBuilder - ->shouldReceive('buildOptions') - ->once() - ->with(Mockery::on(function (array $annotations) { - return count($annotations) === 1 && $annotations[0] instanceof RequiredPermissions; - }), Mockery::any()) - ->andReturn($options) - ; - $this->requestHelper->shouldReceive('setOptionsForRoute')->once()->with(Mockery::any(), $options); - - $routes = $loader->load(DocblockOptionsOnAttributeRouteController::class); - - $this->assertCount(1, $routes); - } - - public function testIgnoresTheBundleDocblockAnnotationsWithoutAnAnnotationReader() - { - if (!class_exists(AttributeRouteControllerLoader::class)) { - $this->markTestSkipped('Needs the attribute route loader of Symfony 6.4 and later'); - } - $loader = $this->createLoader(); - $this->annotationOptionsBuilder->shouldNotReceive('buildOptions'); - $this->requestHelper->shouldNotReceive('setOptionsForRoute'); - - $routes = $loader->load(DocblockOptionsOnAttributeRouteController::class); - - $this->assertCount(1, $routes); - } - - private function skipUnlessTheRouteLoaderHasAnAnnotationReader() - { - if (!class_exists(AttributeRouteControllerLoader::class) - || !property_exists(AttributeRouteControllerLoader::class, 'reader') - ) { - $this->markTestSkipped('Needs Symfony 6.4: the attribute route loader with an annotation reader property'); - } - } - - private function skipUnlessTheRouteLoaderHasNoAnnotationReader() - { - if (!class_exists(AttributeRouteControllerLoader::class) - || property_exists(AttributeRouteControllerLoader::class, 'reader') - ) { - $this->markTestSkipped('Symfony 6.4 and older give the route loader an annotation reader'); - } - } - - private function createLoader(...$constructorArguments): RoutingAttributeLoader - { - $loader = new RoutingAttributeLoader(...$constructorArguments); - $loader->setRequestHelper($this->requestHelper); - $loader->setRestRequestAnnotationOptionsBuilder($this->annotationOptionsBuilder); - $loader->setRestRequestAttributeOptionsBuilder($this->attributeOptionsBuilder); - - return $loader; - } -} diff --git a/tests/Unit/Service/Validation/EntityValidatorTest.php b/tests/Unit/Service/Validation/EntityValidatorTest.php index 6574634..6f53884 100644 --- a/tests/Unit/Service/Validation/EntityValidatorTest.php +++ b/tests/Unit/Service/Validation/EntityValidatorTest.php @@ -9,7 +9,6 @@ use Paysera\Bundle\ApiBundle\Exception\ApiException; use Paysera\Bundle\ApiBundle\Service\Validation\EntityValidator; use Paysera\Bundle\ApiBundle\Service\Validation\PropertyPathConverterInterface; -use RuntimeException; use Mockery\Adapter\Phpunit\MockeryTestCase; use stdClass; use Symfony\Component\Validator\Constraints\Type; @@ -20,16 +19,6 @@ class EntityValidatorTest extends MockeryTestCase { - public function testValidateRequiresTheValidator() - { - $entityValidator = new EntityValidator(null, Mockery::mock(PropertyPathConverterInterface::class)); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('To use validation in RestBundle you must configure framework.validation'); - - $entityValidator->validate(new stdClass(), new ValidationOptions()); - } - public function testValidateDoesNotFailWithNonObject() { $validator = Mockery::mock(ValidatorInterface::class); @@ -57,7 +46,6 @@ public function testValidate($expectedException, ValidationOptions $validationOp $entity = new stdClass(); $validator ->shouldReceive('validate') - ->once() ->with($entity, null, $groups) ->andReturn(new ConstraintViolationList($violationList)) ; @@ -79,6 +67,7 @@ public function testValidate($expectedException, ValidationOptions $validationOp if ($expectedException !== null) { $this->fail('Expected exception'); } + $this->expectNotToPerformAssertions(); } catch (ApiException $exception) { $this->assertEquals($expectedException, $exception); } From 3e2125521667fea4de6c6a7093f408b08f70bab2 Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Mon, 28 Sep 2026 17:33:31 +0530 Subject: [PATCH 41/42] EE-283 Test that Symfony 7 refuses a route configured by the bundle's docblock annotations Symfony 7 reads no docblock annotations, so a leftover @RequiredPermissions would leave its endpoint without the permission check. Loading such a route must fail and name the attributes to use. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...blockOptionsOnAttributeRouteController.php | 19 +++++++++++ .../RoutingAttributeLoaderTest.php | 32 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php create mode 100644 tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php b/tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php new file mode 100644 index 0000000..384715d --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/DocblockOptionsOnAttributeRouteController.php @@ -0,0 +1,19 @@ +markTestSkipped('Symfony 6.4 and older read docblock annotations through an annotation reader'); + } + + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage( + DocblockOptionsOnAttributeRouteController::class . '::show() uses docblock annotations of ' + . 'paysera/lib-api-bundle (@RequiredPermissions), which Symfony 7 does not read. Use the attributes of the ' + . 'same name from Paysera\Bundle\ApiBundle\Attribute instead.' + ); + + (new RoutingAttributeLoader())->load(DocblockOptionsOnAttributeRouteController::class); + } +} From 9f13c4c3b03a703a9378fac6e66764498584a21d Mon Sep 17 00:00:00 2001 From: Vinayak Iyer Date: Mon, 28 Sep 2026 17:42:53 +0530 Subject: [PATCH 42/42] EE-283 Refuse the bundle's docblock annotations on Symfony 7 by name On Symfony 7 the route loader has no annotation reader, so a leftover @RequiredPermissions would leave its endpoint without the permission check. If the method or class docblock of a route names one of the bundle's seven annotations, loading the route now fails with a ConfigurationException that names them and points to the attributes of the same name. This replaces the Doctrine-rules reader removed two commits earlier with a match on names only, to keep the pull request at the size of the fix. Symfony 4.4 to 6.4 keep reading the annotations as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 ++-- README.md | 4 +- .../RoutingLoader/RoutingAttributeLoader.php | 41 +++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08c085b..001bb0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Configuration::getConfigTreeBuilder()` declares its `TreeBuilder` return type and `PayseraApiExtension::load()` declares `void`. Breaking for subclasses that override either method without the return type: add `: TreeBuilder` or `: void` to the override -- Symfony 7 reads no docblock annotations, so on Symfony 7 the bundle's docblock annotations (`@Body`, `@Query`, - `@PathAttribute`, `@ResponseNormalization`, `@RequiredPermissions`, `@Validation`, `@BodyContentType`) have no effect: - configure those routes with the attributes of the same name. Symfony 4.4 to 6.4 are unchanged +- Symfony 7 reads no docblock annotations, so on Symfony 7 loading a route whose controller still uses the bundle's + docblock annotations (`@Body`, `@Query`, `@PathAttribute`, `@ResponseNormalization`, `@RequiredPermissions`, + `@Validation`, `@BodyContentType`) fails with a `ConfigurationException` naming them: use the attributes of the same + name. Symfony 4.4 to 6.4 are unchanged - Optional parameters are declared nullable explicitly (`?Type $parameter = null`), as PHP 8.4 expects - CI runs the tests on Symfony 7 with PHP 8.2 and 8.3 diff --git a/README.md b/README.md index ccf433b..0f6c466 100644 --- a/README.md +++ b/README.md @@ -647,8 +647,8 @@ cursor and iterating this way until we have `"has_previous": false` is a reliabl Every option below exists as a docblock annotation (`Paysera\Bundle\ApiBundle\Annotation\*`) and, since 1.8.0, as a PHP attribute of the same name (`Paysera\Bundle\ApiBundle\Attribute\*`, read on Symfony 6.4 and later). On Symfony 7 use the -attributes: Symfony 7 reads no docblock annotations, so the bundle's docblock annotations have no effect there, and the -routes themselves need `#[Route]` with `type: attribute` imports. The attributes take the same options, passed by name: +attributes: Symfony 7 reads no docblock annotations, so loading a route whose controller still uses the bundle's docblock +annotations fails with an error naming them, and the routes themselves need `#[Route]` with `type: attribute` imports. The attributes take the same options, passed by name: `@RequiredPermissions(permissions={"ROLE_ADMIN"})` becomes `#[RequiredPermissions(permissions: ['ROLE_ADMIN'])]`. ### `Body` diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index 955f27d..1774270 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -6,6 +6,7 @@ use Paysera\Bundle\ApiBundle\Annotation\RestAnnotationInterface; use Paysera\Bundle\ApiBundle\Attribute\RestAttributeInterface; +use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use ReflectionClass; use ReflectionMethod; @@ -17,6 +18,16 @@ */ class RoutingAttributeLoader extends AttributeRouteControllerLoader { + private const ANNOTATION_NAMES = [ + 'Body', + 'BodyContentType', + 'PathAttribute', + 'Query', + 'RequiredPermissions', + 'ResponseNormalization', + 'Validation', + ]; + /** * @var RestRequestHelper */ @@ -59,8 +70,17 @@ protected function configureRoute( $this->loadAttributes($route, $class, $method); } + /** + * @throws ConfigurationException + */ private function loadAnnotations(Route $route, ReflectionClass $class, ReflectionMethod $method): void { + if (!property_exists($this, 'reader')) { + $this->refuseDocblockAnnotations($class, $method); + + return; + } + if (!isset($this->reader)) { return; } @@ -88,6 +108,27 @@ private function loadAnnotations(Route $route, ReflectionClass $class, Reflectio ); } + /** + * @throws ConfigurationException + */ + private function refuseDocblockAnnotations(ReflectionClass $class, ReflectionMethod $method): void + { + $pattern = '/(?getDocComment() . $method->getDocComment(), $matches); + $names = array_values(array_unique($matches[1])); + if ($names === []) { + return; + } + + throw new ConfigurationException(sprintf( + '%s::%s() uses docblock annotations of paysera/lib-api-bundle (@%s), which Symfony 7 does not read. ' + . 'Use the attributes of the same name from Paysera\\Bundle\\ApiBundle\\Attribute instead.', + $class->getName(), + $method->getName(), + implode(', @', $names) + )); + } + private function loadAttributes(Route $route, ReflectionClass $class, ReflectionMethod $method): void { $attributes = array_merge($class->getAttributes(), $method->getAttributes());