API Client and usage implementation

Question

I'm really bothered with my approach in this, as I'd like to use the client in multiple projects.

The client extends Guzzle and I'm using a factory method to initialize the client with the necessary settings:

<?php

namespace App\Services\ApiClient;


use App\Tenant;
use GuzzleHttp\Client as GuzzleClient;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Middleware;
use GuzzleHttp\Psr7\Request;
use Illuminate\Support\Str;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\UriInterface;

class ApiClient extends GuzzleClient {

    protected Tenant $tenant;

    public static function factory(Tenant $tenant)
    {


        $handler = HandlerStack::create();

        $handler->push(Middleware::mapRequest(function (RequestInterface $request) use ($tenant) {
            $extraParams = ['tenant' => $tenant->api_tenant];

            $uri = $request->getUri();
            $uri .= (isset(parse_url($uri)['query']) ? '&' : '?');
            $uri .= http_build_query($extraParams);

            return new Request(
                $request->getMethod(),
                $uri,
                $request->getHeaders(),
                $request->getBody(),
                $request->getProtocolVersion()
            );
        }));

        $client = [
            'base_uri' => rtrim($tenant->api_base_url, '/') . "/api/{$tenant->api_rest_version}/companies({$tenant->api_company_id})/",
            'timeout' => 5.0,
            'handler' => $handler,
            'auth' => [$tenant->api_user, $tenant->api_password],
            'curl' => [CURLOPT_SSL_VERIFYPEER => false],
            'debug' => false,
            'headers' => [
                'Accept' => 'application/json',
                'If-Match' => '*',
                'Accept-Language' => 'en-US',
                'OData-Version' => '4.0',
                'Prefer' => 'odata.continue-on-error',
            ],
        ];

        return new static($client, $tenant);
    }
    public function __construct(array $config = [], Tenant $tenant)
    {
        parent::__construct($config);
        $this->tenant = $tenant;
    }

    public function getBatchUri() {
        return rtrim($this->tenant->api_base_url, '/') . "/api/{$this->tenant->api_rest_version}/\$batch";
    }

    /**
     * Converts request data for batch preparation.
     *
     * @param array $body
     * @param string $method
     * @param string $url
     * @return array
     */
    public function batch(array $body = [], $method = 'POST', string $url)
    {

        $extraParams = ['tenant' => $this->tenant->api_tenant];

        $uri = $url;
        $uri .= (isset(parse_url($uri)['query']) ? '&' : '?');
        $uri .= http_build_query($extraParams);

        return array_filter([
            'method' => $method,
            'atomicityGroup' => uniqid(null, true),
            'id' => 'id_' . uniqid(null, true),
            'url' => $uri,
            'body' => (empty($body)) ? null : $body,
            'headers' => [
                'Content-Type' => 'application/json; odata.metadata=minimal; odata.streaming=true',
                'OData-Version' => '4.0',
                'If-Match' => '*',
                'Prefer' => 'odata.continue-on-error',
            ],
        ]);

    }

}

The available endpoints that the client is using differs based on the Tenant and the application the client is being used in.

As an example, I'm using the client to get some contacts from the external service and I then try and use that information for authentication.

Example:

<?php

namespace App\Http\Controllers\Api;

use App\Http\Controllers\Controller;
use Illuminate\Http\Request;

class AuthenticationController extends Controller
{

    public function login(Request $request)
    {
        $request->validate([
            'email' => 'required|string',
            'password' => 'required|string',
        ]);

        if (!$user = Auth::guard('nav')->attempt($request->all())) {
            throw ValidationException::withMessages([
                'email' => ['The provided credentials are incorrect.'],
            ]);
        }

        return response()->json(['token' => $this->createToken($user)]);
    }
}

The attempt method contains some nested methods that ends up with the following use of the API Client:

    public function retrieveByCredentials(array $credentials)
    {
        $request = $this->apiClient->get('contacts', ['query' => [
            '$filter' => "E_Mail eq '{$credentials['email']}'",
            '$top' => 1
        ]]);


        $data = json_decode($request->getBody()->getContents());

        if(!empty($data->value)) {
           return new NavContact((array)$data->value[0]);
        }

        return null;
    }

NavContact is a Data Transfer Object (that resembles an Eloquent Model).

Now, the whole ordeal feels and looks awful (to me at least). And I'm not particularly interested in having a client library blowing up to the size of PayPal or Google's PHP libraries in the same way I'm not particularly interested in having something that requires me to use my memory alone to fetch the right endpoints with the required query parameters.

I'm sure there must be a place in the middle?

Answer 1

While there isn't a lot of code here, there is some duplicated code - e.g.

$extraParams = ['tenant' => $tenant->api_tenant];

$uri = $request->getUri();
$uri .= (isset(parse_url($uri)['query']) ? '&' : '?');
$uri .= http_build_query($extraParams);

in the callback to Middleware::mapRequest passed to $handler->push() in the factory() method, as well as similar lines in the batch() method:

$extraParams = ['tenant' => $this->tenant->api_tenant];

$uri = $url;
$uri .= (isset(parse_url($uri)['query']) ? '&' : '?');
$uri .= http_build_query($extraParams);

This could be seen as a violation of the Don't Repeat Yourself principle. The similar lines could be abstracted into a static method that accepts a URL and a Tenant object.