diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a515e63..12c7fadb 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/OpenSearch/Client.php b/src/OpenSearch/Client.php index 36cf4406..1977f2d4 100644 --- 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 index 0762d648..748c5421 100644 --- 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 index a37e2176..6facdd28 100644 --- 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 index f76602ac..b04d25fb 100644 --- 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 index 968b3778..c0fd1fd8 100644 --- 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/Connections/ConnectionTest.php b/tests/Connections/ConnectionTest.php index f3737477..e4235093 100644 --- a/tests/Connections/ConnectionTest.php +++ b/tests/Connections/ConnectionTest.php @@ -42,6 +42,7 @@ /** * @covers \OpenSearch\Connections\Connection * @group legacy + * @group Integration * * @deprecated in 2.4.0 and will be removed in 3.0.0. */ diff --git a/tests/GuzzleClientFactoryTest.php b/tests/GuzzleClientFactoryTest.php index 6cc6bc08..f054d0d0 100644 --- a/tests/GuzzleClientFactoryTest.php +++ b/tests/GuzzleClientFactoryTest.php @@ -10,6 +10,8 @@ /** * @coversDefaultClass \OpenSearch\GuzzleClientFactory + * + * @group integration */ class GuzzleClientFactoryTest extends TestCase { diff --git a/tests/HttpTransportTest.php b/tests/HttpTransportTest.php index 6c566bb1..98f6d0fa 100644 --- 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 index f5315dc3..acd907b0 100644 --- a/tests/SymfonyClientFactoryTest.php +++ b/tests/SymfonyClientFactoryTest.php @@ -10,6 +10,7 @@ /** * @coversDefaultClass \OpenSearch\SymfonyClientFactory + * @group Integration */ class SymfonyClientFactoryTest extends TestCase { @@ -21,11 +22,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 index f38a848a..56e06071 100644 --- 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);