Update abstract namespace to use new transport. Add tests

Signed-off-by: Kim Pepper <[email protected]>
opensearch-project · Dec 16, 2024 · f3089fb · f3089fb
1 parent a4a02c9
commit f3089fb
Show file tree

Hide file tree

Showing 8 changed files with 116 additions and 22 deletions.
diff --git a/src/OpenSearch/Namespaces/AbstractNamespace.php b/src/OpenSearch/Namespaces/AbstractNamespace.php
@@ -21,22 +21,24 @@
 
 namespace OpenSearch\Namespaces;
 
-use Http\Promise\Promise;
 use OpenSearch\EndpointFactoryInterface;
-use OpenSearch\EndpointInterface;
 use OpenSearch\Endpoints\AbstractEndpoint;
 use OpenSearch\LegacyEndpointFactory;
+use OpenSearch\LegacyTransportWrapper;
 use OpenSearch\Transport;
 use OpenSearch\TransportInterface;
-use Psr\Http\Message\ResponseInterface;
 
 abstract class AbstractNamespace
 {
     /**
      * @var \OpenSearch\Transport
+     *
+     * @deprecated in 2.3.2 and will be removed in 3.0.0. Use $httpTransport property instead.
      */
     protected $transport;
 
+    protected TransportInterface $httpTransport;
+
     protected EndpointFactoryInterface $endpointFactory;
 
     /**
@@ -46,9 +48,15 @@ abstract class AbstractNamespace
      */
     protected $endpoints;
 
-    public function __construct(Transport $transport, callable|EndpointFactoryInterface $endpointFactory)
+    public function __construct(TransportInterface|Transport $transport, callable|EndpointFactoryInterface $endpointFactory)
     {
-        $this->transport = $transport;
+        if (!$transport instanceof TransportInterface) {
+            @trigger_error('Passing an instance of \OpenSearch\Transport to ' . __METHOD__ . '() is deprecated in 2.3.2 and will be removed in 3.0.0. Pass an instance of \OpenSearch\TransportInterface instead.', E_USER_DEPRECATED);
+            $this->transport = $transport;
+            $this->httpTransport = new LegacyTransportWrapper($transport);
+        } else {
+            $this->httpTransport = $transport;
+        }
         if (is_callable($endpointFactory)) {
             @trigger_error('Passing a callable as $endpointFactory param to ' . __METHOD__ . '() is deprecated in 2.3.2 and will be removed in 3.0.0. Pass an instance of \OpenSearch\EndpointFactoryInterface instead.', E_USER_DEPRECATED);
             $endpoints = $endpointFactory;
@@ -79,14 +87,13 @@ public function extractArgument(array &$params, string $arg)
 
     protected function performRequest(AbstractEndpoint $endpoint)
     {
-        $response = $this->transport->performRequest(
+        return $this->httpTransport->sendRequest(
             $endpoint->getMethod(),
             $endpoint->getURI(),
             $endpoint->getParams(),
             $endpoint->getBody(),
             $endpoint->getOptions()
         );
 
-        return $this->transport->resultOrFuture($response, $endpoint->getOptions());
     }
 }
diff --git a/src/OpenSearch/Namespaces/BooleanRequestWrapper.php b/src/OpenSearch/Namespaces/BooleanRequestWrapper.php
@@ -26,17 +26,45 @@
 use OpenSearch\Endpoints\AbstractEndpoint;
 use OpenSearch\Transport;
 use GuzzleHttp\Ring\Future\FutureArrayInterface;
+use OpenSearch\TransportInterface;
+use Psr\Http\Client\ClientExceptionInterface;
 
 abstract class BooleanRequestWrapper
 {
+    /**
+     * Send a request with a boolean response.
+     *
+     * Throwing a client exception will return FALSE, otherwise TRUE is returned.
+     */
+    public static function sendRequest(AbstractEndpoint $endpoint, TransportInterface $transport): bool
+    {
+        try {
+            $transport->sendRequest(
+                $endpoint->getMethod(),
+                $endpoint->getURI(),
+                $endpoint->getParams(),
+                $endpoint->getBody(),
+                $endpoint->getOptions()
+            );
+        } catch (ClientExceptionInterface) {
+            return false;
+        }
+        return true;
+    }
+
     /**
      * Perform Request
      *
      * @throws Missing404Exception
      * @throws RoutingMissingException
+     *
+     * @deprecated in 2.3.2 and will be removed in 3.0.0. Use \OpenSearch\Namespaces\BooleanRequestWrapper::sendRequest() instead.
      */
     public static function performRequest(AbstractEndpoint $endpoint, Transport $transport)
     {
+        @trigger_error(
+            __METHOD__ . '() is deprecated in 2.3.2 and will be removed in 3.0.0. Use \OpenSearch\Namespaces\BooleanRequestWrapper::sendRequest() instead.'
+        );
         try {
             $response = $transport->performRequest(
                 $endpoint->getMethod(),

diff --git a/src/OpenSearch/Namespaces/ClusterNamespace.php b/src/OpenSearch/Namespaces/ClusterNamespace.php
@@ -174,7 +174,7 @@ public function existsComponentTemplate(array $params = []): bool
         $endpoint->setParams($params);
         $endpoint->setName($name);
 
-        return BooleanRequestWrapper::performRequest($endpoint, $this->transport);
+        return BooleanRequestWrapper::sendRequest($endpoint, $this->httpTransport);
     }
 
     /**

diff --git a/src/OpenSearch/TransportInterface.php b/src/OpenSearch/TransportInterface.php
@@ -10,7 +10,7 @@ interface TransportInterface
     /**
      * Create a new request.
      *
-     * @throws \Exception
+     * @throws \Psr\Http\Client\ClientExceptionInterface
      */
     public function sendRequest(
         string $method,

diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -25,6 +25,7 @@
 use OpenSearch\Common\Exceptions\RuntimeException;
 use OpenSearch\EndpointFactoryInterface;
 use OpenSearch\Endpoints\Delete;
+use OpenSearch\Transport;
 use OpenSearch\TransportInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -111,20 +112,17 @@ public function testIdCannotBeNullForDelete()
      */
     public function testSendRawRequest(): void
     {
-        $this->transport->expects($this->once())
-            ->method('createRequest')
-            ->with('GET', '/', ['foo' => 'bar'], 'whizz')
-            ->willReturn($this->createMock(RequestInterface::class));
-
         $this->transport->expects($this->once())
             ->method('sendRequest')
-            ->with($this->isInstanceOf(RequestInterface::class))
-            ->willReturn($this->createMock(ResponseInterface::class));
+            ->with('GET', '/', ['foo' => 'bar'], 'whizz')
+            ->willReturn(['bang']);
 
-        $this->client->request('GET', '/', [
+        $response = $this->client->request('GET', '/', [
             'params' => ['foo' => 'bar'],
             'body' => 'whizz',
         ]);
+
+        $this->assertEquals(['bang'], $response);
     }
 
 }
diff --git a/tests/HttpTransportTest.php b/tests/HttpTransportTest.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace OpenSearch\Tests;
+
+use OpenSearch\HttpTransport;
+use OpenSearch\RequestFactoryInterface;
+use OpenSearch\Serializers\SmartSerializer;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Client\ClientInterface;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\StreamInterface;
+
+/**
+ * Tests the HTTP transport.
+ *
+ * @coversDefaultClass \OpenSearch\HttpTransport
+ */
+class HttpTransportTest extends TestCase
+{
+    /**
+     * @covers ::sendRequest
+     */
+    public function testHttpTransport(): void
+    {
+        $request = $this->createMock(RequestInterface::class);
+
+        $requestFactory = $this->createMock(RequestFactoryInterface::class);
+        $requestFactory->expects($this->once())
+            ->method('createRequest')
+            ->with($this->anything())
+            ->willReturn($request);
+
+        $bodyStream = $this->createMock(StreamInterface::class);
+        $bodyStream->expects($this->once())
+            ->method('getContents')
+            ->willReturn('{"foo":"bar"}');
+
+        $response = $this->createMock(ResponseInterface::class);
+        $response->expects($this->once())
+            ->method('getBody')
+            ->willReturn($bodyStream);
+
+        $client = $this->createMock(ClientInterface::class);
+        $client->expects($this->once())
+            ->method('sendRequest')
+            ->with($request)
+            ->willReturn($response);
+
+        $serializer = new SmartSerializer();
+
+        $transport = new HttpTransport($client, $requestFactory, $serializer);
+        $response = $transport->sendRequest('GET', '/');
+
+        $this->assertEquals(['foo' => 'bar'], $response);
+    }
+}
diff --git a/tests/TransportTest.php → tests/LegacyTransportTest.php b/tests/TransportTest.php → tests/LegacyTransportTest.php
@@ -21,19 +21,23 @@
 
 namespace OpenSearch\Tests;
 
+use GuzzleHttp\Ring\Future\FutureArray;
+use GuzzleHttp\Ring\Future\FutureArrayInterface;
 use OpenSearch\Common\Exceptions\ServerErrorResponseException;
 use OpenSearch\ConnectionPool\AbstractConnectionPool;
 use OpenSearch\Connections\Connection;
-use OpenSearch\Serializers\SerializerInterface;
 use OpenSearch\Transport;
-use GuzzleHttp\Ring\Future\FutureArray;
-use GuzzleHttp\Ring\Future\FutureArrayInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
 use React\Promise\Deferred;
 
-class TransportTest extends TestCase
+/**
+ * Legacy transport test.
+ *
+ * @group legacy
+ */
+class LegacyTransportTest extends TestCase
 {
     /**
      * @var Connection|MockObject

diff --git a/util/template/endpoint-function-bool b/util/template/endpoint-function-bool
@@ -8,5 +8,5 @@
         $endpoint = $this->endpointFactory->getEndpoint(:EndpointClass::class);
         $endpoint->setParams($params);
 :setparam
-        return BooleanRequestWrapper::performRequest($endpoint, $this->transport);
+        return BooleanRequestWrapper::sendRequest($endpoint, $this->httpTransport);
     }