2
\$\begingroup\$

I had this S3Helper class before such that, every time I called a function to any of its methods (which I haven't shown here, but which do things like upload files to s3 or get temporary s3 links), it called the getClient() method and it would make a new client every time.

Now I don't know about the performance hit on that, if calling new S3Client([]) makes http requests every time it's called, but I supposed it did so decided I'd rather just make the client once and then use the same client for future calls.

So I cached the result in a static variable, and now my getClient function checks if the cache exists first, and if it does it returns it, otherwise it makes a new client and sets the cache.

Is this a recommended PHP way of doing things? Can it be improved?

<?php // Code within app\Helpers\Helper.php

namespace App\Helpers;
use Aws\S3\S3Client;

class S3Helper {
    protected static $client = null;

    private static function getAuth() {
        return [
                'endpoint' => env('S3_ENDPOINT'),
                'key' => env('S3_KEY'),
                'secret' => env('S3_SECRET'),
                'bucket' => env('S3_BUCKET')
            ];
    }

    private static function getClient() {
        if (self::$client) return self::$client;

        $auth = self::getAuth();
        self::$client = new S3Client([
            'region' => 'eu-west-2',
            'version' => '2006-03-01',
            'endpoint' => $auth['endpoint'],
            'credentials' => [
                'key' => $auth['key'],
                'secret' => $auth['secret']
            ],
            'use_path_style_endpoint' => true
        ]);
        return self::$client;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Before going through advises just wanted to remind you can alternatively use Aws\Sdk class as a client factory.
Sample usage:

// The same options that can be provided to a specific client constructor can also be supplied to the Aws\Sdk class.
// Use the us-west-2 region and latest version of each client.
$sharedConfig = [
    'region' => 'us-west-2',
    'version' => 'latest'
];

// Create an SDK class used to share configuration across clients.
$sdk = new Aws\Sdk($sharedConfig);

// Create an Amazon S3 client using the shared configuration data.
$client = $sdk->createS3();

Observe AWS SDK for PHP guide, just in case you hadn't yet.
They give a notation there:

Note

We highly recommended that you use the Sdk class to create clients if you're using multiple client instances in your application. The Sdk class automatically uses the same HTTP client for each SDK client, allowing SDK clients for different services to perform nonblocking HTTP requests. If the SDK clients don't use the same HTTP client, then HTTP requests sent by the SDK client might block promise orchestration between services.


But, proceeding with your approach, you have a variation of Singleton pattern (restricts the instantiation of a class to a single object, which can be useful when only one object is required across the system).
In your case S3Helper acting as a factory producing/returning the same S3Client instance.

To prevent instantiation/cloning of S3Helper from the outer code you can close __construct, __clone (and even __wakeup) methods:

class S3Helper {
    ...
    private function __construct() { }
    private function __clone() { }

To eliminate multiple return statements in your static getClient() method use the following condition (set client if it's not set yet):

private static function getClient() {
    if (self::$client === null) {   # use strict comparison here
        $auth = self::getAuth();
        self::$client = new S3Client([
            'region' => 'eu-west-2',
            'version' => '2006-03-01',
            'endpoint' => $auth['endpoint'],
            'credentials' => [
                'key' => $auth['key'],
                'secret' => $auth['secret']
            ],
            'use_path_style_endpoint' => true
        ]);
    }
    return self::$client;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.