Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Symfony 5.3 #1217

Merged
merged 7 commits into from Jun 2, 2021
Merged

Upgrade to Symfony 5.3 #1217

merged 7 commits into from Jun 2, 2021

Conversation

javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented May 20, 2021

For now, it's Symfony 5.3 RC1, but we'll keep updating this branch until 5.3 stable.


Things that I found while upgrading:

Missing UPGRADE guide entry?

In the UPGRADE guide (https://github.com/symfony/symfony/blob/5.3/UPGRADE-5.3.md) we don't even mention enable_csrf, but this change is needed in config/packages/security.yaml:

Replace this:

form_login:
    csrf_token_generator: security.csrf.token_manager

by this:

form_login:
    enable_csrf: true

Unexpected failures

The upgrade guide mentions this:

PropertyInfo:

Deprecated the Type::getCollectionKeyType() and Type::getCollectionValueType() methods,
use Type::getCollectionKeyTypes() and Type::getCollectionValueTypes() instead

But we have these test failures:


App\Tests\Controller\Admin\BlogControllerTest::testAdminNewPost
Error: Call to undefined method Symfony\Component\PropertyInfo\Type::getCollectionValueTypes()

demo/vendor/symfony/validator/Mapping/Loader/PropertyInfoLoader.php:122
demo/vendor/symfony/validator/Mapping/Loader/LoaderChain.php:54
demo/vendor/symfony/validator/Mapping/Factory/LazyLoadingMetadataFactory.php:101
demo/vendor/symfony/validator/Validator/RecursiveValidator.php:76
demo/vendor/symfony/validator/Validator/TraceableValidator.php:50
demo/vendor/symfony/form/Extension/Validator/ValidatorTypeGuesser.php:265
demo/vendor/symfony/form/Extension/Validator/ValidatorTypeGuesser.php:38
demo/vendor/symfony/form/FormTypeGuesserChain.php:47
demo/vendor/symfony/form/FormTypeGuesserChain.php:93
demo/vendor/symfony/form/FormTypeGuesserChain.php:48
demo/vendor/symfony/form/FormFactory.php:84
demo/vendor/symfony/form/FormBuilder.php:97
demo/vendor/symfony/form/FormBuilder.php:244
demo/vendor/symfony/form/FormBuilder.php:195
demo/vendor/symfony/form/FormFactory.php:28
demo/vendor/symfony/framework-bundle/Controller/AbstractController.php:359
demo/src/Controller/Admin/BlogController.php:78
demo/vendor/symfony/http-kernel/HttpKernel.php:157
demo/vendor/symfony/http-kernel/HttpKernel.php:79
demo/vendor/symfony/http-kernel/Kernel.php:196
demo/vendor/symfony/http-kernel/HttpKernelBrowser.php:63
demo/vendor/symfony/framework-bundle/KernelBrowser.php:159
demo/vendor/symfony/browser-kit/AbstractBrowser.php:402
demo/tests/Controller/Admin/BlogControllerTest.php:89

Unexpected direct deprecations

I see the following "direct deprecation" notices, but I don't know how they can be "direct deprecations". We don't use any of these in our code:

Remaining direct deprecation notices (7)

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\SodiumPasswordHasher" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface" class is deprecated, use "Symfony\Component\PasswordHasher\PasswordHasherInterface" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\SelfSaltingEncoderInterface" interface is deprecated, use "Symfony\Component\PasswordHasher\LegacyPasswordHasherInterface" on hasher implementations that deal with salts instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\BasePasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\CheckPasswordLengthTrait" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\MessageDigestPasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\MessageDigestPasswordHasher" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\MigratingPasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\MigratingPasswordHasher" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

Indirect deprecations to fix?

There are still these "indirect deprecations". Are we missing some dependency updates ... or are these really deprecations pending to be fixed?

Remaining indirect deprecation notices (24)

  19x: Since symfony/security-core 5.3: Not implementing method "loadUserByIdentifier()" in user provider "Symfony\Bridge\Doctrine\Security\User\EntityUserProvider" is deprecated. This method will replace "loadUserByUsername()" in Symfony 6.0.
    4x in BlogControllerTest::testAccessDeniedForRegularUsers from App\Tests\Controller\Admin
    4x in BlogControllerTest::testNewComment from App\Tests\Controller
    2x in BlogControllerTest::testAdminDeletePost from App\Tests\Controller\Admin
    2x in UserControllerTest::testEditUser from App\Tests\Controller
    2x in UserControllerTest::testChangePassword from App\Tests\Controller
    1x in BlogControllerTest::testAdminBackendHomePage from App\Tests\Controller\Admin
    1x in BlogControllerTest::testAdminNewPost from App\Tests\Controller\Admin
    1x in BlogControllerTest::testAdminNewDuplicatedPost from App\Tests\Controller\Admin
    1x in BlogControllerTest::testAdminShowPost from App\Tests\Controller\Admin
    1x in BlogControllerTest::testAdminEditPost from App\Tests\Controller\Admin

  4x: Since symfony/http-foundation 5.3: "Symfony\Component\HttpFoundation\RequestStack::getMasterRequest()" is deprecated, use "getMainRequest()" instead.
    4x in BlogControllerTest::testAccessDeniedForRegularUsers from App\Tests\Controller\Admin

  1x: The "DAMA\DoctrineTestBundle\Doctrine\DBAL\AbstractStaticDriverV2" class implements "Doctrine\DBAL\Driver\ExceptionConverterDriver" that is deprecated.
    1x in PHPUnitExtension::executeBeforeFirstTest from DAMA\DoctrineTestBundle\PHPUnit



Other deprecation notices (18)

  7x: Since symfony/security-bundle 5.3: The "security.password_encoder" service is deprecated, use "security.user_password_hasher" instead.
    2x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command
    2x in AddUserCommandTest::testCreateUserInteractive from App\Tests\Command
    2x in UserControllerTest::testChangePassword from App\Tests\Controller
    1x in UserControllerTest::testAccessDeniedForAnonymousUsers from App\Tests\Controller

  7x: Since symfony/security-bundle 5.3: The "security.encoder_factory.generic" service is deprecated, use "security.password_hasher_factory" instead.
    2x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command
    2x in AddUserCommandTest::testCreateUserInteractive from App\Tests\Command
    2x in UserControllerTest::testChangePassword from App\Tests\Controller
    1x in UserControllerTest::testAccessDeniedForAnonymousUsers from App\Tests\Controller

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\UserPasswordEncoder" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface" interface is deprecated, use "Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\EncoderFactory" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactory" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

  1x: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface" class is deprecated, use "Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

composer.json Outdated Show resolved Hide resolved
@nicolas-grekas
Copy link
Member

I just force-pushed on your fork @javiereguiluz. This is now rebased and has more fixes for 5.3.

I didn't touch the assets, but it would be good to align them with Symfony UX also.

composer.json Show resolved Hide resolved
config/packages/security.yaml Outdated Show resolved Hide resolved
@stof
Copy link
Member

stof commented Jun 1, 2021

The ErrorException in the CI output might be a bug in symfony/validator rather than in the demo

@nicolas-grekas
Copy link
Member

I added a commit to update assets.
Note that yarn displayed this, dunno if that's an issue or not:

warning " > eonasdan-bootstrap-datetimepicker@4.17.49" has unmet peer dependency "bootstrap@^3.3".
warning " > eonasdan-bootstrap-datetimepicker@4.17.49" has unmet peer dependency "moment@^2.10".
warning " > eonasdan-bootstrap-datetimepicker@4.17.49" has unmet peer dependency "moment-timezone@^0.4.0 || ^0.5.0".
warning " > sass-loader@9.0.3" has unmet peer dependency "webpack@^4.36.0 || ^5.0.0".

@tgalopin
Copy link
Member

tgalopin commented Jun 2, 2021

Yes, unmet peer dependencies are usually fine (either the package is present but in a different version that still works and that's okay, or it's not present but it's optional for the usage of the application). That's how peerDeps are mostly used in JS (not a fan either).

assets/app.js Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@
"license": "MIT",
"type": "project",
"description": "Symfony Demo Application",
"minimum-stability": "stable",
"minimum-stability": "dev",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be stable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that we also have "prefer-stable": true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the 5.4 skeleton is meaningful about using dev stability as 5.4 is still in dev. Here, we talk about updating to 5.3 which is stable.

And prefer-stable might still select dev versions of other packages depending on the order of package selection (see composer/composer#9917)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep sorry bad link, we use dev since 5.2:
https://github.com/symfony/skeleton/blob/5.2/composer.json#L6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but is it an intended choice, or just forgetting to update the skeleton once the stable release happen ? I never remember discussing using that behavior for stable skeletons (I might have missed the discussion of course)

@nicolas-grekas
Copy link
Member

PR is green, ready for merge!

@javiereguiluz
Copy link
Member Author

Fantastic work 👏 Thanks Nicolas and all reviewers! 🚀

@javiereguiluz javiereguiluz merged commit 6b03342 into symfony:main Jun 2, 2021
@javiereguiluz javiereguiluz deleted the sf53 branch June 2, 2021 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants