Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HTTP client handling and configuration in Typesense PHP client #78

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.idea
.tmp
.phpunit.result.cache
.env
/composer.lock
phpunit.xml
vendor
33 changes: 24 additions & 9 deletions src/Lib/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Typesense\Lib;

use Http\Client\Common\HttpMethodsClient;
use Http\Client\HttpClient;
use Http\Discovery\Psr17FactoryDiscovery;
use Http\Discovery\Psr18ClientDiscovery;
use Monolog\Handler\StreamHandler;
Expand Down Expand Up @@ -56,9 +57,9 @@ class Configuration
private LoggerInterface $logger;

/**
* @var null|ClientInterface
* @var HttpMethodsClient|ClientInterface|null
*/
private ?ClientInterface $client = null;
private $client = null;

/**
* @var int
Expand Down Expand Up @@ -103,8 +104,18 @@ public function __construct(array $config)
$this->logger = new Logger('typesense');
$this->logger->pushHandler(new StreamHandler('php://stdout', $this->logLevel));

if (true === \array_key_exists('client', $config) && $config['client'] instanceof ClientInterface) {
$this->client = $config['client'];
if (isset($config['client'])) {
if ($config['client'] instanceof HttpMethodsClient || $config['client'] instanceof ClientInterface) {
$this->client = $config['client'];
} elseif ($config['client'] instanceof HttpClient) {
$this->client = new HttpMethodsClient(
$config['client'],
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory()
);
} else {
throw new ConfigError('Client must implement PSR-18 ClientInterface or Http\Client\HttpClient');
}
}
}

Expand Down Expand Up @@ -216,10 +227,14 @@ public function getLogger(): LoggerInterface
*/
public function getClient(): ClientInterface
{
return new HttpMethodsClient(
$this->client ?? Psr18ClientDiscovery::find(),
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory(),
);
if ($this->client === null) {
$discoveredClient = Psr18ClientDiscovery::find();
$this->client = new HttpMethodsClient(
$discoveredClient,
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory()
);
}
return $this->client;
}
}
84 changes: 84 additions & 0 deletions tests/Feature/HttpClientsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<?php

namespace Feature;

use Tests\TestCase;
use Http\Client\Common\HttpMethodsClient;
use Http\Discovery\Psr17FactoryDiscovery;
use Http\Discovery\Psr18ClientDiscovery;
use Typesense\Exceptions\ConfigError;
use Symfony\Component\HttpClient\Psr18Client;
use Typesense\Client;
use \stdClass;

class HttpClientsTest extends TestCase
{
private array $baseConfig;

protected function setUp(): void
{
parent::setUp();
$this->baseConfig = [
'api_key' => $_ENV['TYPESENSE_API_KEY'],
'nodes' => [[
'host' => $_ENV['TYPESENSE_NODE_HOST'],
'port' => $_ENV['TYPESENSE_NODE_PORT'],
'protocol' => $_ENV['TYPESENSE_NODE_PROTOCOL']
]]
];
}

public function testWorksWithDefaultClient(): void
{
$client = new Client($this->baseConfig);
$response = $client->health->retrieve();
$this->assertIsBool($response['ok']);
}

public function testWorksWithPsr18Client(): void
{
$httpClient = new Psr18Client();
$wrappedClient = new HttpMethodsClient(
$httpClient,
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory()
);

$config = array_merge($this->baseConfig, ['client' => $wrappedClient]);
$client = new Client($config);
$response = $client->health->retrieve();
$this->assertIsBool($response['ok']);
}

public function testWorksWithHttpMethodsClient(): void
{
$httpClient = new HttpMethodsClient(
Psr18ClientDiscovery::find(),
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory()
);

$config = array_merge($this->baseConfig, ['client' => $httpClient]);

$client = new Client($config);
$response = $client->health->retrieve();
$this->assertIsBool($response['ok']);
}

public function testWorksWithLegacyPsr18Client(): void
{
$httpClient = $this->createMock(\Psr\Http\Client\ClientInterface::class);
$config = array_merge($this->baseConfig, ['client' => $httpClient]);
$client = new Client($config);
$this->assertInstanceOf(Client::class, $client);
}

public function testRejectsInvalidClient(): void
{
$this->expectException(ConfigError::class);
$this->expectExceptionMessage('Client must implement PSR-18 ClientInterface or Http\Client\HttpClient');

$config = array_merge($this->baseConfig, ['client' => new stdClass()]);
new Client($config);
}
}