Symfony 6 compatibility

[Th3Mouk]

Hello !

I took a little time to check what https://github.com/symfony/symfony/issues/43021 involve for this bundle. And the only blocker is https://github.com/FriendsOfSymfony/FOSRestBundle/blob/3.x/Controller/AbstractFOSRestController.php#L39 where we must add an array return type. As this is a BC and respecting semver, this modification should occurs in a 4.0 release. But I want first of all discuss with you about all the possibilities.

There's is another thing, the API changes on authenticators passports, and the PassportInterface deletion will make the tests failed by the end of november, because symfony/security-http is upstream to symfony/security-bundle.

Finally, tests are failing with symfony/routing v6, as this package is an upstream dependency of symfony/framework-bundle, it will be automatically installed with the release of 5.4 in november too. It will be more careful to block this version installation until the resolution of https://github.com/FriendsOfSymfony/FOSRestBundle/issues/2301, and then check if tests are passing.

• [ ] Make AbstractFOSRestController compatible with new API
• [ ] Make tests pass with symfony/security-http v6
• [ ] Make tests pass with symfony/routing v6

[mbabker]

Well, it's a little worse than just the typehint in the AbstractController.

Declaration of FOS\RestBundle\Serializer\Normalizer\FlattenExceptionNormalizer::normalize($exception, $format = null, array $context = []) must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize(mixed $object, ?string $format = null, array $context = []): ArrayObject|array|string|int|float|bool|null

The typehints in Symfony\Component\Serializer\Normalizer\NormalizerInterface alone are a major blocker because they require PHP 8 (union return type and mixed param type). That might be something for https://github.com/symfony/symfony/issues/43021 because the native typehints might be too restrictive for sane cross-version support (not just because they're typed, but because they require PHP 8 features).

After fixing the normalizer and test kernel typehints (patch below), I'm left with these PHPUnit issues:

Typehints Patch

```diff diff --git a/Serializer/Normalizer/FlattenExceptionNormalizer.php b/Serializer/Normalizer/FlattenExceptionNormalizer.php index 8e40bab6..858cf0f2 100644 --- a/Serializer/Normalizer/FlattenExceptionNormalizer.php +++ b/Serializer/Normalizer/FlattenExceptionNormalizer.php @@ -36,7 +36,7 @@ final class FlattenExceptionNormalizer implements NormalizerInterface $this->rfc7807 = $rfc7807; } - public function normalize($exception, $format = null, array $context = []) + public function normalize(mixed $exception, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { if (isset($context['status_code'])) { $statusCode = $context['status_code']; @@ -73,7 +73,7 @@ final class FlattenExceptionNormalizer implements NormalizerInterface } } - public function supportsNormalization($data, $format = null) + public function supportsNormalization(mixed $data, string $format = null): bool { return $data instanceof FlattenException; } diff --git a/Tests/Functional/Bundle/TestBundle/Security/ApiTokenAuthenticator.php b/Tests/Functional/Bundle/TestBundle/Security/ApiTokenAuthenticator.php index c5ccae15..a931f907 100644 --- a/Tests/Functional/Bundle/TestBundle/Security/ApiTokenAuthenticator.php +++ b/Tests/Functional/Bundle/TestBundle/Security/ApiTokenAuthenticator.php @@ -20,7 +20,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Http\Authenticator\AbstractAuthenticator; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; -use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; class ApiTokenAuthenticator extends AbstractAuthenticator @@ -28,7 +28,10 @@ class ApiTokenAuthenticator extends AbstractAuthenticator protected $headerName = 'x-foo'; protected $tokenValue = 'FOOBAR'; - public function authenticate(Request $request): PassportInterface + /** + * @return Passport + */ + public function authenticate(Request $request) { $credentials = $request->headers->get($this->headerName); diff --git a/Tests/Functional/DependencyInjectionTest.php b/Tests/Functional/DependencyInjectionTest.php index f84bbb09..74b438b4 100644 --- a/Tests/Functional/DependencyInjectionTest.php +++ b/Tests/Functional/DependencyInjectionTest.php @@ -47,7 +47,7 @@ class DependencyInjectionTest extends KernelTestCase class TestKernel extends Kernel { - public function registerBundles() + public function registerBundles(): array { return [ new FrameworkBundle(), @@ -75,7 +75,7 @@ class TestKernel extends Kernel }); } - public function getCacheDir() + public function getCacheDir(): string { return sys_get_temp_dir().'/'.str_replace('\\', '-', get_class($this)).'/cache/'.$this->environment; } diff --git a/Tests/Functional/app/AppKernel.php b/Tests/Functional/app/AppKernel.php index a9c7f7f0..8aa6b4c1 100644 --- a/Tests/Functional/app/AppKernel.php +++ b/Tests/Functional/app/AppKernel.php @@ -70,7 +70,7 @@ class AppKernel extends Kernel parent::__construct($environment, $debug); } - public function registerBundles() + public function registerBundles(): array { if (!file_exists($filename = $this->getProjectDir().'/'.$this->testCase.'/bundles.php')) { throw new \RuntimeException(sprintf('The bundles file "%s" does not exist.', $filename)); @@ -79,17 +79,17 @@ class AppKernel extends Kernel return include $filename; } - public function getProjectDir() + public function getProjectDir(): string { return __DIR__; } - public function getCacheDir() + public function getCacheDir(): string { return sys_get_temp_dir().'/'.Kernel::VERSION.'/'.$this->testCase.'/cache/'.$this->environment; } - public function getLogDir() + public function getLogDir(): string { return sys_get_temp_dir().'/'.Kernel::VERSION.'/'.$this->testCase.'/logs'; } @@ -114,7 +114,7 @@ class AppKernel extends Kernel $this->__construct($a[0], $a[1], $a[2], $a[3]); } - protected function getKernelParameters() + protected function getKernelParameters(): array { $parameters = parent::getKernelParameters(); $parameters['kernel.test_case'] = $this->testCase; ```

There were 2 errors:

1) FOS\RestBundle\Tests\Request\ParamFetcherTest::testNoValidationErrors
PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method validate may not return value of type array, its declared return type is "Symfony\Component\Validator\ConstraintViolationListInterface"

Tests/Request/ParamFetcherTest.php:157

2) FOS\RestBundle\Tests\Request\RequestBodyParamConverterTest::testValidatorParameters
PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method validate may not return value of type string, its declared return type is "Symfony\Component\Validator\ConstraintViolationListInterface"

Tests/Request/RequestBodyParamConverterTest.php:154

--

There were 7 failures:

1) FOS\RestBundle\Tests\Controller\Annotations\RouteTest::testCanInstantiate
Failed asserting that null matches expected '/path'.

Tests/Controller/Annotations/RouteTest.php:44

2) FOS\RestBundle\Tests\Functional\BasicAuthTest::testWrongCredentialsGives401
Failed asserting that 500 matches expected 401.

Tests/Functional/AbstractAuthenticatorTestCase.php:53

3) FOS\RestBundle\Tests\Functional\BasicAuthTest::testSuccessfulLogin
Failed asserting that 500 matches expected 200.

Tests/Functional/AbstractAuthenticatorTestCase.php:63

4) FOS\RestBundle\Tests\Functional\BasicAuthTest::testAccessDeniedExceptionGives403
Failed asserting that 500 matches expected 403.

Tests/Functional/AbstractAuthenticatorTestCase.php:73

5) FOS\RestBundle\Tests\Functional\RouteAttributesTest::testGet
Failed asserting that 404 is identical to 200.

Tests/Functional/RouteAttributesTest.php:44

6) FOS\RestBundle\Tests\Functional\RouteAttributesTest::testPost
Failed asserting that 404 is identical to 201.

Tests/Functional/RouteAttributesTest.php:61

7) FOS\RestBundle\Tests\Functional\ViewResponseListenerTest::testRedirect
Failed asserting that 404 is identical to 201.

Tests/Functional/ViewResponseListenerTest.php:34

ERRORS!
Tests: 332, Assertions: 629, Errors: 2, Failures: 7, Skipped: 1.

The two errors are just bad mocks (the mocked returns weren't compliant with the interface to begin with and now that the interface is typed it's causing errors).

The failure from FOS\RestBundle\Tests\Controller\Annotations\RouteTest::testCanInstantiate looks to be an issue with the compat layer introduced in #2312, only the $route->getPath() call looks to be failing as when I var_dump($route) the rest of the Route object looks correct (ping @W0rma since they've been working on attributes support and added that compat layer). At a glance, I think the failures in FOS\RestBundle\Tests\Functional\RouteAttributesTest might be related.

For the 3 500 response errors, the password hasher config for the functional tests needs to be updated to account for the removed User class, which this patch should cover:

diff --git a/Tests/Functional/app/BasicAuth/security.php b/Tests/Functional/app/BasicAuth/security.php
index 6729697c..bde9ca25 100644
--- a/Tests/Functional/app/BasicAuth/security.php
+++ b/Tests/Functional/app/BasicAuth/security.php
@@ -32,7 +32,11 @@ $securityConfig = [
     ],
 ];

-$passwordHasherConfig = ['Symfony\Component\Security\Core\User\User' => 'plaintext'];
+if (class_exists(\Symfony\Component\Security\Core\User\InMemoryUser::class)) {
+    $passwordHasherConfig = ['Symfony\Component\Security\Core\User\InMemoryUser' => 'plaintext'];
+} else {
+    $passwordHasherConfig = ['Symfony\Component\Security\Core\User\User' => 'plaintext'];
+}

 // BC layer to avoid deprecation warnings in symfony/security-bundle < 5.3
 if (class_exists(\Symfony\Bundle\SecurityBundle\RememberMe\FirewallAwareRememberMeHandler::class)) {
diff --git a/Tests/Functional/app/CustomGuardAuthenticator/security.php b/Tests/Functional/app/CustomGuardAuthenticator/security.php
index 781caa4f..1199cb2d 100644
--- a/Tests/Functional/app/CustomGuardAuthenticator/security.php
+++ b/Tests/Functional/app/CustomGuardAuthenticator/security.php
@@ -21,7 +21,11 @@ $securityConfig = [
     ],
 ];

-$passwordHasherConfig = ['Symfony\Component\Security\Core\User\User' => 'plaintext'];
+if (class_exists(\Symfony\Component\Security\Core\User\InMemoryUser::class)) {
+    $passwordHasherConfig = ['Symfony\Component\Security\Core\User\InMemoryUser' => 'plaintext'];
+} else {
+    $passwordHasherConfig = ['Symfony\Component\Security\Core\User\User' => 'plaintext'];
+}

 // BC layer to avoid deprecation warnings in symfony/security-bundle < 5.3
 if (class_exists(\Symfony\Bundle\SecurityBundle\RememberMe\FirewallAwareRememberMeHandler::class)) {

There are still a handful of packages that can't be installed at 6.0 even after updating composer.json:

$ composer show symfony/*
symfony/console              5.4.x-dev 6b71e16
symfony/dependency-injection 5.4.x-dev f84ead2
symfony/event-dispatcher     5.4.x-dev ac2f062
symfony/filesystem           5.4.x-dev d685d0c
symfony/finder               5.4.x-dev 3d70c86
symfony/framework-bundle     5.4.x-dev c8f18ee
symfony/options-resolver     5.4.x-dev cd63dba
symfony/process              5.4.x-dev b076aa9
symfony/stopwatch            5.4.x-dev 68e61ec

Of those, there are really only two that are of concern to this bundle: the DependencyInjection component and the FrameworkBundle (this bundle has a few event subscribers but there aren't any real compat concerns with EventSubscriberInterface, and the rest are just minor utils or not even used here). For both of those, https://github.com/schmittjoh/JMSSerializerBundle/pull/869 unblocks that upgrade.