Please remove dependency on puli. We do not use it, nor symfony stuff. The dependency is too much for our project. I like the discovery for the adapters, but...

Reverting php-http/discovery back to 0.5 saves a lot of deps!

- Removing padraic/phar-updater (dev-master)
- Removing symfony/finder (2.8.x-dev)
- Removing padraic/humbug_get_contents (dev-master)
- Removing webmozart/console (dev-master)
- Removing symfony/console (2.8.x-dev)
- Removing symfony/polyfill-mbstring (dev-master)
- Removing symfony/process (2.8.x-dev)
- Removing psr/log (dev-master)
- Removing ramsey/uuid (2.8.x-dev)
- Removing symfony/event-dispatcher (2.8.x-dev)
- Removing symfony/filesystem (2.8.x-dev)
- Removing webmozart/expression (dev-master)
- Removing webmozart/key-value-store (dev-master)
- Removing webmozart/json (dev-master)
- Removing seld/jsonlint (1.4.0)
- Removing justinrainbow/json-schema (1.5.0)
- Removing webmozart/glob (dev-master)
- Removing webmozart/path-util (dev-master)
- Removing webmozart/assert (dev-master)

I have been looking at puli for the last few weeks. I think @joelwurtz also spent some time with it, at least during PHP Forum with @webmozart.

I think it is quite awesome and we could easily use it not just in the discovery package, but in the whole project. Since we have nice contracts, we already have out binding types: HttpClient, HttpAsyncClient, *Factory. In each of our packages we can provide a puli.json with the actual bindings (like Guzzle6HttpAdapter is bind to HttpClient and HttpAsyncClient)

Downside of this solution is that every resource must be registered in the repository (?) and conditional auto-discovery (and caching) during runtime won't work. However, this is not necessarily a downside, but probably makes flexibility a bit harder.

While this could make our static discoveries obsolete, I would still keep them. Although Puli does not recommend it to use it in global context or instantiate it more than once, I think we should use them inside our discovery layer. The workflow would be the following:

If someone wants to resolve something from the discovery, and it is the first call to any discoveries, we instantiate the Puli factory and store it in the ClassDiscovery object. After that, inside the specific discovery (like HttpClientDiscovery) we loop through our internal list of clients and append them to Puli ONCE. We set an initialized bit or something. This replaces our internal cache mechanism. After that, we resolve the first match of the specific binding and return it.

Possible pitfalls:

• Performance: Puli recommends NOT to this global context thing for performance (and other) reasons. However our discovery thing's aim is zero config, which itself is not recommended. Configuration, DI should be used, but if you need it as is, you can use it. Not sure if we need to care about this apart from placing a big fat warning somewhere.
• Priority: not sure about Puli's priority handling. FIFO? LIFO? Does it really matter? IMO when we use a discovery, we just want something fast, should we really care about priorities? Currently it is possible to add resources to discoveries and they are returned FIFO.
• Puli required: with this we would add puli as a dependency. Not sure if it is really a downside, as puli is quite awesome. Putting a puli.json file in all of the related repos should really not be a problem.

Reference: https://speakerdeck.com/webmozart/puli-phps-next-package-revolution?slide=102

I was not too happy about #58. We treat Puli in a special manner which feels weird. Also, I was not too happy about the logic in the fallback strategies.

This PR continues the work and replaces #58. The user has now the possibility to select what strategies to use and also add some more custom if there is a need for it.

Changes

• Introduced Stratiegies that do the discovery. We try the strategies one by one until we find what we're looking for
• many public functions are moved from ClassDiscovery to Strategy\Puli.
• I've also created some new exceptions.
• Added a simple cache mechanism
• When Puli is missing we will not throw a \RuntimeException anymore. There will be a NotFoundException which has previous exception so you can see that Puli was missing.
• The NotFoundException is moved from Http\Discovery to Http\Discovery\Exception

TODO

This is just a proof of concept. Do we like to go this way?

• [x] Add tests
• [ ] Update documentation

| Q | A | ------------ | --- | Bug? | no|yes | New Feature? | no|yes | Version | 1.4.0

Actual Behavior

Due to the missing dependency, the sourcecode of nyholm/psr7 is never retrieved from packagist.org, resulting in the following bug:

[2018-03-07 15:33:57] main.ERROR: Source class "\Nyholm\Psr7\Factory\Stream" for "Nyholm\Psr7\Factory\StreamFactory" generation does not exist. [] []

Expected Behavior

No errors...

Steps to Reproduce

1. git clone
2. composer install
3. run it?

Possible Solutions

specify nyholm/psr7 in the require section of composer.json

| Q | A | --------------- | --- | Bug fix? | yes | New feature? |no | BC breaks? | no | Deprecations? | no | Related tickets | #55 | Documentation | | License | MIT

What's in this PR?

This PR adds the php-http/mock-client to the list of discoverable classes via the Common Classes Strategy.

Why?

Since Puli is not a hard dependency on this package, all php-http clients should be able to be discovered via the discovery. Since package developers using php-http are likely to rely on the Mock client in tests, it is useful to make this client discoverable for better unit testing.

Example Usage

// ThirdPartyService.php

use Http\Client\HttpClient;
use Http\Discovery\HttpClientDiscovery;

class ThirdPartyService
{
    public function __construct(HttpClient $http = null)
    {
        $this->http = $http ?: HttpClientDiscovery::find();
    }
}

Now, before this PR, if we were to pass no value into ThirdPartyService's constructor, the code would fail complaining about missing Puli dependency (assuming our package requires only php-http/mock-client). That means the following test in our package will fail:

// ThirdPartyServiceTest.php

use ThirdPartyService

class ThirdPartyServiceTest extends PHPUnit_Framework_TestCase
{
    public function setUp()
    {
        $this->service = new ThirdPartyService;
    }
}

This PR fixes that issue and allows the above test to pass, without requiring Puli and using the Mock HTTP client.

Checklist

• [ ] Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• [ ] Documentation pull request created (if not simply a bugfix)

To Do

• [ ] If the PR is not complete but you want to discuss the approach, list what remains to be done here

I started this issue from the discussion here: https://github.com/php-http/HttplugBundle/pull/81

Our BC promise in this library is the following functions:

HttpAsyncClientDiscovery::find();
HttpClientDiscovery::find();
MessageFactoryDiscovery::find();
StreamFactoryDiscovery::find();
UriFactoryDiscovery::find();

ClassDiscovery::setStrategies();
ClassDiscovery::clearCache();

// And the
DiscoveryStrategy interface

// And our exceptions and when they are thrown

This API has nothing to do with the discovery methods (strategies) itself. The strategy classes should not be a part of our API promise. Which is a great thing.

If we change the namespace of our clients we just add another "NewNamespacesStrategy" together with our existing CommonClassesStrategy. Same with if Puli changes their API when they release 1.0. We just add a "Puli1Strategy".

So if we leave the strategies outside our BC promise, are we ready for a stable release? At least, can we mature in to this API and tag 1.0 after the summer?

Hello,

I'm using the Happyr/LinkedIn-API-client package, which has recently been updated to use PSR-7.

Since the update I'm getting the following error:

[RuntimeException]             
Puli Factory is not available

This happens in the ClassDiscovery class on getPuliFactory method.

When going to https://github.com/puli/factory, I see it's not maintained anymore.

Anything I missed ?

Fixes https://github.com/php-http/discovery/issues/158

The change introduced in https://github.com/php-http/discovery/commit/366a549f1a9d8f0ac6fc91535a42d7080b560e57 breaks our application.

In some cases it doesn't matter if the installed client is sync or async (for example when known to be used in PluginClient). For that we could have a discovery which finds a sync or async client.

I just update my vendor and the puli build command becomes broken. I have try to remove my local puli.json and ./puli directory but still got the same error.

Here the stacktrace of the puli build command:

./bin/puli build -vv



  [InvalidArgumentException]                  
  The flags must be an integer. Got: boolean  



Exception trace:
  ()
    vendor/webmozart/assert/src/Assert.php:149
  Webmozart\Assert\Assert::integer()
    vendor/webmozart/key-value-store/src/JsonFileStore.php:119
  Webmozart\KeyValueStore\JsonFileStore->__construct()
    .puli/GeneratedPuliFactory.php:41
  Puli\GeneratedPuliFactory->createRepository()
    vendor/puli/manager/src/Api/Puli.php:508
  Puli\Manager\Api\Puli->getRepository()
    vendor/puli/manager/src/Api/Puli.php:653
  Puli\Manager\Api\Puli->getRepositoryManager()
    vendor/puli/cli/src/PuliApplicationConfig.php:208
  Puli\Cli\PuliApplicationConfig->Puli\Cli\{closure}()
    n/a:n/a
  call_user_func()
    vendor/webmozart/console/src/Api/Config/Config.php:285
  Webmozart\Console\Api\Config\Config->getHandler()
    vendor/webmozart/console/src/Api/Command/Command.php:499
  Webmozart\Console\Api\Command\Command->doHandle()
    vendor/webmozart/console/src/Api/Command/Command.php:388
  Webmozart\Console\Api\Command\Command->handle()
    vendor/webmozart/console/src/ConsoleApplication.php:266
  Webmozart\Console\ConsoleApplication->run()
    vendor/puli/cli/bin/puli:23

| Q | A | ------------ | --- | Bug? | no | New Feature? | no | Version | master

Just a quick question: In Http\Discovery\Strategy\CommonClassesStrategy you use classes i.e. Nyholm\Psr7\Request and this dependency is not defined in the composer.json. How does it work? This package composer.json states:

"require": {
        "php": "^5.5 || ^7.0"
    }

The Nyholm composer.json (https://github.com/Nyholm/psr7/blob/master/composer.json) states:

"require": {
        "php": "^7.1",

How can I use this package without conflicts and how is it supposed to be used when the dependencies are not explicitly declared? Thanks a lot guys.

| Q | A | --------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | yes | Deprecations? | no | Related tickets | - | Documentation | - | License | MIT

What's in this PR?

PR is primarily context to explain proposal to remove Zend UriFactory support.

Why?

The Zend UriFactory is not PSR-17 compliant in the way that it generates it's URIs.

Example:

$uri = new \Zend\Diactoros\Uri('testing');
$this->assertEquals('testing', (string) $uri);

This test case would fail as the resulting Uri will actually be /testing.

Prepending a / is an assumption which has consequences - mentioned on\Psr\Http\Message\UriInterface

* Normally, the empty path "" and absolute path "/" are considered equal as
* defined in RFC 7230 Section 2.7.3. But this method MUST NOT automatically
* do this normalization because in contexts with a trimmed base path, e.g.
* the front controller, this difference becomes significant. It's the task
* of the user to handle both "" and "/".

More info here: https://github.com/php-fig/http-message/blob/master/src/UriInterface.php#L134

As this UriFactory is prepending a / and not allowing the user to handle both "" and "/", different UriFactories can not be used interchangeably - preventing Liskov Substitution principle here.

Recently this caused us an issue where in many of our services, our new client operated as expected - primarily using versions of Guzzle. However, the Zend UriFactory was used in one of our legacy systems. Since our services have a path as part of their base_uri, this was stripped and our URIs were malformed due to prepending / - we addressed this issue by using league/uri.

Checklist

• [ ] Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• [ ] Documentation pull request created (if not simply a bugfix)

To Do

• [ ] The checklist above - primarily raising the PR to first discuss the approach and gain feedback

| Q | A | --------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | - | Documentation | TBC | License | MIT

What's in this PR?

POC for adding support for passing options through to HTTP clients. It is important that these options are standardized, largely supported by every implementation, and that arbitrary implementation specifics are not forwarded, breaking the abstraction. I am initially wanting to support "connect_timeout" and "timeout" only.

Why?

These are important because these mostly default to no timeout, leading in blocking for ever (this was recently a problem for me when the github.com API was partially down for multiple hours last month).

Example Usage

Psr18ClientDiscovery::find(['timeout' => 30]);

Checklist

• [ ] Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• [ ] Documentation pull request created (if not simply a bugfix)

To Do

• [ ] Determine if this feature is wanted
• [ ] Determine what options we want to support
• [ ] Determine if this approach is reasonable, while maintaining BC
• [ ] Determine if we should crash if the resolved client does not support some of the passed options or not
• [ ] Finish implementation
• [ ] Add tests

This package operates (at runtime) on the assumption that following packages are available:

• psr/http-message-implementation
• psr/http-factory-implementation

IMO a good idea to include these in "require".

Yes, that means that downstream implementations need to add a "provide" section on their side, but effectively, that removes one possibly very annoying runtime error that will only be observed very late on.

Ref: https://github.com/sensiolabs/SensioFrameworkExtraBundle/issues/620#issuecomment-805140215