Skip to content

[FrameworkBundle] Add integration of http-client component #30419

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

Merged
merged 2 commits into from
Mar 17, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
[FrameworkBundle] Add integration of http-client component
  • Loading branch information
Thomas Talbot authored and nicolas-grekas committed Mar 17, 2019
commit 0023a712604decb5ce3de9fe17128c8e2b6f55cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Bundle\FullStack;
use Symfony\Component\Asset\Package;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\NodeBuilder;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\DependencyInjection\Exception\LogicException;
Expand Down Expand Up @@ -109,6 +110,7 @@ public function getConfigTreeBuilder()
$this->addLockSection($rootNode);
$this->addMessengerSection($rootNode);
$this->addRobotsIndexSection($rootNode);
$this->addHttpClientSection($rootNode);

return $treeBuilder;
}
Expand Down Expand Up @@ -1170,4 +1172,124 @@ private function addRobotsIndexSection(ArrayNodeDefinition $rootNode)
->end()
;
}

private function addHttpClientSection(ArrayNodeDefinition $rootNode)
{
$subNode = $rootNode
->children()
->arrayNode('http_client')
->info('HTTP Client configuration')
->canBeEnabled()
->fixXmlConfig('client')
->children();

$this->addHttpClientOptionsSection($subNode);

$subNode = $subNode
->arrayNode('clients')
->useAttributeAsKey('name')
->normalizeKeys(false)
->arrayPrototype()
->children();

$this->addHttpClientOptionsSection($subNode);

$subNode = $subNode
->end()
->end()
->end()
->end()
->end()
->end()
;
}

private function addHttpClientOptionsSection(NodeBuilder $rootNode)
{
$rootNode
->integerNode('max_host_connections')
->info('The maximum number of connections to a single host.')
->end()
->arrayNode('default_options')
Copy link
Member

Choose a reason for hiding this comment

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

These are the default options for each request that's made. What about default_request_options (it's even documented as Default requests' options in HttpClient phpdoc). And also, should we make these options not automatically be used as the "default options" for all the other HTTP clients?

The problem is that there are 2 concepts conflicting right now. Does "default" mean:
A) default options for the request that can be overridden on each request
OR
B) default options that will be applied to all other HTTP clients

Right now, it's both - that confused me. It's really (A), so maybe we should make it only (A) and clarify the name.

Copy link
Member

Choose a reason for hiding this comment

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

right now, it is default options applied to all other HTTP clients for their default request options...

Copy link
Member

Choose a reason for hiding this comment

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

exactly - the "default" means to things - it's (A) and (B).

Copy link
Member

@nicolas-grekas nicolas-grekas Mar 13, 2019

Choose a reason for hiding this comment

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

Correct, it's A and B. If one wants to configure the default request options for the default client only, one should set them under framework.http_client.clients.http_client.default_options (since http_client is the name of the service that is aliased for HttpClientInterface).
I'm not sure I understand how your suggestion would apply. To me, all is fine :)

Copy link
Member

Choose a reason for hiding this comment

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

I was suggesting that we remove (B), but it's subjective. The question is: is it valuable for these options to cascade onto other clients? Or is it annoying/surprising, and then I am unsetting options on my individual clients, because I don't want them to cascade from my default client?

I'm not convinced that re-using options across clients will be all that common - things like auth, base_uri, headers all seem very client-specific. I could argue that it might be surprising if my auth on my default client starts getting sent to another server via another client (because I didn't realize the options would cascade down to my client).

Copy link
Member

Choose a reason for hiding this comment

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

e.g. cabundle/capath: you want to configure them once for all

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, ok, really fine by me then.

->fixXmlConfig('header')
->children()
->scalarNode('auth')
->info('An HTTP Basic authentication "username:password".')
->end()
->arrayNode('query')
->info('Associative array of query string values merged with URL parameters.')
->useAttributeAsKey('key')
->normalizeKeys(false)
->scalarPrototype()->end()
->end()
->arrayNode('headers')
->info('Associative array: header => value(s).')
->useAttributeAsKey('name')
->normalizeKeys(false)
->variablePrototype()->end()
->end()
->integerNode('max_redirects')
->info('The maximum number of redirects to follow.')
->end()
->scalarNode('http_version')
->info('The default HTTP version, typically 1.1 or 2.0. Leave to null for the best version.')
->end()
->scalarNode('base_uri')
->info('The URI to resolve relative URLs, following rules in RFC 3986, section 2.')
->end()
->booleanNode('buffer')
->info('Indicates if the response should be buffered or not.')
->end()
->arrayNode('resolve')
->info('Associative array: domain => IP.')
->useAttributeAsKey('host')
->normalizeKeys(false)
->scalarPrototype()->end()
->end()
->scalarNode('proxy')
->info('The URL of the proxy to pass requests through or null for automatic detection.')
->end()
->scalarNode('no_proxy')
->info('A comma separated list of hosts that do not require a proxy to be reached.')
->end()
->floatNode('timeout')
->info('Defaults to "default_socket_timeout" ini parameter.')
->end()
->scalarNode('bindto')
->info('A network interface name, IP address, a host name or a UNIX socket to bind to.')
->end()
->booleanNode('verify_peer')
->info('Indicates if the peer should be verified in a SSL/TLS context.')
->end()
->booleanNode('verify_host')
->info('Indicates if the host should exist as a certificate common name.')
->end()
->scalarNode('cafile')
->info('A certificate authority file.')
->end()
->scalarNode('capath')
->info('A directory that contains multiple certificate authority files.')
->end()
->scalarNode('local_cert')
->info('A PEM formatted certificate file.')
->end()
->scalarNode('local_pk')
->info('A private key file.')
->end()
->scalarNode('passphrase')
->info('The passphrase used to encrypt the "local_pk" file.')
->end()
->scalarNode('ciphers')
->info('A list of SSL/TLS ciphers separated by colons, commas or spaces (e.g. "RC4-SHA:TLS13-AES-128-GCM-SHA256"...)')
->end()
->arrayNode('peer_fingerprint')
->info('Associative array: hashing algorithm => hash(es).')
->useAttributeAsKey('algo')
->normalizeKeys(false)
->variablePrototype()->end()
->end()
->end()
->end()
;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Doctrine\Common\Annotations\Reader;
use Psr\Cache\CacheItemPoolInterface;
use Psr\Container\ContainerInterface as PsrContainerInterface;
use Psr\Http\Client\ClientInterface;
use Psr\Log\LoggerAwareInterface;
use Symfony\Bridge\Monolog\Processor\DebugProcessor;
use Symfony\Bridge\Twig\Extension\CsrfExtension;
Expand Down Expand Up @@ -57,6 +58,9 @@
use Symfony\Component\Form\FormTypeExtensionInterface;
use Symfony\Component\Form\FormTypeGuesserInterface;
use Symfony\Component\Form\FormTypeInterface;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\HttpClientTrait;
use Symfony\Component\HttpClient\Psr18Client;
use Symfony\Component\HttpKernel\CacheClearer\CacheClearerInterface;
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
Expand Down Expand Up @@ -110,6 +114,8 @@
use Symfony\Component\Yaml\Command\LintCommand as BaseYamlLintCommand;
use Symfony\Component\Yaml\Yaml;
use Symfony\Contracts\Cache\CacheInterface;
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\Service\ResetInterface;
use Symfony\Contracts\Service\ServiceSubscriberInterface;

Expand Down Expand Up @@ -301,6 +307,10 @@ public function load(array $configs, ContainerBuilder $container)
$this->registerLockConfiguration($config['lock'], $container, $loader);
}

if ($this->isConfigEnabled($container, $config['http_client'])) {
$this->registerHttpClientConfiguration($config['http_client'], $container, $loader);
}

if ($this->isConfigEnabled($container, $config['web_link'])) {
if (!class_exists(HttpHeaderSerializer::class)) {
throw new LogicException('WebLink support cannot be enabled as the WebLink component is not installed. Try running "composer require symfony/weblink".');
Expand Down Expand Up @@ -1747,6 +1757,48 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con
}
}

private function registerHttpClientConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader)
{
if (!class_exists(HttpClient::class)) {
throw new LogicException('HttpClient support cannot be enabled as the component is not installed. Try running "composer require symfony/http-client".');
}

$loader->load('http_client.xml');

$merger = new class() {
use HttpClientTrait;

public function merge(array $options, array $defaultOptions)
{
try {
[, $options] = $this->prepareRequest(null, null, $options, $defaultOptions);

return $options;
} catch (TransportExceptionInterface $e) {
throw new InvalidArgumentException($e->getMessage(), 0, $e);
}
}
};

$defaultOptions = $merger->merge($config['default_options'] ?? [], []);
$container->getDefinition('http_client')->setArguments([$defaultOptions, $config['max_host_connections'] ?? 6]);

foreach ($config['clients'] as $name => $clientConfig) {
$options = $merger->merge($clientConfig['default_options'] ?? [], $defaultOptions);

$container->register($name, HttpClientInterface::class)
->setFactory([HttpClient::class, 'create'])
->setArguments([$options, $clientConfig['max_host_connections'] ?? $config['max_host_connections'] ?? 6]);

$container->register('psr18.'.$name, Psr18Client::class)
->setAutowired(true)
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want to use autowiring here? IIRC we don't do that anywhere else with core services.

Copy link
Member

Choose a reason for hiding this comment

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

autowiring allow providing working services once the required aliases are installed
see eg https://github.com/symfony/recipes/blob/master/nyholm/psr7/1.0/config/packages/nyholm_psr7.yaml
This looked the best to achieve loose coupling at the configuration level to me. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe related to this, I'm currently getting a fatal error when running debug:container or debug:config with this component enabled in the config:

PHP Fatal error:  Method class@anonymous::__toString() must not throw an exception, caught Symfony\Component\DependencyInjection\Exception\AutowiringFailedException:  in /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php on line 229
PHP Stack trace:
PHP   1. {main}() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:0
PHP   2. Symfony\Bundle\FrameworkBundle\Console\Application->run() /Users/weaverryan/Sites/os/docs/http_client_play/bin/console:38
PHP   3. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:145
PHP   4. Symfony\Bundle\FrameworkBundle\Console\Application->doRun() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:73
PHP   5. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:269
PHP   6. Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:87
PHP   7. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->run() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Application.php:926
PHP   8. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->execute() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/Console/Command/Command.php:255
PHP   9. Symfony\Bundle\FrameworkBundle\Command\ContainerDebugCommand->getContainerBuilder() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:117
PHP  10. Symfony\Component\DependencyInjection\ContainerBuilder->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php:213
PHP  11. Symfony\Component\DependencyInjection\Compiler\Compiler->compile() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/ContainerBuilder.php:754
PHP  12. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:95
PHP  13. Symfony\Component\DependencyInjection\Compiler\AutowirePass->process() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:52
PHP  14. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:46
PHP  15. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71
PHP  16. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:99
PHP  17. Symfony\Component\DependencyInjection\Compiler\AutowirePass->processValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php:82
PHP  18. Symfony\Component\DependencyInjection\Compiler\AutowirePass->doProcessValue() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:71
PHP  19. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireCalls() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:122
PHP  20. Symfony\Component\DependencyInjection\Compiler\AutowirePass->autowireMethod() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:164
PHP  21. Symfony\Component\DependencyInjection\Compiler\AutowirePass->Symfony\Component\DependencyInjection\Compiler\{closure}() /Users/weaverryan/Sites/os/symfony/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php:255

This is coming from the psr18.http_client service - not sure what the problem is though!

Copy link
Member

Choose a reason for hiding this comment

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

See #30571

->setArguments([new Reference($name)]);

$container->registerAliasForArgument($name, HttpClientInterface::class);
$container->registerAliasForArgument('psr18.'.$name, ClientInterface::class, $name);
}
}

/**
* Returns the base path for the XSD files.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<service id="http_client" class="Symfony\Contracts\HttpClient\HttpClientInterface">
<factory class="Symfony\Component\HttpClient\HttpClient" method="create" />
<argument type="collection" /> <!-- default options -->
<argument /> <!-- max host connections -->
</service>
<service id="Symfony\Contracts\HttpClient\HttpClientInterface" alias="http_client" />

<service id="psr18.http_client" class="Symfony\Component\HttpClient\Psr18Client" autowire="true">
<argument type="service" id="http_client" />
</service>
<service id="Psr\Http\Client\ClientInterface" alias="psr18.http_client" />
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<xsd:element name="php-errors" type="php-errors" minOccurs="0" maxOccurs="1" />
<xsd:element name="lock" type="lock" minOccurs="0" maxOccurs="1" />
<xsd:element name="messenger" type="messenger" minOccurs="0" maxOccurs="1" />
<xsd:element name="http_client" type="http_client" minOccurs="0" maxOccurs="1" />
</xsd:choice>

<xsd:attribute name="http-method-override" type="xsd:boolean" />
Expand Down Expand Up @@ -444,4 +445,64 @@
</xsd:sequence>
<xsd:attribute name="id" type="xsd:string" use="required"/>
</xsd:complexType>

<xsd:complexType name="http_client">
<xsd:sequence>
<xsd:element name="max_host_connections" type="xsd:integer" minOccurs="0" />
<xsd:element name="default_options" type="http_client_options" minOccurs="0" />
<xsd:element name="client" type="http_client_client" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="enabled" type="xsd:boolean" />
</xsd:complexType>

<xsd:complexType name="http_client_options">
<xsd:sequence>
<xsd:element name="auth" type="xsd:string" minOccurs="0" />
<xsd:element name="query" type="http_query" minOccurs="0" />
<xsd:element name="headers" type="http_headers" minOccurs="0" />
<xsd:element name="max_redirects" type="xsd:integer" minOccurs="0" />
<xsd:element name="http_version" type="xsd:string" minOccurs="0" />
<xsd:element name="base_uri" type="xsd:string" minOccurs="0" />
<xsd:element name="buffer" type="xsd:boolean" minOccurs="0" />
<xsd:element name="resolve" type="metadata" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="proxy" type="xsd:string" minOccurs="0" />
<xsd:element name="no_proxy" type="xsd:string" minOccurs="0" />
<xsd:element name="timeout" type="xsd:float" minOccurs="0" />
<xsd:element name="bindto" type="xsd:string" minOccurs="0" />
<xsd:element name="verify_peer" type="xsd:boolean" minOccurs="0" />
<xsd:element name="verify_host" type="xsd:boolean" minOccurs="0" />
<xsd:element name="cafile" type="xsd:string" minOccurs="0" />
<xsd:element name="capath" type="xsd:string" minOccurs="0" />
<xsd:element name="local_cert" type="xsd:string" minOccurs="0" />
<xsd:element name="local_pk" type="xsd:string" minOccurs="0" />
<xsd:element name="passphrase" type="xsd:string" minOccurs="0" />
<xsd:element name="ciphers" type="xsd:string" minOccurs="0" />
<xsd:element name="peer_fingerprint" type="fingerprint" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="http_client_client">
<xsd:sequence>
<xsd:element name="default_options" type="http_client_options" minOccurs="0" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="fingerprint">
<xsd:sequence>
<xsd:any minOccurs="0" processContents="lax" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="http_query">
<xsd:sequence>
<xsd:any minOccurs="0" processContents="lax" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="http_headers">
<xsd:sequence>
<xsd:any minOccurs="0" processContents="lax" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>
</xsd:schema>
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
'buses' => ['messenger.bus.default' => ['default_middleware' => true, 'middleware' => []]],
],
'disallow_search_engine_index' => true,
'http_client' => [
'enabled' => false,
'max_host_connections' => 6,
'clients' => [],
],
];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

$container->loadFromExtension('framework', [
'http_client' => [
'max_host_connections' => 4,
'default_options' => null,
'clients' => [
'foo' => [
'default_options' => null,
],
],
],
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

$container->loadFromExtension('framework', [
'http_client' => [
'default_options' => [
'auth' => 'foo:bar',
'query' => ['foo' => 'bar', 'bar' => 'baz'],
'headers' => ['X-powered' => 'PHP'],
'max_redirects' => 2,
'http_version' => '2.0',
'base_uri' => 'http://example.com',
'buffer' => true,
'resolve' => ['localhost' => '127.0.0.1'],
'proxy' => 'proxy.org',
'timeout' => 3.5,
'bindto' => '127.0.0.1',
'verify_peer' => true,
'verify_host' => true,
'cafile' => '/etc/ssl/cafile',
'capath' => '/etc/ssl',
'local_cert' => '/etc/ssl/cert.pem',
'local_pk' => '/etc/ssl/private_key.pem',
'passphrase' => 'password123456',
'ciphers' => 'RC4-SHA:TLS13-AES-128-GCM-SHA256',
'peer_fingerprint' => [
'pin-sha256' => ['14s5erg62v1v8471g2revg48r7==', 'jsda84hjtyd4821bgfesd215bsfg5412='],
'md5' => 'sdhtb481248721thbr=',
],
],
],
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

$container->loadFromExtension('framework', [
'http_client' => [
'default_options' => [
'headers' => ['foo' => 'bar'],
],
'clients' => [
'foo' => [
'default_options' => [
'headers' => ['bar' => 'baz'],
],
],
],
],
]);
Loading