Skip to content

Commit

Permalink
feature #36516 [Notifier] Throw an exception when the Slack DSN is no…
Browse files Browse the repository at this point in the history
…t valid (fabpot)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Notifier] Throw an exception when the Slack DSN is not valid

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes-ish
| New feature?  | yes-ish <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | n/a

Improved errors in case of a DSN issue.
+ proper error for the Slack DSN when path is empty (will help catch when people haven't updated their Slack DSN for 5.1).

Commits
-------

6b1a64a [Notifier] Throw an exception when the Slack DSN is not valid
  • Loading branch information
fabpot committed Apr 21, 2020
2 parents 5a94817 + 6b1a64a commit 2235be0
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 7 deletions.
Expand Up @@ -34,8 +34,8 @@ public function create(Dsn $dsn): TransportInterface
$password = $this->getPassword($dsn);
$phone = $dsn->getOption('phone');

if (null === $phone || '' === $phone) {
throw new IncompleteDsnException('Missing phone.');
if (!$phone) {
throw new IncompleteDsnException('Missing phone.', $dsn->getOriginalDsn());
}

if ('freemobile' === $scheme) {
Expand Down
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Notifier\Bridge\Slack;

use Symfony\Component\Notifier\Exception\IncompleteDsnException;
use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
use Symfony\Component\Notifier\Transport\AbstractTransportFactory;
use Symfony\Component\Notifier\Transport\Dsn;
Expand All @@ -33,6 +34,10 @@ public function create(Dsn $dsn): TransportInterface
$host = 'default' === $dsn->getHost() ? null : $dsn->getHost();
$port = $dsn->getPort();

if (!$id) {
throw new IncompleteDsnException('Missing path (maybe you haven\'t update the DSN when upgrading from 5.0).', $dsn->getOriginalDsn());
}

if ('slack' === $scheme) {
return (new SlackTransport($id, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
}
Expand Down
Expand Up @@ -50,11 +50,11 @@ protected function getSupportedSchemes(): array
private function getToken(Dsn $dsn): string
{
if (null === $dsn->getUser() && null === $dsn->getPassword()) {
throw new IncompleteDsnException('Missing token.');
throw new IncompleteDsnException('Missing token.', $dsn->getOriginalDsn());
}

if (null === $dsn->getPassword()) {
throw new IncompleteDsnException('Malformed token.');
throw new IncompleteDsnException('Malformed token.', $dsn->getOriginalDsn());
}

return sprintf('%s:%s', $dsn->getUser(), $dsn->getPassword());
Expand Down
Expand Up @@ -18,4 +18,20 @@
*/
class IncompleteDsnException extends InvalidArgumentException
{
private $dsn;

public function __construct(string $message, string $dsn = null, ?\Throwable $previous = null)
{
$this->dsn = $dsn;
if ($dsn) {
$message = sprintf('Invalid "%s" notifier DSN: ', $dsn).$message;
}

parent::__construct($message, 0, $previous);
}

public function getOriginalDsn(): string
{
return $this->dsn;
}
}
Expand Up @@ -48,7 +48,7 @@ protected function getUser(Dsn $dsn): string
{
$user = $dsn->getUser();
if (null === $user) {
throw new IncompleteDsnException('User is not set.');
throw new IncompleteDsnException('User is not set.', $dsn->getOriginalDsn());
}

return $user;
Expand All @@ -58,7 +58,7 @@ protected function getPassword(Dsn $dsn): string
{
$password = $dsn->getPassword();
if (null === $password) {
throw new IncompleteDsnException('Password is not set.');
throw new IncompleteDsnException('Password is not set.', $dsn->getOriginalDsn());
}

return $password;
Expand Down
11 changes: 10 additions & 1 deletion src/Symfony/Component/Notifier/Transport/Dsn.php
Expand Up @@ -27,6 +27,7 @@ final class Dsn
private $port;
private $options;
private $path;
private $dsn;

public function __construct(string $scheme, string $host, ?string $user = null, ?string $password = null, ?int $port = null, array $options = [], ?string $path = null)
{
Expand Down Expand Up @@ -59,7 +60,10 @@ public static function fromString(string $dsn): self
$path = $parsedDsn['path'] ?? null;
parse_str($parsedDsn['query'] ?? '', $query);

return new self($parsedDsn['scheme'], $parsedDsn['host'], $user, $password, $port, $query, $path);
$dsnObject = new self($parsedDsn['scheme'], $parsedDsn['host'], $user, $password, $port, $query, $path);
$dsnObject->dsn = $dsn;

return $dsnObject;
}

public function getScheme(): string
Expand Down Expand Up @@ -96,4 +100,9 @@ public function getPath(): ?string
{
return $this->path;
}

public function getOriginalDsn(): string
{
return $this->dsn;
}
}

0 comments on commit 2235be0

Please sign in to comment.