Skip to content

throttle plugin requires a name #464

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 1, 2024
Merged

throttle plugin requires a name #464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c3e184
throttle plugin requires a name
dbu Sep 1, 2024
64362f1
fix build
dbu Sep 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ The change log describes what is "Added", "Removed", "Changed" or "Fixed" betwee

# Version 1

# 1.34.1 - 2024-09-01

- The rate-limiter name in the throttle plugin configuration is required.

# 1.34.0 - 2024-06-17

- Support to configure the throttle plugin.
Expand Down
9 changes: 5 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,19 @@
},
"require-dev": {
"guzzlehttp/psr7": "^1.7 || ^2.0",
"matthiasnoback/symfony-dependency-injection-test": "^4.0 || ^5.0",
"matthiasnoback/symfony-config-test": "^4.3 || ^5.0",
"matthiasnoback/symfony-dependency-injection-test": "^4.3.1 || ^5.0",
"nyholm/nsa": "^1.1",
"nyholm/psr7": "^1.2.1",
"php-http/cache-plugin": "^1.7",
"php-http/mock-client": "^1.2",
"php-http/promise": "^1.0",
"phpunit/phpunit": "^9.6",
"symfony/browser-kit": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/cache": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/dom-crawler": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/framework-bundle": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/http-foundation": "^4.4.19 || ^5.0 || ^6.0 || ^7.0",
"symfony/phpunit-bridge": "^6.4.1",
"symfony/stopwatch": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/twig-bundle": "^4.4 || ^5.0 || ^6.0 || ^7.0",
"symfony/web-profiler-bundle": "^4.4.19 || ^5.0 || ^6.0 || ^7.0",
Expand Down Expand Up @@ -97,7 +98,7 @@
},
"prefer-stable": false,
"scripts": {
"test": "vendor/bin/simple-phpunit",
"test-ci": "vendor/bin/simple-phpunit --coverage-text --coverage-clover=build/coverage.xml"
"test": "vendor/bin/phpunit",
"test-ci": "vendor/bin/phpunit --coverage-text --coverage-clover=build/coverage.xml"
}
}
9 changes: 6 additions & 3 deletions src/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableA
->end();
// End stopwatch plugin

$error = $children->arrayNode('error')
$children->arrayNode('error')
->canBeEnabled()
->addDefaultsIfNotSet()
->children()
Expand All @@ -625,11 +625,14 @@ private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableA
->end();
// End error plugin

$throttle = $children->arrayNode('throttle')
$children->arrayNode('throttle')
->canBeEnabled()
->addDefaultsIfNotSet()
->children()
->scalarNode('name')->end()
->scalarNode('name')
->info('The name of the configured symfony/rate-limiter to use')
->isRequired()
->end()
->scalarNode('key')->defaultNull()->end()
->integerNode('tokens')->defaultValue(1)->end()
->floatNode('max_time')->defaultNull()->end()
Expand Down
5 changes: 2 additions & 3 deletions src/DependencyInjection/HttplugExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,14 +299,13 @@ private function configurePluginByName($name, Definition $definition, array $con
throw new InvalidConfigurationException('You need to require the Throttle Plugin to be able to use it: "composer require php-http/throttle-plugin".');
}

$key = $config['name'] ? '.'.$config['name'] : '';
$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.

->addArgument($config['key'])
->setPublic(false);

$definition->replaceArgument(0, new Reference($serviceId.$key));
$definition->replaceArgument(0, new Reference($serviceId.$config['name']));
$definition->setArgument('$tokens', $config['tokens']);
$definition->setArgument('$maxTime', $config['max_time']);

Expand Down