-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Added support for RedisURL according to IANA spec #56
Changes from all commits
c7ca212
5b8b974
5b3ee01
ff75c13
69f7442
a8f8ae8
954474c
0d482f4
dc93124
2ae5842
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 |
---|---|---|
|
@@ -7,8 +7,7 @@ | |
use Amp\Socket\ClientConnectContext; | ||
use Amp\Socket\Socket; | ||
use Amp\Success; | ||
use Amp\Uri\InvalidUriException; | ||
use Amp\Uri\Uri; | ||
use Exception; | ||
use function Amp\asyncCall; | ||
use function Amp\call; | ||
use function Amp\Socket\connect; | ||
|
@@ -25,12 +24,6 @@ class Connection | |
/** @var RespParser */ | ||
private $parser; | ||
|
||
/** @var string */ | ||
private $uri; | ||
|
||
/** @var int */ | ||
private $timeout = 5000; | ||
|
||
/** @var Socket */ | ||
private $socket; | ||
|
||
|
@@ -40,17 +33,19 @@ class Connection | |
/** @var int */ | ||
private $state; | ||
|
||
/** @var ConnectionConfig */ | ||
private $config; | ||
|
||
/** @var Deferred[] */ | ||
private $deferreds; | ||
|
||
/** | ||
* @param string $uri | ||
* @param ConnectionConfig $config | ||
*/ | ||
public function __construct(string $uri) | ||
public function __construct(ConnectionConfig $config) | ||
{ | ||
if (\strpos($uri, "tcp://") !== 0 && \strpos($uri, "unix://") !== 0) { | ||
throw new InvalidUriException("URI must start with tcp:// or unix://"); | ||
} | ||
|
||
$this->applyUri($uri); | ||
|
||
$this->deferreds = []; | ||
$this->config = $config; | ||
$this->state = self::STATE_DISCONNECTED; | ||
|
||
$this->handlers = [ | ||
|
@@ -65,19 +60,53 @@ public function __construct(string $uri) | |
$handler($response); | ||
} | ||
}); | ||
} | ||
|
||
private function applyUri(string $uri) | ||
{ | ||
$uri = new Uri($uri); | ||
$this->addEventHandler("response", function ($response) { | ||
$deferred = \array_shift($this->deferreds); | ||
|
||
if ($uri->getScheme() === "tcp") { | ||
$this->uri = $uri->getScheme() . "://" . $uri->getHost() . ":" . $uri->getPort(); | ||
} else { | ||
$this->uri = $uri->getScheme() . "://" . $uri->getPath(); | ||
if (empty($this->deferreds)) { | ||
$this->setIdle(true); | ||
} | ||
if (!$deferred instanceof Deferred) { | ||
return; | ||
} | ||
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. This shouldn't happen, so it should throw instead of returning silently. |
||
|
||
if ($response instanceof Exception) { | ||
$deferred->fail($response); | ||
} else { | ||
$deferred->resolve($response); | ||
} | ||
}); | ||
|
||
$this->addEventHandler(["close", "error"], function ($error = null) { | ||
if ($error) { | ||
// Fail any outstanding promises | ||
while ($this->deferreds) { | ||
$deferred = \array_shift($this->deferreds); | ||
$deferred->fail($error); | ||
} | ||
} | ||
}); | ||
|
||
if ($this->config->hasPassword()) { | ||
$this->addEventHandler("connect", function () { | ||
// AUTH must be before any other command, so we unshift it last | ||
\array_unshift($this->deferreds, new Deferred); | ||
$password = $this->config->getPassword(); | ||
|
||
return "*2\r\n$4\r\rAUTH\r\n$" . \strlen($password) . "\r\n{$password}\r\n"; | ||
}); | ||
} | ||
|
||
$this->timeout = $uri->getQueryParameter("timeout") ?? $this->timeout; | ||
if ($this->config->getDatabase() !== 0) { | ||
$this->addEventHandler("connect", function () { | ||
// SELECT must be called for every new connection if another database than 0 is used | ||
\array_unshift($this->deferreds, new Deferred); | ||
$database = $this->config->getDatabase(); | ||
|
||
return "*2\r\n$6\r\rSELECT\r\n$" . \strlen($database) . "\r\n{$database}\r\n"; | ||
}); | ||
} | ||
} | ||
|
||
public function addEventHandler($event, callable $callback) | ||
|
@@ -100,13 +129,16 @@ public function addEventHandler($event, callable $callback) | |
*/ | ||
public function send(array $strings): Promise | ||
{ | ||
$deferred = new Deferred; | ||
$this->deferreds[] = $deferred; | ||
|
||
foreach ($strings as $string) { | ||
if (!\is_scalar($string)) { | ||
throw new \TypeError("All elements must be of type string or scalar and convertible to a string."); | ||
} | ||
} | ||
|
||
return call(function () use ($strings) { | ||
call(function () use ($strings) { | ||
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. The return value here shouldn't be ignored. I'd move |
||
$this->setIdle(false); | ||
|
||
$payload = ""; | ||
|
@@ -118,6 +150,8 @@ public function send(array $strings): Promise | |
yield $this->connect(); | ||
yield $this->socket->write($payload); | ||
}); | ||
|
||
return $deferred->promise(); | ||
} | ||
|
||
private function connect(): Promise | ||
|
@@ -135,7 +169,10 @@ private function connect(): Promise | |
$this->state = self::STATE_CONNECTING; | ||
$this->connectPromisor = new Deferred; | ||
$connectPromise = $this->connectPromisor->promise(); | ||
$socketPromise = connect($this->uri, (new ClientConnectContext)->withConnectTimeout($this->timeout)); | ||
$socketPromise = connect( | ||
$this->config->getUri(), | ||
(new ClientConnectContext)->withConnectTimeout($this->config->getTimeout()) | ||
); | ||
|
||
$socketPromise->onResolve(function ($error, Socket $socket = null) { | ||
$connectPromisor = $this->connectPromisor; | ||
|
@@ -205,18 +242,26 @@ public function setIdle(bool $idle) | |
|
||
public function close() | ||
{ | ||
$this->parser->reset(); | ||
$promise = Promise\all(\array_map(function (Deferred $deferred) { | ||
return $deferred->promise(); | ||
}, $this->deferreds)); | ||
|
||
if ($this->socket) { | ||
$this->socket->close(); | ||
$this->socket = null; | ||
} | ||
$promise->onResolve(function () { | ||
$this->parser->reset(); | ||
|
||
foreach ($this->handlers["close"] as $handler) { | ||
$handler(); | ||
} | ||
if ($this->socket) { | ||
$this->socket->close(); | ||
$this->socket = null; | ||
} | ||
|
||
$this->state = self::STATE_DISCONNECTED; | ||
foreach ($this->handlers["close"] as $handler) { | ||
$handler(); | ||
} | ||
|
||
$this->state = self::STATE_DISCONNECTED; | ||
}); | ||
|
||
return $promise; | ||
} | ||
|
||
public function getState(): int | ||
|
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 think we should roll
ConnectionConfig::parse
in here for now to keep it backward compatible.