fix(storage): correctly use global retryStrategy if defined

[jayktaylor]

fixes #9334

[cy-yun]

The logic of the fix here is sound, and changing the test to use createHmacKey (which isn't idempotent) correctly verifies that the global config is now being honored.

However, I noticed that the exact same bug is still present in Rest::headObject() (around line 320 in Storage/src/Connection/Rest.php).

In headObject(), $this->getRestRetryFunction() is being called before retryStrategy gets merged into the array:

        $args['restRetryFunction'] = $this->restRetryFunction ?? $this->getRestRetryFunction(
            'objects',
            'get',
            $args
        );

        $args += array_filter([
            'retryStrategy' => $this->retryStrategy, // <--- Too late!
            'restDelayFunction' => $this->restDelayFunction,
            // ...
        ]);

Could you apply the exact same fix to headObject() as well?

        $args['retryStrategy'] ??= $this->retryStrategy;

        $args['restRetryFunction'] = $this->restRetryFunction ?? $this->getRestRetryFunction(
            'objects',
            'get',
            $args
        );

        $args += array_filter([
            'restDelayFunction' => $this->restDelayFunction,
            // ...

I'll be happy to approve once that spot is updated as well. Thanks for working on this!

[jayktaylor]

@cy-yun thanks for looking over this! Good spot - I hadn't noticed that but it should be fixed now. From a quick look elsewhere in the library, I think there shouldn't be anything else that has the exact same problem.

[jayktaylor]

Hey @cy-yun and co, is it possible to get this merged? Been waiting on this to be fixed upstream for one of our projects for a while :)