Fix legacy options leaking into request headers

CHANGELOG.md

@@ -10,6 +10,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Removed
 ### Fixed
 - Fixed mismatch in return types between `Client::performRequest()` and `Transport::sendRequest()` ([#307](https://github.com/opensearch-project/opensearch-php/issues/307))
+- Fixed legacy client options being passed as headers ([#301](https://github.com/opensearch-project/opensearch-php/issues/301))
 ### Security
 ### Updated APIs
 - Updated opensearch-php APIs to reflect [opensearch-api-specification@5ed668d](https://github.com/opensearch-project/opensearch-api-specification/commit/5ed668d81b34ae90c22a605755fe1c340f38c27d)

src/OpenSearch/Client.php

@@ -783,7 +783,8 @@ public function exists(array $params = []): bool
         $id = $this->extractArgument($params, 'id');
         $index = $this->extractArgument($params, 'index');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(Exists::class);
@@ -822,7 +823,8 @@ public function existsSource(array $params = []): bool
         $id = $this->extractArgument($params, 'id');
         $index = $this->extractArgument($params, 'index');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsSource::class);
@@ -1281,7 +1283,8 @@ public function mtermvectors(array $params = [])
      */
     public function ping(array $params = []): bool
     {
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(Ping::class);

src/OpenSearch/HttpTransport.php

@@ -37,6 +37,12 @@ public function sendRequest(
         mixed $body = null,
         array $headers = [],
     ): iterable|string|null {
+        // @todo Remove support for legacy options in 3.0.0.
+        // @phpstan-ignore isset.offset
+        if (isset($headers['client']['headers'])) {
+            $headers = array_merge($headers, $headers['client']['headers']);
+        }
+        unset($headers['client']);
         $request = $this->createRequest($method, $uri, $params, $body, $headers);
         $response = $this->client->sendRequest($request);
         $statusCode = $response->getStatusCode();

src/OpenSearch/Namespaces/ClusterNamespace.php

@@ -187,7 +187,8 @@ public function existsComponentTemplate(array $params = []): bool
     {
         $name = $this->extractArgument($params, 'name');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsComponentTemplate::class);

src/OpenSearch/Namespaces/IndicesNamespace.php

@@ -481,7 +481,8 @@ public function exists(array $params = []): bool
     {
         $index = $this->extractArgument($params, 'index');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(Exists::class);
@@ -514,7 +515,8 @@ public function existsAlias(array $params = []): bool
         $name = $this->extractArgument($params, 'name');
         $index = $this->extractArgument($params, 'index');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsAlias::class);
@@ -546,7 +548,8 @@ public function existsIndexTemplate(array $params = []): bool
     {
         $name = $this->extractArgument($params, 'name');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsIndexTemplate::class);
@@ -577,7 +580,8 @@ public function existsTemplate(array $params = []): bool
     {
         $name = $this->extractArgument($params, 'name');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsTemplate::class);

src/OpenSearch/Namespaces/IsmNamespace.php

@@ -128,7 +128,8 @@ public function existsPolicy(array $params = []): bool
     {
         $policy_id = $this->extractArgument($params, 'policy_id');
 
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(ExistsPolicy::class);

tests/HttpTransportTest.php

@@ -50,8 +50,53 @@ public function testHttpTransport(): void
         $serializer = new SmartSerializer();
 
         $transport = new HttpTransport($client, $requestFactory, $serializer);
-        $response = $transport->sendRequest('GET', '/');
+        // Check legacy options are removed.
+        // @phpstan-ignore argument.type
+        $response = $transport->sendRequest('GET', '/', [], null, ['client' => ['foo' => 'bar']]);
 
         $this->assertEquals(['foo' => 'bar'], $response);
     }
+
+    /**
+     * @covers ::sendRequest
+     */
+    public function testHttpTransportWithLegacyHeaders(): void
+    {
+        $request = $this->createMock(RequestInterface::class);
+
+        $requestFactory = $this->createMock(RequestFactoryInterface::class);
+        $requestFactory->expects($this->once())
+            ->method('createRequest')
+            ->with(
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->identicalTo(['foo' => 'bar']),
+            )
+            ->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);
+        // Check legacy headers are merged.
+        // @phpstan-ignore argument.type
+        $response = $transport->sendRequest('GET', '/', [], null, ['client' => ['headers' => ['foo' => 'bar']]]);
+    }
 }

tests/SymfonyClientFactoryTest.php

@@ -28,4 +28,20 @@ public function testCreate(): void
 
         $this->assertInstanceOf(Client::class, $client);
     }
+
+    /**
+     * @covers ::__construct
+     */
+    public function testLegacyOptions(): void
+    {
+        $factory = new SymfonyClientFactory();
+        $client = $factory->create([
+            'base_uri' => 'https://localhost:9200',
+            'auth_basic' => ['admin', 'password'],
+            'verify_peer' => false,
+        ]);
+
+        $exists = $client->indices()->exists(['index' => 'test']);
+        $this->assertFalse($exists);
+    }
 }

util/template/endpoint-function-bool

@@ -2,7 +2,8 @@
     public function :endpoint(array $params = []): bool
     {
 :extract
-        // manually make this verbose so we can check status code
+        // Legacy option to manually make this verbose so we can check status code.
+        // @todo remove in 3.0.0
         $params['client']['verbose'] = true;
 
         $endpoint = $this->endpointFactory->getEndpoint(:EndpointClass::class);