version 4 installation with psr client

[dbu]

feat-v4 only is installable with the current instructions by "chance".

{
    "name": "test/foo",
    "require": {
        "surfoo/geocaching-php-sdk": "dev-feat-v4"
    }
}

$ composer update
...
$ composer why guzzlehttp/guzzle
league/oauth2-client 2.7.0 requires guzzlehttp/guzzle (^6.0 || ^7.0) 

while the oauth2-client will probably always need a psr client, this is a rather indirect way. if you require php-http/discovery in your require section, the discovery plugin will install a client if none is already installed.

it should then also be possible to remove the require-dev for php-http/curl-client (unless you need specifically that one for mocking, i did not check that)

[Surfoo]

Hello @dbu, thanks for your feedback! I have updated #14

php-http/curl-client is installed for the unit tests.

[dbu]

i don't see you explicitly using the curl client in any tests. if you remove the requirement, the discovery composer plugin should now automatically install a psr client if needed.

oh, i tried this in the code and noticed you use the old httplug instead of psr18 client. HttpClientDiscovery is deprecated in favor of Psr18ClientDiscovery.
php-http/httplug is the predecessor of psr/http-client. the authors of httplug recommend to use the psr18 http client interface (which happens to have compatible signatures, so the upgrade path should be easy).

[Surfoo]

Oh, thanks! I have fixed the dependency : 51101f2

It's not really easy to understand, I read an good but old article about in order to help me: https://madewithlove.com/blog/building-an-sdk-with-php-part-1/

[dbu]

yay, that looks right to me.

the tutorial seems good to me, only unfortunately while it talks about Psr18 client, the code uses the old httplug discovery which likely is what mislead you.

[Surfoo]

What do you think about these repetitions of http_build_query: https://github.com/Surfoo/geocaching-php-sdk/blob/feat-v4/src/GeocachingSdk.php#L70 How can I avoid them?

[Surfoo]

I rewrote the code, but the API have a swagger, what do you recommend to generate the code?

[dbu]

for the query: you could use the psr client directly and the psr17 request factory to build a request object. then you can use $request->withQueryParams. the HttpMethodsClient is a convenience wrapper but currently does not offer dedicated support for building the query string.

for generating code from openapi spec, i used janephp once. the nice thing is that it builds you models for all the data. the not so nice thing is that the generated code is on the verbose side and if the openapi spec is not super nice the resulting sdk will also not be nice...

[Surfoo]

for generating code from openapi spec...

Thanks, I think it's better to write my own code so...

for the query: you could use the psr client directly and the psr17 request factory to build a request object. then you can use $request->withQueryParams. the HttpMethodsClient is a convenience wrapper but currently does not offer dedicated support for building the query string.

Thanks for the help, but I think I don't get it... I tried not to use the ClientBuilder and Options class, but I missed something in the process.. My commit is here: 46728ff do you think you can help me?

[dbu]

i don't think the http_build_query is bad, you asked about alternatives ;-)

i think you would keep the structure the same with options and all, just change for the ClientInterface & the RequestFactoryInterface of psr17 (php-http/discovery has a Psr17FactoryDiscovery.php to find the request and uri factories). you then build the request with the factory, set the uri on it and have withQueryParams on that. the ClientInterface only has a method to send a request, no shortcuts like the HttpMethodsClient.

but imho it was alright before, with the methods client.

[Surfoo]

Ok, I changed to the ClientInterface in c305830
But for the remaining changes, Psr17FactoryDiscovery is already used by default in the constructor : https://github.com/Surfoo/geocaching-php-sdk/blob/feat-v4/src/ClientBuilder.php#L32 and if I skip the HttpMethodsClient, how to use the authentication middleware ?

It's very interesting for me, I want to go further to understand it better but without a real example of code it's very difficult to understand 😲

[dbu]

the PluginClient is a decorator on the psr client, so you can return that one, to use the authentication middleware. when you look at ClientInterface, you can see that it only has the method sendRequest(RequestInterface $request): ResponseInterface. so in the places where you send requests, you use the request factory to build a request object, and use the request->withQueryParams to set your parameters.

i would create getter methods getRequestFactory(): RequestFactoryInterface and getStreamFactory(): StreamFactoryInterface on the ClientBuilder

[dbu]

alternatively, you could do your own variant of the HttpMethodClient and use that in the application.

class HttpClient
{
    public function __construct(
        ClientInterface $httpClient = null,
        RequestFactoryInterface $requestFactoryInterface = null,
        StreamFactoryInterface $streamFactoryInterface = null
    ) {
        $this->httpClient = $httpClient ?: Psr18ClientDiscovery::find();
        $this->requestFactory = $requestFactory ?: Psr17FactoryDiscovery::findRequestFactory();
        $this->streamFactory = $streamFactory ?: Psr17FactoryDiscovery::findStreamFactory();
    }
    public function get(string $uri, array $query, array $headers): ResponseInterface
    {
        $request = $this->requestFactory->createRequest('GET', $uri)
            ->withQueryParams($query)
        ;
        foreach ($headers as $key => $value) {
            $request = $request->withHeader($key, $value);
        }

        return $this->httpClient->sendRequest($request);
    }
}

[dbu]

and analog methods for the other HTTP verbs that you use in the sdk.

[Surfoo]

Hum, withQueryParams in on ServerRequestInterface according to the PSR 7: https://www.php-fig.org/psr/psr-7/
I can't use it on RequestInterface 🤔

And VSCode tell me the same thing:

[dbu]

oh damn, you are right. sorry about that - i did not look closely enough.

sorry for the false trail, i think then the http_build_query really is the way to go. you could still do your own variant of client wrapper like i suggested in my comment above but have it use http_build_query and decorate the HttpMethodsClient (or you request factory, as you prefer)

or you can just leave it as it was with the http_build_query in all the places.