Add PSR-7/17/18 HTTP services

[Mark-H]

What does it do?

Provides a standardized way of sending HTTP requests using Guzzle (or other compatible clients).

This should also be seen as deprecating modRest, alongside modRestClient which was already marked as such.

Why is it needed?

Was working on fixing the package manager to no longer use modRestClient (#15469), however instead of trying to do it all ourselves we really ought to make use of PHP-wide standards. As it happens, PSR-7, PSR-17 and PSR-18 clearly outlines how to work with HTTP requests, so this implements that.

For the default implementation I've selected Guzzle. It's the de-facto standard, compliant with these specs, and actively supported matching our minimum requirements (PHP 7.2) as it happens.

How to test

Here's an example GET request as it may appear in a snippet. Notice how it only uses interfaces, decoupling the implementation entirely from the calling code.

<?php
$client = $modx->services->get(\Psr\Http\Client\ClientInterface::class);
$factory = $modx->services->get(\Psr\Http\Message\RequestFactoryInterface::class);
$request = $factory->createRequest('GET', 'http://ipinfo.io/')
    ->withHeader('Accept', 'application/json')
    ->withHeader('Content-Type', 'application/json');
try {
    $response = $client->sendRequest($request);
} catch (\Psr\Http\Client\ClientExceptionInterface $e) {
    $modx->log(1, $e->getMessage());
    return 'Error: ' . $e->getMessage();
}
$body = json_decode($response->getBody()->getContents());
return '<pre>' . $response->getStatusCode() . ' : ' . print_r($body, true) . '</pre>';

After pulling in the branch, it's necessary to run a composer update to load new dependencies.

Related issue(s)/PR(s)

Documentation: https://github.com/modxorg/Docs/pull/349/files
modRestClient deprecated: #15469

Following up on this assuming positive feedback/approval I will start implementing these into the package manager (which is the primary modRestClient implementation in core) which is a beta-blocking task.

[opengeek]

Awesome work, @Mark-H ! I will review this tomorrow.

[JoshuaLuckers]

In other places in the code we define services using an alias. E.g; $this->services->add('parser', new $parserClass($this));
This is in line with the documentation of Pimple.

Is there a particular reason you decided using the FQC-names here?

[opengeek]

In other places in the code we define services using an alias. E.g; $this->services->add('parser', new $parserClass($this));
This is in line with the documentation of Pimple.

Is there a particular reason you decided using the FQC-names here?

I think in some cases using a simple alias is appropriate (our parser is not one of those cases—this is to maintain BC), but when configuring implementations of interfaces like those defined here, I think it is best to use the interface name itself. Most container configuration is done this way. You have to remember that Pimple is a bit old and some of the documentation is not inline with modern best practices. In nearly all applications I build these days, I use the FQ class or interface names to define 90% or more of dependencies in the container. I'll try to find some discussions that explain why this is now considered best practice.

[Mark-H]

EDIT: Looks like Jason and I were responding at the same time.

Worth debating.

I'd say the use of a short alias is a legacy thing we don't have to follow for new services, and that using the actual interface names we're implementing helps with auto discovery/autowiring if we switch to a more powerful container like PHP-DI. IDEs may also be clever enough to understand the type of what's returned from a container that way.

It's my anecdotal experience using classnames is "more correct".

For the services currently in the container, we don't have interfaces.

[opengeek]

I'm ready to merge this if no one has objections. @JoshuaLuckers ?

[JoshuaLuckers]

@opengeek sounds good to me!