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

feat(symfony): Deprecate the $exceptionOnNoToken parameter in ResourceAccessChecker::__construct() #4900

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

chalasr
Copy link
Contributor

@chalasr chalasr commented Aug 21, 2022

Q A
Branch? 2.7
Tickets -
License MIT
Doc PR -

Just syncing with symfony/security: symfony/symfony#41965.
This argument is useless with the not-so-new authenticator system, and it must be false everywhere in Symfony 6.
The plan is to do the same in 3.0: deprecate the parameter entirely and throw if it's passed as true, then remove it in 4.0.
The impact of this deprecation is likely to be small since very few or nobody should be extending this class anyway.

@chalasr chalasr force-pushed the deprec-erronnotoken branch from 1d3560a to 42f794c Compare August 21, 2022 17:37
@vincentchalamon
Copy link
Contributor

Why not removing this parameter and force its value to false in 3.0, as you mentionned in the trigger?:

It will be the default and only supported value in 3.0

@chalasr
Copy link
Contributor Author

chalasr commented Aug 22, 2022

Because the current default value for this constructor param is true, changing it to false in 2.7 would be a BC break.
As such, deprecating the parameter itself in 2.7 would not let a chance to the developer to opt-in and adopt the new behavior (false).

So in order to be clean from a BC pov, we need to make it go away in 3 steps as for Symfony:
1- deprecate relying on the current default value to encourage adopting the new behavior (2.7)
2- change the default value + deprecate the parameter entirely (3.0)
3- remove it (4.0)

I'm going to submit the PR for step 2 on 3.0.

@soyuka
Copy link
Member

soyuka commented Aug 29, 2022

2- change the default value + deprecate the parameter entirely (3.0)

We can remove this in 3.0 as it'll support symfony ^6.1 and deprecate the parameter in 2.7.

@chalasr chalasr force-pushed the deprec-erronnotoken branch from 42f794c to 394fc82 Compare August 30, 2022 12:59
@chalasr chalasr force-pushed the deprec-erronnotoken branch 2 times, most recently from 074c626 to 74eac65 Compare August 30, 2022 13:08
@chalasr chalasr changed the title feat(symfony): Deprecate not setting $exceptionOnNoToken to false in ResourceAccessChecker constructor feat(symfony): Deprecate the $exceptionOnNoToken parameter in ResourceAccessChecker::__construct() Aug 30, 2022
@chalasr
Copy link
Contributor Author

chalasr commented Aug 30, 2022

Fair enough, PR updated. See #4905 for the removal

@chalasr chalasr force-pushed the deprec-erronnotoken branch from 74eac65 to f381bad Compare September 1, 2022 09:17
@soyuka soyuka merged commit f81e3e8 into api-platform:2.7 Sep 2, 2022
@soyuka
Copy link
Member

soyuka commented Sep 2, 2022

thanks @chalasr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants