Skip to content

Allow to register PSR17 and PSR18 classes as services #372

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 1 commit into from
Dec 27, 2019
Merged

Allow to register PSR17 and PSR18 classes as services #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@
"php-http/curl-client": "<2.0"
},
"require-dev": {
"guzzlehttp/psr7": "^1.0",
"matthiasnoback/symfony-dependency-injection-test": "^4.0",
"nyholm/nsa": "^1.1",
"nyholm/psr7": "^1.2.1",
"php-http/cache-plugin": "^1.7",
"php-http/guzzle6-adapter": "^1.1.1 || ^2.0.1",
"php-http/mock-client": "^1.2",
Expand Down
14 changes: 14 additions & 0 deletions src/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,33 @@ public function getConfigTreeBuilder()
->info('Configure which service the main alias point to.')
->children()
->scalarNode('client')->defaultValue('httplug.client.default')->end()
->scalarNode('psr18_client')->defaultValue('httplug.psr18_client.default')->end()
->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
->scalarNode('uri_factory')->defaultValue('httplug.uri_factory.default')->end()
->scalarNode('stream_factory')->defaultValue('httplug.stream_factory.default')->end()
->scalarNode('psr17_request_factory')->defaultValue('httplug.psr17_request_factory.default')->end()
->scalarNode('psr17_response_factory')->defaultValue('httplug.psr17_response_factory.default')->end()
->scalarNode('psr17_stream_factory')->defaultValue('httplug.psr17_stream_factory.default')->end()
->scalarNode('psr17_uri_factory')->defaultValue('httplug.psr17_uri_factory.default')->end()
->scalarNode('psr17_uploaded_file_factory')->defaultValue('httplug.psr17_uploaded_file_factory.default')->end()
->scalarNode('psr17_server_request_factory')->defaultValue('httplug.psr17_server_request_factory.default')->end()
->end()
->end()
->arrayNode('classes')
->addDefaultsIfNotSet()
->info('Overwrite a service class instead of using the discovery mechanism.')
->children()
->scalarNode('client')->defaultNull()->end()
->scalarNode('psr18_client')->defaultNull()->end()
->scalarNode('message_factory')->defaultNull()->end()
->scalarNode('uri_factory')->defaultNull()->end()
->scalarNode('stream_factory')->defaultNull()->end()
->scalarNode('psr17_request_factory')->defaultNull()->end()
->scalarNode('psr17_response_factory')->defaultNull()->end()
->scalarNode('psr17_stream_factory')->defaultNull()->end()
->scalarNode('psr17_uri_factory')->defaultNull()->end()
->scalarNode('psr17_uploaded_file_factory')->defaultNull()->end()
->scalarNode('psr17_server_request_factory')->defaultNull()->end()
->end()
->end()
->arrayNode('profiling')
Expand Down
29 changes: 27 additions & 2 deletions src/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,31 @@
</service>
<service id="Http\Client\HttpClient" alias="httplug.client" public="false" />

<!-- Discovery for PSR-18 -->
<service id="httplug.psr18_client.default" class="Psr\Http\Client\ClientInterface">
<factory class="Http\Discovery\Psr18ClientDiscovery" method="find" />
</service>

<!-- Discovery for PSR-17 -->
<service id="httplug.psr17_request_factory.default" class="Psr\Http\Message\RequestFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findRequestFactory" />
</service>
<service id="httplug.psr17_response_factory.default" class="Psr\Http\Message\ResponseFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findResponseFactory" />
</service>
<service id="httplug.psr17_stream_factory.default" class="Psr\Http\Message\StreamFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findStreamFactory" />
</service>
<service id="httplug.psr17_uri_factory.default" class="Psr\Http\Message\UriFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findUrlFactory" />
</service>
<service id="httplug.psr17_uploaded_file_factory.default" class="Psr\Http\Message\UploadedFileFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findUploadedFileFactory" />
</service>
<service id="httplug.psr17_server_request_factory.default" class="Psr\Http\Message\ServerRequestFactoryInterface">
<factory class="Http\Discovery\Psr17FactoryDiscovery" method="findServerRequestFactory" />
</service>

<!-- PluginClientFactory -->
<service id="Http\Client\Common\PluginClientFactory" class="Http\Client\Common\PluginClientFactory" public="false" />

Expand All @@ -55,8 +80,8 @@
<argument type="service" id="httplug.message_factory"/>
</service>
<service id="httplug.factory.curl" class="Http\HttplugBundle\ClientFactory\CurlFactory" public="false">
<argument type="service" id="httplug.message_factory"/>
<argument type="service" id="httplug.stream_factory"/>
<argument type="service" id="httplug.psr17_response_factory"/>
<argument type="service" id="httplug.psr17_stream_factory"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

won't this lead to regressions for users that have no psr17 implementation available?

the factories throw exception when no implementation is present. should we have a compiler pass that checks if the factories find anything and removes the services if they don't?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is very true. I have to fix that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just tested in Symfony 5.

It is not true, we are only using the factory if we are actually trying to use the service. So this is fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

but are we only trying to use the service when a psr-17 implementation is available?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ie, the curl client is using psr17. It is currently broken.

So if you use the httplug.factory.curl, then we will use these psr17 factory services.
There is nothing that is using these services out of the box.

</service>
<service id="httplug.factory.guzzle5" class="Http\HttplugBundle\ClientFactory\Guzzle5Factory" public="false">
<argument type="service" id="httplug.message_factory"/>
Expand Down
7 changes: 7 additions & 0 deletions tests/Resources/Fixtures/config/full.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
'message_factory' => 'Http\Message\MessageFactory\GuzzleMessageFactory',
'uri_factory' => 'Http\Message\UriFactory\GuzzleUriFactory',
'stream_factory' => 'Http\Message\StreamFactory\GuzzleStreamFactory',
'psr18_client' => 'Http\Adapter\Guzzle6\Client',
'psr17_request_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
'psr17_response_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
'psr17_stream_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
'psr17_uri_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
'psr17_uploaded_file_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
'psr17_server_request_factory' => 'Nyholm\Psr7\Factory\Psr17Factory',
],
'clients' => [
'test' => [
Expand Down
7 changes: 7 additions & 0 deletions tests/Resources/Fixtures/config/full.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@
<message-factory>Http\Message\MessageFactory\GuzzleMessageFactory</message-factory>
<uri-factory>Http\Message\UriFactory\GuzzleUriFactory</uri-factory>
<stream-factory>Http\Message\StreamFactory\GuzzleStreamFactory</stream-factory>
<psr18-client>Http\Adapter\Guzzle6\Client</psr18-client>
<psr17-request-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-request-factory>
<psr17-response-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-response-factory>
<psr17-stream-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-stream-factory>
<psr17-uri-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-uri-factory>
<psr17-uploaded-file-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-uploaded-file-factory>
<psr17-server-request-factory>Nyholm\Psr7\Factory\Psr17Factory</psr17-server-request-factory>
</classes>
<client name="test" factory="httplug.factory.guzzle6" http-methods-client="true">
<plugin>httplug.plugin.redirect</plugin>
Expand Down
7 changes: 7 additions & 0 deletions tests/Resources/Fixtures/config/full.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ httplug:
message_factory: Http\Message\MessageFactory\GuzzleMessageFactory
uri_factory: Http\Message\UriFactory\GuzzleUriFactory
stream_factory: Http\Message\StreamFactory\GuzzleStreamFactory
psr18_client: Http\Adapter\Guzzle6\Client
psr17_request_factory: Nyholm\Psr7\Factory\Psr17Factory
psr17_response_factory: Nyholm\Psr7\Factory\Psr17Factory
psr17_stream_factory: Nyholm\Psr7\Factory\Psr17Factory
psr17_uri_factory: Nyholm\Psr7\Factory\Psr17Factory
psr17_uploaded_file_factory: Nyholm\Psr7\Factory\Psr17Factory
psr17_server_request_factory: Nyholm\Psr7\Factory\Psr17Factory
clients:
test:
factory: httplug.factory.guzzle6
Expand Down
29 changes: 29 additions & 0 deletions tests/Unit/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Http\Message\MessageFactory\GuzzleMessageFactory;
use Http\Message\UriFactory\GuzzleUriFactory;
use Http\Message\StreamFactory\GuzzleStreamFactory;
use Nyholm\Psr7\Factory\Psr17Factory;
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
Expand All @@ -25,12 +26,26 @@ class ConfigurationTest extends AbstractExtensionConfigurationTestCase
'message_factory' => 'httplug.message_factory.default',
'uri_factory' => 'httplug.uri_factory.default',
'stream_factory' => 'httplug.stream_factory.default',
'psr18_client' => 'httplug.psr18_client.default',
'psr17_request_factory' => 'httplug.psr17_request_factory.default',
'psr17_response_factory' => 'httplug.psr17_response_factory.default',
'psr17_stream_factory' => 'httplug.psr17_stream_factory.default',
'psr17_uri_factory' => 'httplug.psr17_uri_factory.default',
'psr17_uploaded_file_factory' => 'httplug.psr17_uploaded_file_factory.default',
'psr17_server_request_factory' => 'httplug.psr17_server_request_factory.default',
],
'classes' => [
'client' => null,
'psr18_client' => null,
'message_factory' => null,
'uri_factory' => null,
'stream_factory' => null,
'psr17_request_factory' => null,
'psr17_response_factory' => null,
'psr17_stream_factory' => null,
'psr17_uri_factory' => null,
'psr17_uploaded_file_factory' => null,
'psr17_server_request_factory' => null,
],
'clients' => [],
'profiling' => [
Expand Down Expand Up @@ -117,12 +132,26 @@ public function testSupportsAllConfigFormats(): void
'message_factory' => 'my_message_factory',
'uri_factory' => 'my_uri_factory',
'stream_factory' => 'my_stream_factory',
'psr18_client' => 'httplug.psr18_client.default',
'psr17_request_factory' => 'httplug.psr17_request_factory.default',
'psr17_response_factory' => 'httplug.psr17_response_factory.default',
'psr17_stream_factory' => 'httplug.psr17_stream_factory.default',
'psr17_uri_factory' => 'httplug.psr17_uri_factory.default',
'psr17_uploaded_file_factory' => 'httplug.psr17_uploaded_file_factory.default',
'psr17_server_request_factory' => 'httplug.psr17_server_request_factory.default',
],
'classes' => [
'client' => Client::class,
'message_factory' => GuzzleMessageFactory::class,
'uri_factory' => GuzzleUriFactory::class,
'stream_factory' => GuzzleStreamFactory::class,
'psr18_client' => Client::class,
'psr17_request_factory' => Psr17Factory::class,
'psr17_response_factory' => Psr17Factory::class,
'psr17_stream_factory' => Psr17Factory::class,
'psr17_uri_factory' => Psr17Factory::class,
'psr17_uploaded_file_factory' => Psr17Factory::class,
'psr17_server_request_factory' => Psr17Factory::class,
],
'clients' => [
'test' => [
Expand Down