Skip to content

Support multiple tags on middleware #14

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

Open
wizbit wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support multiple tags on middleware #14

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
983207f
Support multiple tags on middleware
Oct 30, 2017
79ab656
Fixed failing tests by moving new test config to separate file.
Oct 30, 2017
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
13 changes: 11 additions & 2 deletions Tests/DependencyInjection/Compiler/Builder/DefinitionBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public function testGetHandlerDefinition()
$container = $this->prophesize(ContainerBuilder::class);

$middleware = ['test_middleware' => [
['method' => 'attach']
['method' => 'attach'],
['method' => 'attach2'],
]];

$handler = $this->subject->getHandlerDefinition($container->reveal(), $clientName, $middleware);
Expand All @@ -51,7 +52,7 @@ public function testGetHandlerDefinition()

$methodCalls = $handler->getMethodCalls();
// Count is the number of the given middleware + the default event and log middleware expressions
$this->assertCount(3, $methodCalls);
$this->assertCount(4, $methodCalls);

$customMiddlewareCall = array_shift($methodCalls);
$this->assertSame('push', array_shift($customMiddlewareCall));
Expand All @@ -61,6 +62,14 @@ public function testGetHandlerDefinition()
$this->assertInstanceOf(Expression::class, $customMiddlewareExpression);
$this->assertSame('service("test_middleware").attach()', $customMiddlewareExpression->__toString());

$customMiddlewareCall = array_shift($methodCalls);
Copy link
Copy Markdown
Contributor

@cjost1988 cjost1988 Nov 2, 2017

Choose a reason for hiding this comment

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

I know it was done for all previous middleware tests as well, but I think it's worth to get rid of the array_shift hell.

I though about sth like:

// define expected middleware calls
$expectedMiddlewareCalls = [
    'service("test_middleware").attach()' => 'push',
    'service("test_middleware").attach2()' => 'push',
    'service("guzzle_bundle.middleware.log.clientName").log()' => 'push',
    'service("guzzle_bundle.middleware.event_dispatch.clientName").dispatch()' => 'unshift',
];

$doneMiddlewareCalls = [];
foreach ($methodCalls as $methodCall) {
    /** @var Expression[] $customMiddlewareExpressions */
    $customMiddlewareExpressions = $methodCall[1];
    $customMiddlewareExpression = $customMiddlewareExpressions[0];
    $this->assertInstanceOf(Expression::class, $customMiddlewareExpression);

    $doneMiddlewareCall = $customMiddlewareExpression->__toString();

    $this->assertSame($expectedMiddlewareCalls[$doneMiddlewareCall], $methodCall[0]);

    $doneMiddlewareCalls[] = $doneMiddlewareCall;
}

$this->assertCount(0, array_diff(array_keys($expectedMiddlewareCalls), $doneMiddlewareCalls));

What do you think @wizbit ?
It would makes it easy to add more system defined Middlewares later on and the code is more readable since it's shorter.

$this->assertSame('push', array_shift($customMiddlewareCall));
/** @var Expression[] $customMiddlewareExpressions */
$customMiddlewareExpressions = array_shift($customMiddlewareCall);
$customMiddlewareExpression = array_shift($customMiddlewareExpressions);
$this->assertInstanceOf(Expression::class, $customMiddlewareExpression);
$this->assertSame('service("test_middleware").attach2()', $customMiddlewareExpression->__toString());

$logMiddlewareCall = array_shift($methodCalls);
$this->assertSame('push', array_shift($logMiddlewareCall));
/** @var Expression[] $logMiddlewareExpressions */
Expand Down
39 changes: 35 additions & 4 deletions Tests/DependencyInjection/Compiler/Filter/MiddlewareFilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

namespace Mapudo\Bundle\GuzzleBundle\Tests\DependencyInjection\Compiler\Filter;

use Mapudo\Bundle\GuzzleBundle\DependencyInjection\Compiler\Filter\MiddlewareFilter;
Expand Down Expand Up @@ -31,9 +32,9 @@ protected function setUp()
/**
* @dataProvider dataProviderFilter
*
* @param array $middleware
* @param array $middleware
* @param string $clientName
* @param array $expectedResult
* @param array $expectedResult
*/
public function testFilter(array $middleware, string $clientName, array $expectedResult)
{
Expand All @@ -54,7 +55,7 @@ public function testFilter(array $middleware, string $clientName, array $expecte
public function dataProviderFilter(): array
{
$parsedMiddleware = Yaml::parse(
file_get_contents(__DIR__ . '/../../../Resources/config/sample_middleware.yml')
file_get_contents(__DIR__ . '/../../../Resources/config/sample_multiple_middleware.yml')
)['services'];

$middleware = [];
Expand All @@ -63,7 +64,37 @@ public function dataProviderFilter(): array
}

return [
[$middleware, 'test_client', $middleware],
[
$middleware,
'test_client',
[
'guzzle.middleware.stuff' => [
[
'name' => 'guzzle.middleware',
'method' => 'addMiddlewareStuff',
'client' => 'test_client'
],
[
'name' => 'guzzle.middleware',
'method' => 'addOtherMiddlewareStuff',
'client' => 'test_client'
]
]
]
],
[
$middleware,
'test_client2',
[
'guzzle.middleware.stuff' => [
[
'name' => 'guzzle.middleware',
'method' => 'addMiddlewareStuff',
'client' => 'test_client2'
]
]
]
],
[$middleware, 'false_client', []],
];
}
Expand Down
1 change: 0 additions & 1 deletion Tests/Resources/config/sample_middleware.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ services:
guzzle.middleware.stuff:
tags:
- { name: guzzle.middleware, method: addMiddlewareStuff, client: test_client }

8 changes: 8 additions & 0 deletions Tests/Resources/config/sample_multiple_middleware.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This file shows a sample middleware configuration for the guzzle library
services:
guzzle.middleware.stuff:
tags:
- { name: guzzle.middleware, method: addMiddlewareStuff, client: test_client }
- { name: guzzle.middleware, method: addOtherMiddlewareStuff, client: test_client }
- { name: guzzle.middleware, method: addMiddlewareStuff, client: test_client2 }

14 changes: 7 additions & 7 deletions src/DependencyInjection/Compiler/Builder/DefinitionBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ public function getHandlerDefinition(
$handler->setFactory(['%guzzle_http.handler_stack.class%', 'create']);

foreach ($middleware as $id => $tags) {
$attributes = reset($tags);

if (!empty($attributes['method'])) {
$middlewareExpression = new Expression(sprintf('service("%s").%s()', $id, $attributes['method']));
} else {
$middlewareExpression = new Expression(sprintf('service("%s")', $id));
foreach ($tags as $attributes) {
if (!empty($attributes['method'])) {
$middlewareExpression = new Expression(sprintf('service("%s").%s()', $id, $attributes['method']));
} else {
$middlewareExpression = new Expression(sprintf('service("%s")', $id));
}
$handler->addMethodCall('push', [$middlewareExpression]);
}
$handler->addMethodCall('push', [$middlewareExpression]);
}

$eventServiceName = sprintf('guzzle_bundle.middleware.event_dispatch.%s', $clientName);
Expand Down
16 changes: 11 additions & 5 deletions src/DependencyInjection/Compiler/Filter/MiddlewareFilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,16 @@ class MiddlewareFilter
*/
public function filter(ContainerBuilder $container, string $clientName): array
{
$middleware = $container->findTaggedServiceIds('guzzle.middleware');
return array_filter($middleware, function ($middleware) use ($clientName) {
$options = reset($middleware);
return !array_key_exists('client', $options) || $clientName === $options['client'];
});
$middleware = [];

foreach ($container->findTaggedServiceIds('guzzle.middleware') as $service => $tags) {
foreach ($tags as $options) {
if (!array_key_exists('client', $options) || $clientName === $options['client']) {
$middleware[$service][] = $options;
}
}
}

return $middleware;
}
}