Fix legacy options leaking into request headers

Signed-off-by: Kim Pepper <[email protected]>
opensearch-project · Feb 24, 2025 · e1e9c62 · e1e9c62
1 parent c9b50b2
commit e1e9c62
Show file tree

Hide file tree

Showing 8 changed files with 89 additions and 12 deletions.
diff --git a/src/OpenSearch/Client.php b/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);

diff --git a/src/OpenSearch/HttpTransport.php b/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();

diff --git a/src/OpenSearch/Namespaces/ClusterNamespace.php b/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);

diff --git a/src/OpenSearch/Namespaces/IndicesNamespace.php b/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);

diff --git a/src/OpenSearch/Namespaces/IsmNamespace.php b/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);

diff --git a/tests/HttpTransportTest.php b/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']]]);
+    }
 }
diff --git a/tests/SymfonyClientFactoryTest.php b/tests/SymfonyClientFactoryTest.php
@@ -21,11 +21,27 @@ public function testCreate(): void
     {
         $factory = new SymfonyClientFactory();
         $client = $factory->create([
-            'base_uri' => 'https://localhost:9200',
+            'base_uri' => 'http://localhost:9200',
             'auth_basic' => ['admin', 'password'],
             'verify_peer' => false,
         ]);
 
         $this->assertInstanceOf(Client::class, $client);
     }
+
+    /**
+     * @covers ::__construct
+     */
+    public function testLegacyOptions(): void
+    {
+        $factory = new SymfonyClientFactory();
+        $client = $factory->create([
+            'base_uri' => 'http://localhost:9200',
+            'auth_basic' => ['admin', 'password'],
+            'verify_peer' => false,
+        ]);
+
+        $exists = $client->indices()->exists(['index' => 'test']);
+        $this->assertFalse($exists);
+    }
 }
diff --git a/util/template/endpoint-function-bool b/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);