Need feedback for consuming an external API

Best Answer

Level 56

I avoid having side-effects on an object's constructor, so I'd move the HTTP client construct to a different method, and accept the API parameters on the constructor:

<?php

namespace App\Services;

use Illuminate\Http\Client\PendingRequest;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\Http;

class SomeWebServiceAPI
{
    public function __construct(
        private readonly string $baseUrl,
        private readonly string $tokenName,
        private readonly int $tokenExpiration,
        private readonly string $username,
        private readonly string $password,
    ) {
    }

    public function allOrders(): array
    {
        return $this->apiClient()
            ->withToken($this->apiToken())
            ->get('/orders')
            ->json();
    }

    public function orderInfo($id): array
    {
        return $this->apiClient()
            ->withToken($this->apiToken())
            ->get('/orders/' . $id)
            ->json();
    }

    private function apiClient(): PendingRequest
    {
        return Http::baseUrl($this->baseUrl)->throw();
    }

    private function apiToken(): string
    {
        $token = Cache::remember($this->tokenName, $this->tokenExpiration, function () {
            $response = $this->apiClient()->post('/auth', [
                'username' => $this->username,
                'password' => $this->password,
            ])->json();

            return Crypt::encryptString($response['token']);
        });

        return Crypt::decryptString($token);
    }
}

Note individual enpoint methods get very slim, and as you suggest in the comment, I would parse the response and return some sort of DTO or value object instead of the raw array from the response.

Also note I am using PHP 8.1 promoted constructor properties. Read about how it works on PHP docs:

To tell the container how to fill the constructor properties, you need to add contextual bindings in a Service Provider's register method, like so:

<?php

namespace App\Providers;

use App\Services\SomeWebServiceAPI;
use Illuminate\Support\ServiceProvider;

class AppServiceProvider extends ServiceProvider
{
    public function register()
    {
        $this->app->when(SomeWebServiceAPI::class)
            ->needs('$baseUrl')
            ->giveConfig('some_web_service.url');

        $this->app->when(SomeWebServiceAPI::class)
            ->needs('$tokenName')
            ->giveConfig('some_webservice.api_token_name');

        $this->app->when(SomeWebServiceAPI::class)
            ->needs('$tokenExpiration')
            ->giveConfig('some_webservice.api_token_expiration');

        $this->app->when(SomeWebServiceAPI::class)
            ->needs('$username')
            ->giveConfig('some_webservice.username');

        $this->app->when(SomeWebServiceAPI::class)
            ->needs('$password')
            ->giveConfig('some_webservice.password');
    }

    public function boot()
    {
        //
    }
}

This way when you need the SomeWebServiceAPI somewhere, you can inject it on a controller's method signature, or use the resolve() helper, and the container will auto-wire the correct values from your config to the service class.

Read more about contextual bind on Laravel docs:

https://laravel.com/docs/9.x/container#contextual-binding

And more about how Laravel's container resolution works also in its docs:

An advantage of avoiding calling the config() helper inside classes is making it easier to reuse the class with different values, for example when you want to test it, without messing with swapping config values.

Also if you want to reorganize your config files as your system grows, you will have less places to search for usage.

Hope this helps =)

3 likes