-
Notifications
You must be signed in to change notification settings - Fork 4
[+] Guzzle Transpost #68
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <?php | ||
|
|
||
| namespace Hawk\Exception; | ||
|
|
||
| class TransportException extends \ErrorException | ||
| { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Hawk; | ||
|
|
||
| use Hawk\Exception\TransportException; | ||
| use Hawk\Transport\CurlTransport; | ||
| use Hawk\Transport\GuzzleTransport; | ||
|
|
||
| /** | ||
| * Interface TransportInterface | ||
| * | ||
| * @package Hawk\Transport | ||
| */ | ||
| class Transport | ||
neSpecc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| /** | ||
| * @var string | ||
| */ | ||
| protected $url; | ||
|
|
||
| /** | ||
| * @var int | ||
|
Comment on lines
+18
to
+24
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. docs missed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. docs are needed not only for typings. Add descriptions: why this property or function is used |
||
| */ | ||
| protected $timeout; | ||
|
|
||
| public static function init(Options $options) | ||
| { | ||
| $transports = self::getTransports(); | ||
|
|
||
| if (!array_key_exists($options->getTransport(), $transports)) { | ||
| throw new TransportException('Invalid transport specified'); | ||
| } | ||
|
|
||
| return new $transports[$options->getTransport()]($options); | ||
| } | ||
|
|
||
| public static function getTransports(): array | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. docs missed |
||
| { | ||
| return [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a hardcoded static registry in getTransports violates the Open-Closed Principle (OCP). Adding a new transport requires modifying the base class. |
||
| 'curl' => CurlTransport::class, | ||
| 'guzzle' => GuzzleTransport::class, | ||
| ]; | ||
| } | ||
|
|
||
| public function __construct(Options $options) | ||
| { | ||
| $this->url = $options->getUrl(); | ||
| $this->timeout = $options->getTimeout(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns URL that object must send an Event | ||
| * | ||
| * @return string | ||
| */ | ||
| public function getUrl(): string | ||
| { | ||
| return $this->url; | ||
| } | ||
|
|
||
| public function getTimeout(): int | ||
| { | ||
| return $this->timeout; | ||
| } | ||
|
|
||
| /** | ||
| * Sends an Event | ||
| * | ||
| * @param Event $event | ||
| * | ||
| * @return mixed | ||
| */ | ||
| public function send(Event $event) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. public function send(Event $event): ?array |
||
| { | ||
| $response = $this->_send($event); | ||
|
|
||
| try { | ||
| $data = json_decode($response, true); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. $data = json_decode($response, true); |
||
| } catch (\Exception $e) { | ||
| $data = null; | ||
| } | ||
|
|
||
| return $data; | ||
| } | ||
|
|
||
| protected function _send(Event $event): string | ||
| { | ||
| throw new TransportException('Not implemented transport method _send()'); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,34 +5,46 @@ | |
| namespace Hawk\Transport; | ||
|
|
||
| use Hawk\Event; | ||
| use Hawk\Options; | ||
| use Hawk\Transport; | ||
|
|
||
| /** | ||
| * Class GuzzleTransport | ||
| * | ||
| * @package Hawk\Transport | ||
| */ | ||
| class GuzzleTransport implements TransportInterface | ||
| class GuzzleTransport extends Transport | ||
| { | ||
| private $url; | ||
|
|
||
| public function __construct(string $url) | ||
| { | ||
| $this->url = $url; | ||
| } | ||
| /** | ||
| * @var \GuzzleHttp\Client | ||
| */ | ||
| private $client; | ||
|
|
||
| /** | ||
| * @inheritDoc | ||
| * GuzzleTransport constructor. | ||
| * | ||
| * @param Options $options | ||
| */ | ||
| public function getUrl(): string | ||
| public function __construct(Options $options) | ||
| { | ||
| return $this->url; | ||
| parent::__construct($options); | ||
|
|
||
| $this->client = new \GuzzleHttp\Client(); | ||
| } | ||
|
|
||
| /** | ||
| * @inheritDoc | ||
| */ | ||
| public function send(Event $event): void | ||
| protected function _send(Event $event): string | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| { | ||
| // TODO: Implement send() method. | ||
| $response = $this->client->post($this->getUrl(), [ | ||
| 'headers' => [ | ||
| 'Content-Type' => 'application/json', | ||
| ], | ||
| 'body' => json_encode($event, JSON_UNESCAPED_UNICODE), | ||
| 'timeout' => $this->getTimeout(), | ||
| ]); | ||
|
|
||
| return $response->getBody()->getContents(); | ||
| } | ||
| } | ||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to add a separate case for transport validation to restrict it only for allowed values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neSpecc
Hawk\Transport::inithave validation