From 6e63822e5dbe99cbcbf7988aceb1a62d95a67c1c Mon Sep 17 00:00:00 2001
From: Kim Pepper <kim@pepper.id.au>
Date: Thu, 30 Jan 2025 17:36:56 +0900
Subject: [PATCH] Adds http client factories (#271)

* Adds client factories

Signed-off-by: Kim Pepper <kim@pepper.id.au>

* Split out decider and add tests

Signed-off-by: Kim Pepper <kim@pepper.id.au>

---------

Signed-off-by: Kim Pepper <kim@pepper.id.au>
---
 CHANGELOG.md                                  |  2 +
 composer.json                                 |  5 +-
 .../HttpClient/GuzzleHttpClientFactory.php    | 58 +++++++++++++++
 .../HttpClient/GuzzleRetryDecider.php         | 51 +++++++++++++
 .../HttpClient/HttpClientFactoryInterface.php | 21 ++++++
 .../HttpClient/SymfonyHttpClientFactory.php   | 52 +++++++++++++
 .../GuzzleHttpClientFactoryTest.php           | 29 ++++++++
 tests/HttpClient/GuzzleRetryDeciderTest.php   | 73 +++++++++++++++++++
 .../SymfonyHttpClientFactoryTest.php          | 29 ++++++++
 9 files changed, 319 insertions(+), 1 deletion(-)
 create mode 100644 src/OpenSearch/HttpClient/GuzzleHttpClientFactory.php
 create mode 100644 src/OpenSearch/HttpClient/GuzzleRetryDecider.php
 create mode 100644 src/OpenSearch/HttpClient/HttpClientFactoryInterface.php
 create mode 100644 src/OpenSearch/HttpClient/SymfonyHttpClientFactory.php
 create mode 100644 tests/HttpClient/GuzzleHttpClientFactoryTest.php
 create mode 100644 tests/HttpClient/GuzzleRetryDeciderTest.php
 create mode 100644 tests/HttpClient/SymfonyHttpClientFactoryTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d28434a2..311ae66b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Added a test for the AWS signing client decorator
 - Added PHPStan Deprecation rules and baseline
 - Added PHPStan PHPUnit extensions and rules
+- Added Guzzle and Symfony HTTP client factories.
+- Added 'colinodell/psr-testlogger' as a dev dependency.
 ### Changed
 - Switched to PSR Interfaces
 - Increased PHP min version to 8.1
diff --git a/composer.json b/composer.json
index 8b31a67a..8e8fe44a 100644
--- a/composer.json
+++ b/composer.json
@@ -38,6 +38,7 @@
   "require-dev": {
     "ext-zip": "*",
     "aws/aws-sdk-php": "^3.0",
+    "colinodell/psr-testlogger": "^1.3",
     "friendsofphp/php-cs-fixer": "^v3.64",
     "guzzlehttp/psr7": "^2.7",
     "mockery/mockery": "^1.6",
@@ -53,7 +54,9 @@
   },
   "suggest": {
     "monolog/monolog": "Allows for client-level logging and tracing",
-    "aws/aws-sdk-php": "Required (^3.0.0) in order to use the SigV4 handler"
+    "aws/aws-sdk-php": "Required (^3.0.0) in order to use the AWS Signing Client Decorator",
+    "guzzlehttp/psr7": "Required (^2.7) in order to use the Guzzle HTTP client",
+    "symfony/http-client": "Required (^6.4|^7.0) in order to use the Symfony HTTP client"
   },
   "autoload": {
     "psr-4": {
diff --git a/src/OpenSearch/HttpClient/GuzzleHttpClientFactory.php b/src/OpenSearch/HttpClient/GuzzleHttpClientFactory.php
new file mode 100644
index 00000000..35682d8e
--- /dev/null
+++ b/src/OpenSearch/HttpClient/GuzzleHttpClientFactory.php
@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\HttpClient;
+
+use GuzzleHttp\Client as GuzzleClient;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Middleware;
+use OpenSearch\Client;
+use Psr\Http\Client\ClientInterface;
+use Psr\Log\LoggerInterface;
+
+/**
+ * Builds an OpenSearch client using Guzzle.
+ */
+class GuzzleHttpClientFactory implements HttpClientFactoryInterface
+{
+    public function __construct(
+        protected int $maxRetries = 0,
+        protected ?LoggerInterface $logger = null,
+    ) {
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function create(array $options): ClientInterface
+    {
+        if (!isset($options['base_uri'])) {
+            throw new \InvalidArgumentException('The base_uri option is required.');
+        }
+        // Set default configuration.
+        $defaults = [
+            'headers' => [
+                'Accept' => 'application/json',
+                'Content-Type' => 'application/json',
+                'User-Agent' => sprintf('opensearch-php/%s (%s; PHP %s)', Client::VERSION, PHP_OS, PHP_VERSION),
+            ],
+        ];
+
+        // Merge the default options with the provided options.
+        $config = array_merge_recursive($defaults, $options);
+
+        $stack = HandlerStack::create();
+
+        // Handle retries if max_retries is set.
+        if ($this->maxRetries > 0) {
+            $decider = new GuzzleRetryDecider($this->maxRetries, $this->logger);
+            $stack->push(Middleware::retry($decider(...)));
+        }
+
+        $config['handler'] = $stack;
+
+        return new GuzzleClient($config);
+    }
+
+}
diff --git a/src/OpenSearch/HttpClient/GuzzleRetryDecider.php b/src/OpenSearch/HttpClient/GuzzleRetryDecider.php
new file mode 100644
index 00000000..59d0a1ef
--- /dev/null
+++ b/src/OpenSearch/HttpClient/GuzzleRetryDecider.php
@@ -0,0 +1,51 @@
+<?php
+
+namespace OpenSearch\HttpClient;
+
+use GuzzleHttp\Exception\ConnectException;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Log\LoggerInterface;
+
+/**
+ * Retry decider for Guzzle HTTP Client.
+ */
+class GuzzleRetryDecider
+{
+    public function __construct(
+        protected ?int $maxRetries = 0,
+        protected ?LoggerInterface $logger = null,
+    ) {
+    }
+
+    public function __invoke(int $retries, ?RequestInterface $request, ?ResponseInterface $response, $exception): bool
+    {
+        if ($retries >= $this->maxRetries) {
+            return false;
+        }
+        if ($exception instanceof ConnectException) {
+            $this->logger?->warning(
+                'Retrying request {retries} of {maxRetries}: {exception}',
+                [
+                    'retries' => $retries,
+                    'maxRetries' => $this->maxRetries,
+                    'exception' => $exception->getMessage(),
+                ]
+            );
+            return true;
+        }
+        if ($response && $response->getStatusCode() >= 500) {
+            $this->logger?->warning(
+                'Retrying request {retries} of {maxRetries}: Status code {status}',
+                [
+                    'retries' => $retries,
+                    'maxRetries' => $this->maxRetries,
+                    'status' => $response->getStatusCode(),
+                ]
+            );
+            return true;
+        }
+        // We only retry if there is a 500 or a ConnectException.
+        return false;
+    }
+}
diff --git a/src/OpenSearch/HttpClient/HttpClientFactoryInterface.php b/src/OpenSearch/HttpClient/HttpClientFactoryInterface.php
new file mode 100644
index 00000000..cd1a78cf
--- /dev/null
+++ b/src/OpenSearch/HttpClient/HttpClientFactoryInterface.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\HttpClient;
+
+use Psr\Http\Client\ClientInterface;
+
+/**
+ * Interface for OpenSearch client factories.
+ */
+interface HttpClientFactoryInterface
+{
+    /**
+     * Build the OpenSearch client.
+     *
+     * @param array<string,mixed> $options
+     */
+    public function create(array $options): ClientInterface;
+
+}
diff --git a/src/OpenSearch/HttpClient/SymfonyHttpClientFactory.php b/src/OpenSearch/HttpClient/SymfonyHttpClientFactory.php
new file mode 100644
index 00000000..e70e4378
--- /dev/null
+++ b/src/OpenSearch/HttpClient/SymfonyHttpClientFactory.php
@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\HttpClient;
+
+use OpenSearch\Client;
+use Psr\Http\Client\ClientInterface;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpClient\HttpClient;
+use Symfony\Component\HttpClient\Psr18Client;
+use Symfony\Component\HttpClient\RetryableHttpClient;
+
+/**
+ * Builds an OpenSearch client using Symfony.
+ */
+class SymfonyHttpClientFactory implements HttpClientFactoryInterface
+{
+    public function __construct(
+        protected int $maxRetries = 0,
+        protected ?LoggerInterface $logger = null,
+    ) {
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function create(array $options): ClientInterface
+    {
+        if (!isset($options['base_uri'])) {
+            throw new \InvalidArgumentException('The base_uri option is required.');
+        }
+        // Set default configuration.
+        $defaults = [
+            'headers' => [
+                'Accept' => 'application/json',
+                'Content-Type' => 'application/json',
+                'User-Agent' => sprintf('opensearch-php/%s (%s; PHP %s)', Client::VERSION, PHP_OS, PHP_VERSION),
+            ],
+        ];
+        $options = array_merge_recursive($defaults, $options);
+
+        $symfonyClient = HttpClient::create()->withOptions($options);
+
+        if ($this->maxRetries > 0) {
+            $symfonyClient = new RetryableHttpClient($symfonyClient, null, $this->maxRetries, $this->logger);
+        }
+
+        return new Psr18Client($symfonyClient);
+    }
+
+}
diff --git a/tests/HttpClient/GuzzleHttpClientFactoryTest.php b/tests/HttpClient/GuzzleHttpClientFactoryTest.php
new file mode 100644
index 00000000..2e7f02ad
--- /dev/null
+++ b/tests/HttpClient/GuzzleHttpClientFactoryTest.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\Tests\HttpClient;
+
+use OpenSearch\HttpClient\GuzzleHttpClientFactory;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Client\ClientInterface;
+
+/**
+ * Test the Guzzle HTTP client factory.
+ *
+ * @coversDefaultClass \OpenSearch\HttpClient\GuzzleHttpClientFactory
+ */
+class GuzzleHttpClientFactoryTest extends TestCase
+{
+    public function testCreate()
+    {
+        $factory = new GuzzleHttpClientFactory(2);
+        $client = $factory->create([
+            'base_uri' => 'http://example.com',
+            'verify' => true,
+            'auth' => ['username', 'password'],
+        ]);
+
+        $this->assertInstanceOf(ClientInterface::class, $client);
+    }
+}
diff --git a/tests/HttpClient/GuzzleRetryDeciderTest.php b/tests/HttpClient/GuzzleRetryDeciderTest.php
new file mode 100644
index 00000000..7d827f75
--- /dev/null
+++ b/tests/HttpClient/GuzzleRetryDeciderTest.php
@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\Tests\HttpClient;
+
+use ColinODell\PsrTestLogger\TestLogger;
+use GuzzleHttp\Exception\ConnectException;
+use OpenSearch\HttpClient\GuzzleRetryDecider;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+
+/**
+ * Test the Guzzle retry decider.
+ *
+ * @coversDefaultClass \OpenSearch\HttpClient\GuzzleRetryDecider
+ */
+class GuzzleRetryDeciderTest extends TestCase
+{
+    /**
+     * @covers ::__invoke
+     */
+    public function testMaxRetriesDoesNotRetry(): void
+    {
+        $decider = new GuzzleRetryDecider(2);
+        $this->assertFalse($decider(2, null, null, null));
+    }
+
+    public function test500orNoExceptionDoesNotRetry(): void
+    {
+        $decider = new GuzzleRetryDecider(2);
+        $this->assertFalse($decider(0, null, null, null));
+    }
+
+    /**
+     * @covers ::__invoke
+     */
+    public function testConnectExceptionRetries(): void
+    {
+        $logger = new TestLogger();
+        $decider = new GuzzleRetryDecider(2, $logger);
+        $this->assertTrue($decider(0, null, null, new ConnectException('Error', $this->createMock(RequestInterface::class))));
+        $this->assertTrue($logger->hasWarning([
+            'level' => 'warning',
+            'message' => 'Retrying request {retries} of {maxRetries}: {exception}',
+            'context' => [
+                'retries' => 0,
+                'maxRetries' => 2,
+                'exception' => 'Error',
+            ],
+        ]));
+    }
+
+    public function testStatus500Retries(): void
+    {
+        $logger = new TestLogger();
+        $decider = new GuzzleRetryDecider(2, $logger);
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(500);
+
+        $this->assertTrue($decider(0, null, $response, null));
+        $this->assertTrue($logger->hasWarning([
+            'level' => 'warning',
+            'message' => 'Retrying request {retries} of {maxRetries}: Status code {status}',
+            'context' => [
+                'retries' => 0,
+                'maxRetries' => 2,
+                'status' => 500,
+            ],
+        ]));
+    }
+}
diff --git a/tests/HttpClient/SymfonyHttpClientFactoryTest.php b/tests/HttpClient/SymfonyHttpClientFactoryTest.php
new file mode 100644
index 00000000..1724f6c8
--- /dev/null
+++ b/tests/HttpClient/SymfonyHttpClientFactoryTest.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenSearch\Tests\HttpClient;
+
+use OpenSearch\HttpClient\SymfonyHttpClientFactory;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Client\ClientInterface;
+
+/**
+ * Test the Symfony HTTP client factory.
+ *
+ * @coversDefaultClass \OpenSearch\HttpClient\SymfonyHttpClientFactory
+ */
+class SymfonyHttpClientFactoryTest extends TestCase
+{
+    public function testCreate()
+    {
+        $factory = new SymfonyHttpClientFactory(2);
+        $client = $factory->create([
+            'base_uri' => 'http://example.com',
+            'verify_peer' => false,
+            'auth_basic' => ['username', 'password'],
+        ]);
+
+        $this->assertInstanceOf(ClientInterface::class, $client);
+    }
+}