bug #39545 [Notifier] [Mattermost] Host is required (OskarStark)

This PR was squashed before being merged into the 5.1 branch.

Discussion
----------

[Notifier] [Mattermost] Host is required

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | ---
| License       | MIT
| Doc PR        | ---

This bridge is the only one right now which cannot use `default` as host in the DSN, otherwise it would fall back to:
090b4256f0/src/Symfony/Component/Notifier/Transport/AbstractTransport.php (L30)

it could also not use:
090b4256f0/src/Symfony/Component/Notifier/Transport/AbstractTransport.php (L83-L86)

Based on the [documentation](https://api.mattermost.com/#tag/authentication) you must use your specific url like:
`your-mattermost-url.com/api/v4/...`

Using `localhost` would have weird side-effects.

Can you confirm this @thePanz , as you provided the bridge?

friendly ping @seb37800, you fixed some bugs in this transport

### Todos after merge
* [ ] adjust recipes with new DSN
* [ ] update the docs

Commits
-------

cd5b48003f [Notifier] [Mattermost] Host is required
This commit is contained in:
Nicolas Grekas 2020-12-18 11:34:54 +01:00
commit 8797138e8d
5 changed files with 35 additions and 5 deletions

View File

@ -28,11 +28,13 @@ final class MattermostTransport extends AbstractTransport
{
private $token;
private $channel;
private $path;
public function __construct(string $token, string $channel, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null)
public function __construct(string $token, string $channel, HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, string $path = null)
{
$this->token = $token;
$this->channel = $channel;
$this->path = $path;
parent::__construct($client, $dispatcher);
}
@ -56,14 +58,15 @@ final class MattermostTransport extends AbstractTransport
throw new LogicException(sprintf('The "%s" transport only supports instances of "%s" (instance of "%s" given).', __CLASS__, ChatMessage::class, get_debug_type($message)));
}
$endpoint = sprintf('https://%s/api/v4/posts', $this->getEndpoint());
$options = ($opts = $message->getOptions()) ? $opts->toArray() : [];
$options['message'] = $message->getSubject();
if (!isset($options['channel_id'])) {
$options['channel_id'] = $message->getRecipientId() ?: $this->channel;
}
$endpoint = sprintf('https://%s/api/v4/posts', $this->getEndpoint());
$response = $this->client->request('POST', $endpoint, [
'auth_bearer' => $this->token,
'json' => array_filter($options),
@ -75,4 +78,9 @@ final class MattermostTransport extends AbstractTransport
throw new TransportException(sprintf('Unable to post the Mattermost message: %s (%s).', $result['message'], $result['id']), $response);
}
}
protected function getEndpoint(): ?string
{
return rtrim($this->host.($this->port ? ':'.$this->port : '').($this->path ?? ''), '/');
}
}

View File

@ -32,6 +32,7 @@ final class MattermostTransportFactory extends AbstractTransportFactory
throw new UnsupportedSchemeException($dsn, 'mattermost', $this->getSupportedSchemes());
}
$path = $dsn->getPath();
$token = $this->getUser($dsn);
$channel = $dsn->getOption('channel');
@ -42,7 +43,7 @@ final class MattermostTransportFactory extends AbstractTransportFactory
$host = $dsn->getHost();
$port = $dsn->getPort();
return (new MattermostTransport($token, $channel, $this->client, $this->dispatcher))->setHost($host)->setPort($port);
return (new MattermostTransport($token, $channel, $this->client, $this->dispatcher, $path))->setHost($host)->setPort($port);
}
protected function getSupportedSchemes(): array

View File

@ -7,11 +7,13 @@ DSN example
-----------
```
MATTERMOST_DSN=mattermost://ACCESS_TOKEN@default?channel=CHANNEL
MATTERMOST_DSN=mattermost://ACCESS_TOKEN@HOST/PATH?channel=CHANNEL
```
where:
- `ACCESS_TOKEN` is your Mattermost access token
- `HOST` is your Mattermost host
- `PATH` is your Mattermost sub-path (optional)
- `CHANNEL` is your Mattermost channel
Resources

View File

@ -31,6 +31,24 @@ final class MattermostTransportFactoryTest extends TestCase
$this->assertSame('mattermost://host.test?channel=testChannel', (string) $transport);
}
public function testCreateWithDsnHostWithSubfolder()
{
$factory = $this->createFactory();
$transport = $factory->create(Dsn::fromString('mattermost://accessToken@example.com/sub?channel=testChannel'));
$this->assertSame('mattermost://example.com/sub?channel=testChannel', (string) $transport);
}
public function testCreateWithDsnHostWithSubfolderWithTrailingSlash()
{
$factory = $this->createFactory();
$transport = $factory->create(Dsn::fromString('mattermost://accessToken@example.com/sub/?channel=testChannel'));
$this->assertSame('mattermost://example.com/sub?channel=testChannel', (string) $transport);
}
public function testCreateWithMissingOptionChannelThrowsIncompleteDsnException()
{
$factory = $this->createFactory();

View File

@ -16,6 +16,7 @@ use Symfony\Component\Notifier\Bridge\Mattermost\MattermostTransport;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Message\ChatMessage;
use Symfony\Component\Notifier\Message\MessageInterface;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
/**