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

Improve client builder and options #728

Merged
merged 9 commits into from
Dec 19, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use Sentry\ClientBuilder;
require 'vendor/autoload.php';

// Instantiate the SDK with your DSN
$client = ClientBuilder::create(['server' => 'http://public@example.com/1'])->getClient();
$client = ClientBuilder::create(new Options(['server' => 'http://public@example.com/1']))->getClient();
Jean85 marked this conversation as resolved.
Show resolved Hide resolved

// Capture an exception
$eventId = $client->captureException(new \RuntimeException('Hello World!'));
Expand Down
2 changes: 1 addition & 1 deletion UPGRADE-2.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@

// or

$client = ClientBuilder::create([...])->getClient();
$client = ClientBuilder::create(new Options([...]))->getClient();
Jean85 marked this conversation as resolved.
Show resolved Hide resolved
```

### Processors
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ parameters:
- src
- tests
ignoreErrors:
- '/Call to an undefined method Sentry\\ClientBuilder::methodThatDoesNotExists\(\)/'
- '/Method Sentry\\ClientBuilder::\w+\(\) should return \$this\(Sentry\\ClientBuilderInterface\) but returns \$this\(Sentry\\ClientBuilder\)/'
- '/Argument of an invalid type object supplied for foreach, only iterables are supported/'
- '/Binary operation "\*" between array and 2 results in an error\./'
- '/Method Sentry\\Serializer\\RepresentationSerializer::(representationSerialize|serializeValue)\(\) should return [\w|]+ but returns [\w|]+/'
Expand Down
91 changes: 23 additions & 68 deletions src/ClientBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,40 +34,6 @@
* The default implementation of {@link ClientBuilderInterface}.
*
* @author Stefano Arlandini <sarlandini@alice.it>
*
* @method int getSendAttempts()
* @method setSendAttempts(int $attemptsCount)
* @method string[] getPrefixes()
* @method setPrefixes(array $prefixes)
* @method float getSampleRate()
* @method setSampleRate(float $sampleRate)
* @method bool shouldAttachStacktrace()
* @method setAttachStacktrace(bool $enable)
* @method int getContextLines()
* @method setContextLines(int $contextLines)
* @method null|string getEnvironment()
* @method setEnvironment(null|string $environment)
* @method string[] getExcludedProjectPaths()
* @method setExcludedProjectPaths(string[] $paths)
* @method setExcludedLoggers(string[] $loggers)
* @method string[] getExcludedExceptions()
* @method string getProjectRoot()
* @method setProjectRoot(string $path)
* @method string getLogger()
* @method setLogger(string $logger)
* @method string getRelease()
* @method setRelease(string $release)
* @method string getDsn()
* @method string getServerName()
* @method setServerName(string $serverName)
* @method string[] getTags()
* @method setTags(string[] $tags)
* @method bool shouldSendDefaultPii()
* @method setSendDefaultPii(bool $enable)
* @method bool hasDefaultIntegrations()
* @method setDefaultIntegrations(bool $enable)
* @method callable getBeforeSendCallback()
* @method setBeforeSendCallback(callable $beforeSend)
*/
final class ClientBuilder implements ClientBuilderInterface
{
Expand Down Expand Up @@ -124,11 +90,11 @@ final class ClientBuilder implements ClientBuilderInterface
/**
* Class constructor.
*
* @param array $options The client options
* @param Options $options The client options
Jean85 marked this conversation as resolved.
Show resolved Hide resolved
*/
public function __construct(array $options = [])
public function __construct(Options $options = null)
Jean85 marked this conversation as resolved.
Show resolved Hide resolved
{
$this->options = new Options($options);
$this->options = $options ?? new Options();

if ($this->options->hasDefaultIntegrations()) {
$this->options->setIntegrations(\array_merge([
Expand All @@ -141,15 +107,23 @@ public function __construct(array $options = [])
/**
* {@inheritdoc}
*/
public static function create(array $options = []): self
public static function create(Options $options = null): ClientBuilderInterface
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
{
return new static($options);
}

/**
* {@inheritdoc}
*/
public function setUriFactory(UriFactory $uriFactory): self
public function getOptions(): Options
{
return $this->options;
}

/**
* {@inheritdoc}
*/
public function setUriFactory(UriFactory $uriFactory): ClientBuilderInterface
{
$this->uriFactory = $uriFactory;

Expand All @@ -159,7 +133,7 @@ public function setUriFactory(UriFactory $uriFactory): self
/**
* {@inheritdoc}
*/
public function setMessageFactory(MessageFactory $messageFactory): self
public function setMessageFactory(MessageFactory $messageFactory): ClientBuilderInterface
{
$this->messageFactory = $messageFactory;

Expand All @@ -169,7 +143,7 @@ public function setMessageFactory(MessageFactory $messageFactory): self
/**
* {@inheritdoc}
*/
public function setTransport(TransportInterface $transport): self
public function setTransport(TransportInterface $transport): ClientBuilderInterface
{
$this->transport = $transport;

Expand All @@ -179,7 +153,7 @@ public function setTransport(TransportInterface $transport): self
/**
* {@inheritdoc}
*/
public function setHttpClient(HttpAsyncClient $httpClient): self
public function setHttpClient(HttpAsyncClient $httpClient): ClientBuilderInterface
{
$this->httpClient = $httpClient;

Expand All @@ -189,7 +163,7 @@ public function setHttpClient(HttpAsyncClient $httpClient): self
/**
* {@inheritdoc}
*/
public function addHttpClientPlugin(Plugin $plugin): self
public function addHttpClientPlugin(Plugin $plugin): ClientBuilderInterface
{
$this->httpClientPlugins[] = $plugin;

Expand All @@ -199,7 +173,7 @@ public function addHttpClientPlugin(Plugin $plugin): self
/**
* {@inheritdoc}
*/
public function removeHttpClientPlugin(string $className): self
public function removeHttpClientPlugin(string $className): ClientBuilderInterface
{
foreach ($this->httpClientPlugins as $index => $httpClientPlugin) {
if (!$httpClientPlugin instanceof $className) {
Expand All @@ -215,7 +189,7 @@ public function removeHttpClientPlugin(string $className): self
/**
* {@inheritdoc}
*/
public function setSerializer(SerializerInterface $serializer): self
public function setSerializer(SerializerInterface $serializer): ClientBuilderInterface
{
$this->serializer = $serializer;

Expand All @@ -225,7 +199,7 @@ public function setSerializer(SerializerInterface $serializer): self
/**
* {@inheritdoc}
*/
public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): self
public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): ClientBuilderInterface
{
$this->representationSerializer = $representationSerializer;

Expand All @@ -235,7 +209,7 @@ public function setRepresentationSerializer(RepresentationSerializerInterface $r
/**
* {@inheritdoc}
*/
public function setSdkIdentifier(string $sdkIdentifier): self
public function setSdkIdentifier(string $sdkIdentifier): ClientBuilderInterface
{
$this->sdkIdentifier = $sdkIdentifier;

Expand All @@ -259,7 +233,7 @@ private function getSdkVersion(): string
/**
* {@inheritdoc}
*/
public function setSdkVersion(string $sdkVersion): self
public function setSdkVersion(string $sdkVersion): ClientBuilderInterface
{
$this->sdkVersion = $sdkVersion;

Expand All @@ -273,7 +247,7 @@ public function setSdkVersion(string $sdkVersion): self
*
* @return $this
*/
public function setSdkVersionByPackageName(string $packageName): self
public function setSdkVersionByPackageName(string $packageName): ClientBuilderInterface
{
$this->sdkVersion = PrettyVersions::getVersion($packageName)->getPrettyVersion();

Expand All @@ -293,25 +267,6 @@ public function getClient(): ClientInterface
return new Client($this->options, $this->transport, $this->createEventFactory());
}

/**
* This method forwards all methods calls to the options object.
*
* @param string $name The name of the method being called
* @param array $arguments Parameters passed to the $name'ed method
*
* @return $this
*
* @throws \BadMethodCallException If the called method does not exists
*/
public function __call($name, $arguments)
{
if (!method_exists($this->options, $name)) {
throw new \BadMethodCallException(sprintf('The method named "%s" does not exists.', $name));
}

return $this->options->$name(...$arguments);
}

/**
* Creates a new instance of the HTTP client.
*
Expand Down
31 changes: 19 additions & 12 deletions src/ClientBuilderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,18 @@ interface ClientBuilderInterface
/**
* Creates a new instance of this builder.
*
* @param array $options The client options
* @param Options $options The client options; of nothing is passed, the default values are assumed
*
* @return static
*/
public static function create(array $options = []);
public static function create(Options $options = null): self;
ste93cry marked this conversation as resolved.
Show resolved Hide resolved

/**
* The options that will be used to create the {@see Client}.
*
* @return Options
*/
public function getOptions(): Options;

/**
* Sets the factory to use to create URIs.
Expand All @@ -35,7 +42,7 @@ public static function create(array $options = []);
*
* @return $this
*/
public function setUriFactory(UriFactory $uriFactory);
public function setUriFactory(UriFactory $uriFactory): self;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explicitly avoided to use self in the interfaces because PHP doesn't have covariant returns and this forces anyone implementing this interface to define that the methods returns the interface and not the type of the class itself which can cause issues with fluent interfaces like this one. Please revert everything back


/**
* Sets the factory to use to create PSR-7 messages.
Expand All @@ -44,7 +51,7 @@ public function setUriFactory(UriFactory $uriFactory);
*
* @return $this
*/
public function setMessageFactory(MessageFactory $messageFactory);
public function setMessageFactory(MessageFactory $messageFactory): self;

/**
* Sets the transport that will be used to send events.
Expand All @@ -53,7 +60,7 @@ public function setMessageFactory(MessageFactory $messageFactory);
*
* @return $this
*/
public function setTransport(TransportInterface $transport);
public function setTransport(TransportInterface $transport): self;

/**
* Sets the HTTP client.
Expand All @@ -62,7 +69,7 @@ public function setTransport(TransportInterface $transport);
*
* @return $this
*/
public function setHttpClient(HttpAsyncClient $httpClient);
public function setHttpClient(HttpAsyncClient $httpClient): self;

/**
* Adds a new HTTP client plugin to the end of the plugins chain.
Expand All @@ -71,7 +78,7 @@ public function setHttpClient(HttpAsyncClient $httpClient);
*
* @return $this
*/
public function addHttpClientPlugin(Plugin $plugin);
public function addHttpClientPlugin(Plugin $plugin): self;

/**
* Removes a HTTP client plugin by its fully qualified class name (FQCN).
Expand All @@ -80,7 +87,7 @@ public function addHttpClientPlugin(Plugin $plugin);
*
* @return $this
*/
public function removeHttpClientPlugin(string $className);
public function removeHttpClientPlugin(string $className): self;

/**
* Gets the instance of the client built using the configured options.
Expand All @@ -96,7 +103,7 @@ public function getClient(): ClientInterface;
*
* @return $this
*/
public function setSerializer(SerializerInterface $serializer);
public function setSerializer(SerializerInterface $serializer): self;

/**
* Sets a representation serializer instance to be injected as a dependency of the client.
Expand All @@ -107,7 +114,7 @@ public function setSerializer(SerializerInterface $serializer);
*
* @return $this
*/
public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer);
public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): self;

/**
* Sets the SDK identifier to be passed onto {@see Event} and HTTP User-Agent header.
Expand All @@ -118,7 +125,7 @@ public function setRepresentationSerializer(RepresentationSerializerInterface $r
*
* @internal
*/
public function setSdkIdentifier(string $sdkIdentifier);
public function setSdkIdentifier(string $sdkIdentifier): self;

/**
* Sets the SDK version to be passed onto {@see Event} and HTTP User-Agent header.
Expand All @@ -129,5 +136,5 @@ public function setSdkIdentifier(string $sdkIdentifier);
*
* @internal
*/
public function setSdkVersion(string $sdkVersion);
public function setSdkVersion(string $sdkVersion): self;
}
14 changes: 13 additions & 1 deletion src/Options.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Options
/**
* @var array The configuration options
*/
private $options = [];
private $options;

/**
* @var string|null A simple server string, set to the DSN found on your Sentry settings
Expand Down Expand Up @@ -393,6 +393,18 @@ public function getDsn(): ?string
return $this->dsn;
}

/**
* Sets the DSN of the Sentry server the authenticated user is bound to.
*
* @param string $dsn
*/
public function setDsn(string $dsn): void
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
{
$options = array_merge($this->options, ['dsn' => $dsn]);

$this->options = $this->resolver->resolve($options);
}

/**
* Gets the name of the server the SDK is running on (e.g. the hostname).
*
Expand Down
3 changes: 2 additions & 1 deletion src/Sdk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
*/
function init(array $options = []): void
{
Hub::setCurrent(new Hub(ClientBuilder::create($options)->getClient()));
$client = ClientBuilder::create(new Options($options))->getClient();
Hub::setCurrent(new Hub($client));
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Loading