Skip to content

Commit 10bad0c

Browse files
committed
Enhance StreamConnection
Also, fixes edge case error when StreamConnection::open() was returning null instead of the Greeting object: TypeError: Return value of Tarantool\Client\Connection\StreamConnection::open() must be an instance of Tarantool\Client\Connection\Greeting, null returned
1 parent afc89bd commit 10bad0c

File tree

3 files changed

+75
-11
lines changed

3 files changed

+75
-11
lines changed

src/Connection/StreamConnection.php

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,9 @@ public static function create(string $uri, array $options = []) : self
7676
: self::createTcp($uri, $options);
7777
}
7878

79-
/**
80-
* @see https://github.com/vimeo/psalm/issues/3021
81-
* @psalm-suppress InvalidNullableReturnType
82-
*/
8379
public function open() : Greeting
8480
{
85-
if (\is_resource($this->stream)) {
86-
/** @see https://github.com/vimeo/psalm/issues/3021 */
87-
/** @psalm-suppress NullableReturnStatement */
81+
if ($this->greeting) {
8882
return $this->greeting;
8983
}
9084

@@ -125,7 +119,7 @@ public function open() : Greeting
125119

126120
public function close() : void
127121
{
128-
if (\is_resource($this->stream)) {
122+
if ($this->stream) {
129123
/** @psalm-suppress InvalidPropertyAssignmentValue */
130124
\fclose($this->stream);
131125
}
@@ -136,13 +130,12 @@ public function close() : void
136130

137131
public function isClosed() : bool
138132
{
139-
return !\is_resource($this->stream);
133+
return !$this->stream;
140134
}
141135

142136
public function send(string $data) : string
143137
{
144-
/** @psalm-suppress PossiblyNullArgument */
145-
if (!\fwrite($this->stream, $data)) {
138+
if (!$this->stream || !\fwrite($this->stream, $data)) {
146139
throw new CommunicationFailed('Unable to write request');
147140
}
148141

tests/Integration/Connection/ConnectionTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,17 @@
1313

1414
namespace Tarantool\Client\Tests\Integration\Connection;
1515

16+
use Tarantool\Client\Exception\CommunicationFailed;
1617
use Tarantool\Client\Exception\ConnectionFailed;
1718
use Tarantool\Client\Exception\UnexpectedResponse;
1819
use Tarantool\Client\Request\PingRequest;
1920
use Tarantool\Client\Schema\Criteria;
2021
use Tarantool\Client\Schema\Operations;
22+
use Tarantool\Client\Tests\GreetingDataProvider;
2123
use Tarantool\Client\Tests\Integration\ClientBuilder;
24+
use Tarantool\Client\Tests\Integration\FakeServer\FakeServerBuilder;
25+
use Tarantool\Client\Tests\Integration\FakeServer\Handler\AtConnectionHandler;
26+
use Tarantool\Client\Tests\Integration\FakeServer\Handler\WriteHandler;
2227
use Tarantool\Client\Tests\Integration\TestCase;
2328

2429
final class ConnectionTest extends TestCase
@@ -181,4 +186,32 @@ public function testUnexpectedResponse() : void
181186

182187
self::fail(UnexpectedResponse::class.' was not thrown');
183188
}
189+
190+
public function testOpenConnectionHandlesTheMissingGreetingCorrectly() : void
191+
{
192+
$clientBuilder = ClientBuilder::createFromEnvForTheFakeServer();
193+
194+
FakeServerBuilder::create(
195+
new AtConnectionHandler(1, new WriteHandler('')),
196+
new AtConnectionHandler(2, new WriteHandler(GreetingDataProvider::generateGreeting()))
197+
)
198+
->setUri($clientBuilder->getUri())
199+
->start();
200+
201+
$client = $clientBuilder->build();
202+
$connection = $client->getHandler()->getConnection();
203+
204+
try {
205+
$connection->open();
206+
self::fail('Connection not established');
207+
} catch (CommunicationFailed $e) {
208+
self::assertSame('Unable to read greeting', $e->getMessage());
209+
// at that point the connection was successfully established,
210+
// but the greeting message was not read
211+
}
212+
213+
// the second call should correctly handle
214+
// the missing greeting from the previous call
215+
$connection->open();
216+
}
184217
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
/**
4+
* This file is part of the Tarantool Client package.
5+
*
6+
* (c) Eugene Leonovich <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace Tarantool\Client\Tests\Integration\FakeServer\Handler;
15+
16+
class AtConnectionHandler implements Handler
17+
{
18+
private $atConnectionNumber;
19+
private $handler;
20+
private $connectionCount = 0;
21+
22+
public function __construct(int $atConnectionNumber, Handler $handler)
23+
{
24+
$this->atConnectionNumber = $atConnectionNumber;
25+
$this->handler = $handler;
26+
}
27+
28+
public function __invoke($conn, string $sid) : ?bool
29+
{
30+
++$this->connectionCount;
31+
32+
if ($this->connectionCount === $this->atConnectionNumber) {
33+
return ($this->handler)($conn, $sid);
34+
}
35+
36+
return null;
37+
}
38+
}

0 commit comments

Comments
 (0)