Skip to content

Commit

Permalink
Lazy initialize default values in client builder (#1699)
Browse files Browse the repository at this point in the history
  • Loading branch information
stayallive authored Feb 8, 2024
1 parent 28fac24 commit 6f916e9
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 22 deletions.
16 changes: 16 additions & 0 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,22 @@ public function getStacktraceBuilder(): StacktraceBuilder
return $this->stacktraceBuilder;
}

/**
* @internal
*/
public function getLogger(): LoggerInterface
{
return $this->logger;
}

/**
* @internal
*/
public function getTransport(): TransportInterface
{
return $this->transport;
}

/**
* Assembles an event and prepares it to be sent of to Sentry.
*
Expand Down
33 changes: 16 additions & 17 deletions src/ClientBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ final class ClientBuilder
private $options;

/**
* @var TransportInterface The transport
* @var TransportInterface|null The transport
*/
private $transport;

/**
* @var HttpClientInterface The HTTP client
* @var HttpClientInterface|null The HTTP client
*/
private $httpClient;

Expand Down Expand Up @@ -60,16 +60,6 @@ final class ClientBuilder
public function __construct(?Options $options = null)
{
$this->options = $options ?? new Options();

$this->logger = $this->options->getLogger() ?? null;

$this->httpClient = $this->options->getHttpClient() ?? new HttpClient($this->sdkIdentifier, $this->sdkVersion);
$this->transport = $this->options->getTransport() ?? new HttpTransport(
$this->options,
$this->httpClient,
new PayloadSerializer($this->options),
$this->logger
);
}

/**
Expand All @@ -94,7 +84,7 @@ public function setRepresentationSerializer(RepresentationSerializerInterface $r

public function getLogger(): ?LoggerInterface
{
return $this->logger;
return $this->logger ?? $this->options->getLogger();
}

public function setLogger(LoggerInterface $logger): self
Expand All @@ -120,7 +110,14 @@ public function setSdkVersion(string $sdkVersion): self

public function getTransport(): TransportInterface
{
return $this->transport;
return $this->transport
?? $this->options->getTransport()
?? new HttpTransport(
$this->options,
$this->getHttpClient(),
new PayloadSerializer($this->options),
$this->getLogger()
);
}

public function setTransport(TransportInterface $transport): self
Expand All @@ -132,7 +129,9 @@ public function setTransport(TransportInterface $transport): self

public function getHttpClient(): HttpClientInterface
{
return $this->httpClient;
return $this->httpClient
?? $this->options->getHttpClient()
?? new HttpClient($this->sdkIdentifier, $this->sdkVersion);
}

public function setHttpClient(HttpClientInterface $httpClient): self
Expand All @@ -146,11 +145,11 @@ public function getClient(): ClientInterface
{
return new Client(
$this->options,
$this->transport,
$this->getTransport(),
$this->sdkIdentifier,
$this->sdkVersion,
$this->representationSerializer,
$this->logger
$this->getLogger()
);
}
}
8 changes: 8 additions & 0 deletions src/Transport/HttpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ public function close(?int $timeout = null): Result
return new Result(ResultStatus::success());
}

/**
* @internal
*/
public function getHttpClient(): HttpClientInterface
{
return $this->httpClient;
}

private function sendRequestToSpotlight(Event $event): void
{
if (!$this->options->isSpotlightEnabled()) {
Expand Down
98 changes: 93 additions & 5 deletions tests/ClientBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace Sentry\Tests;

use PHPUnit\Framework\TestCase;
use Psr\Log\AbstractLogger;
use Psr\Log\LoggerInterface;
use Sentry\Client;
use Sentry\ClientBuilder;
use Sentry\Event;
Expand Down Expand Up @@ -79,7 +81,7 @@ public function testCreateWithNoOptionsIsTheSameAsDefaultOptions(): void
);
}

public function testDefaultHttpClientAndTransport()
public function testDefaultHttpClientAndTransport(): void
{
$options = new Options();
$clientBuilder = new ClientBuilder($options);
Expand All @@ -88,7 +90,58 @@ public function testDefaultHttpClientAndTransport()
$this->assertInstanceOf(HttpTransport::class, $clientBuilder->getTransport());
}

public function testSettingCustomHttpClinet()
public function testSettingCustomLogger(): void
{
$logger = new CustomLogger();

$clientBuilder = new ClientBuilder();
$clientBuilder->setLogger($logger);

$this->assertSame($logger, $clientBuilder->getLogger());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);
$this->assertSame($logger, $client->getLogger());
}

public function testSettingCustomLoggerFromOptions(): void
{
$logger = new CustomLogger();

$options = new Options([
'logger' => $logger,
]);
$clientBuilder = new ClientBuilder($options);

$this->assertSame($logger, $clientBuilder->getLogger());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);
$this->assertSame($logger, $client->getLogger());
}

public function testSettingCustomHttpClient(): void
{
$httpClient = new CustomHttpClient();

$clientBuilder = new ClientBuilder();
$clientBuilder->setHttpClient($httpClient);

$this->assertSame($httpClient, $clientBuilder->getHttpClient());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);

$transport = $client->getTransport();

$this->assertInstanceOf(HttpTransport::class, $transport);
$this->assertSame($httpClient, $transport->getHttpClient());
}

public function testSettingCustomHttpClientFromOptions(): void
{
$httpClient = new CustomHttpClient();

Expand All @@ -98,10 +151,33 @@ public function testSettingCustomHttpClinet()
$clientBuilder = new ClientBuilder($options);

$this->assertSame($httpClient, $clientBuilder->getHttpClient());
$this->assertInstanceOf(HttpTransport::class, $clientBuilder->getTransport());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);

$transport = $client->getTransport();

$this->assertInstanceOf(HttpTransport::class, $transport);
$this->assertSame($httpClient, $transport->getHttpClient());
}

public function testSettingCustomTransport()
public function testSettingCustomTransport(): void
{
$transport = new CustomTransport();

$clientBuilder = new ClientBuilder();
$clientBuilder->setTransport($transport);

$this->assertSame($transport, $clientBuilder->getTransport());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);
$this->assertSame($transport, $client->getTransport());
}

public function testSettingCustomTransportFromOptions(): void
{
$transport = new CustomTransport();

Expand All @@ -110,8 +186,12 @@ public function testSettingCustomTransport()
]);
$clientBuilder = new ClientBuilder($options);

$this->assertInstanceOf(HttpClient::class, $clientBuilder->getHttpClient());
$this->assertSame($transport, $clientBuilder->getTransport());

$client = $clientBuilder->getClient();

$this->assertInstanceOf(Client::class, $client);
$this->assertSame($transport, $client->getTransport());
}
}

Expand Down Expand Up @@ -142,3 +222,11 @@ public function close(?int $timeout = null): Result
return new Result(ResultStatus::success());
}
}

final class CustomLogger extends AbstractLogger implements LoggerInterface
{
public function log($level, $message, array $context = []): void
{
// noop
}
}

0 comments on commit 6f916e9

Please sign in to comment.