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

throttle plugin requires a name #464

Merged
merged 2 commits into from
Sep 1, 2024
Merged

throttle plugin requires a name #464

merged 2 commits into from
Sep 1, 2024

Conversation

dbu
Copy link
Collaborator

@dbu dbu commented Sep 1, 2024

Q A
Bug fix? yes
New feature? no
BC breaks? (no) if the name option for the throttle plugin was not specified, 1.34.0 did not report invalid configuration, but the extension class was broken and if php ignored the undefined array key, the plugin can not be instantiated because the service does not exist.
Deprecations? no
Related tickets -
Documentation -
License MIT

What's in this PR?

Throttle plugin references a symfony/rate-limiter service with the name. This is not optional.

@dbu dbu force-pushed the fix-throttle branch 2 times, most recently from 88a76ed to ea4606a Compare September 1, 2024 07:31
$container
->register($serviceId.$key, LimiterInterface::class)
->register($serviceId.$config['name'], LimiterInterface::class)
->setFactory([new Reference('limiter.'.$config['name']), 'create'])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is asserting that the name must exist.

and if we would use $key, we would set a reference to limiter. which would not be found.

@dbu dbu merged commit 3fdfecc into 1.x Sep 1, 2024
14 checks passed
@dbu dbu deleted the fix-throttle branch September 1, 2024 07:36
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.

1 participant