Skip to content

Commit 6aa1fca

Browse files
transistivetransistive
committed
fixed connection reuse bug during tests
1 parent 10831e7 commit 6aa1fca

File tree

8 files changed

+52
-28
lines changed

8 files changed

+52
-28
lines changed

‎src/Bolt/BoltConnection.php

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@
1818
use Bolt\error\MessageException;
1919
use Bolt\protocol\V3;
2020
use Bolt\protocol\V4;
21-
use Laudis\Neo4j\Types\CypherList;
22-
use function count;
2321
use Laudis\Neo4j\BoltFactory;
2422
use Laudis\Neo4j\Common\ConnectionConfiguration;
2523
use Laudis\Neo4j\Contracts\ConnectionInterface;
2624
use Laudis\Neo4j\Databags\DatabaseInfo;
2725
use Laudis\Neo4j\Databags\DriverConfiguration;
28-
use Laudis\Neo4j\Databags\SummarizedResult;
2926
use Laudis\Neo4j\Enum\AccessMode;
3027
use Laudis\Neo4j\Enum\ConnectionProtocol;
28+
use Laudis\Neo4j\Types\CypherList;
3129
use LogicException;
3230
use Psr\Http\Message\UriInterface;
3331
use RuntimeException;
@@ -127,7 +125,7 @@ public function getDatabaseInfo(): ?DatabaseInfo
127125
*/
128126
public function isOpen(): bool
129127
{
130-
return $this->boltProtocol !== null;
128+
return $this->serverState !== 'DISCONNECTED' && $this->serverState !== 'DEFUNCT';
131129
}
132130

133131
public function open(): void
@@ -157,6 +155,7 @@ public function close(): void
157155
$this->protocol()->goodbye();
158156

159157
$this->serverState = 'DEFUNCT';
158+
$this->boltProtocol = null; // has to be set to null as the sockets don't recover nicely contrary to what the underlying code might lead you to believe
160159
}
161160

162161
private function consumeResults(): void
@@ -214,6 +213,13 @@ public function begin(?string $database, ?float $timeout): void
214213

215214
throw $e;
216215
}
216+
217+
$this->serverState = 'TX_READY';
218+
}
219+
220+
public function getFactory(): BoltFactory
221+
{
222+
return $this->factory;
217223
}
218224

219225
/**
@@ -360,7 +366,7 @@ public function pull(?int $qid, ?int $fetchSize): array
360366
throw $e;
361367
}
362368

363-
$this->interpretResult($tbr);
369+
$this->interpretResult($tbr[count($tbr) - 1]);
364370

365371
return $tbr;
366372
}
@@ -375,7 +381,7 @@ public function getDriverConfiguration(): DriverConfiguration
375381

376382
public function __destruct()
377383
{
378-
if ($this->serverState !== 'DISCONNECTED' && $this->serverState !== 'DEFUNCT') {
384+
if ($this->isOpen()) {
379385
$this->close();
380386
}
381387
}
@@ -396,11 +402,11 @@ private function buildRunExtra(?string $database, ?float $timeout): array
396402
private function buildResultExtra(?int $fetchSize, ?int $qid): array
397403
{
398404
$extra = [];
399-
if ($fetchSize) {
405+
if ($fetchSize !== null) {
400406
$extra['n'] = $fetchSize;
401407
}
402408

403-
if ($qid) {
409+
if ($qid !== null) {
404410
$extra['qid'] = $qid;
405411
}
406412

@@ -429,7 +435,7 @@ private function protocol(): V3
429435
private function interpretResult(array $result): void
430436
{
431437
if (str_starts_with($this->serverState, 'TX_')) {
432-
if ($result['has_more'] ?? count($this->subscribedResults) === 1) {
438+
if ($result['has_more'] ?? ($this->countResults() > 1)) {
433439
$this->serverState = 'TX_STREAMING';
434440
} else {
435441
$this->serverState = 'TX_READY';
@@ -440,4 +446,16 @@ private function interpretResult(array $result): void
440446
$this->serverState = 'READY';
441447
}
442448
}
449+
450+
private function countResults(): int
451+
{
452+
$ctr = 0;
453+
foreach ($this->subscribedResults as $result) {
454+
if ($result->get() !== null) {
455+
++$ctr;
456+
}
457+
}
458+
459+
return $ctr;
460+
}
443461
}

‎src/Bolt/BoltConnectionPool.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,7 @@ public function acquire(
6565

6666
foreach (self::$connectionCache[$key] as $i => $connection) {
6767
if (!$connection->isOpen()) {
68-
$sslConfig = $connection->getDriverConfiguration()->getSslConfiguration();
69-
$newSslConfig = $this->driverConfig->getSslConfiguration();
70-
if ($sslConfig->getMode() !== $newSslConfig->getMode() ||
71-
$sslConfig->isVerifyPeer() === $newSslConfig->isVerifyPeer()
72-
) {
68+
if ($this->compare($connection, $authenticate)) {
7369
$connection = $this->getConnection($connectingTo, $authenticate, $config);
7470

7571
/** @psalm-suppress PropertyTypeCoercion */
@@ -82,6 +78,9 @@ public function acquire(
8278

8379
return $connection;
8480
}
81+
if ($connection->getServerState() === 'READY' && !$this->compare($connection, $authenticate)) {
82+
return $connection;
83+
}
8584
}
8685

8786
$connection = $this->getConnection($connectingTo, $authenticate, $config);
@@ -124,4 +123,14 @@ private function getConnection(
124123

125124
return new BoltConnection($factory, $bolt, $config);
126125
}
126+
127+
private function compare(BoltConnection $connection, AuthenticateInterface $authenticate): bool
128+
{
129+
$sslConfig = $connection->getDriverConfiguration()->getSslConfiguration();
130+
$newSslConfig = $this->driverConfig->getSslConfiguration();
131+
132+
return $sslConfig->getMode() !== $newSslConfig->getMode() ||
133+
$sslConfig->isVerifyPeer() === $newSslConfig->isVerifyPeer() ||
134+
$authenticate !== $connection->getFactory()->getAuth();
135+
}
127136
}

‎src/Bolt/BoltResult.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public function rewind(): void
146146

147147
public function __destruct()
148148
{
149-
if (in_array($this->connection->getServerState(), ['STREAMING', 'TX_STREAMING', 'FAILED', 'INTERRUPTED'], true)) {
149+
if (in_array($this->connection->getServerState(), ['STREAMING', 'TX_STREAMING'], true)) {
150150
$this->discard();
151151
}
152152
}

‎src/Bolt/Session.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ private function startTransaction(TransactionConfiguration $config, SessionConfi
177177

178178
$connection->begin($this->config->getDatabase(), $config->getTimeout());
179179
} catch (MessageException $e) {
180-
if (isset($connection)) {
180+
if (isset($connection) && $connection->getServerState() === 'FAILED') {
181181
$connection->reset();
182182
}
183183
throw Neo4jException::fromMessageException($e);

‎src/BoltFactory.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,9 @@ private static function configureSsl(UriInterface $uri, StreamSocket $socket, Dr
113113
$socket->setSslContextOptions($options);
114114
}
115115
}
116+
117+
public function getAuth(): AuthenticateInterface
118+
{
119+
return $this->auth;
120+
}
116121
}

‎src/Formatter/SummarizedResultFormatter.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,9 @@ public function formatBoltResult(array $meta, BoltResult $result, BoltConnection
197197
/**
198198
* @psalm-suppress MixedArgument
199199
*
200-
* @var SummarizedResult<CypherMap<OGMTypes>> $result
200+
* @var SummarizedResult<CypherMap<OGMTypes>>
201201
*/
202-
$result = (new SummarizedResult($summary, $formattedResult))->withCacheLimit($result->getFetchSize());
203-
204-
$connection->subscribeResult($result);
205-
206-
return $result;
202+
return (new SummarizedResult($summary, $formattedResult))->withCacheLimit($result->getFetchSize());
207203
}
208204

209205
/**

‎tests/Integration/BasicDriverTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function getConnections(): array
4444

4545
$tbr = [];
4646
foreach (explode(',', $connections) as $connection) {
47-
$tbr[] = [$connection];
47+
$tbr[$connection] = [$connection];
4848
}
4949

5050
return $tbr;

‎tests/Integration/ComplexQueryTest.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,7 @@ public function testDiscardAfterTimeout(string $alias): void
409409
$result = $this->getClient()
410410
->getDriver($alias)
411411
->createSession()
412-
->run(
413-
"MATCH (n:Node) SET n.testing = 'hello' WITH * CALL apoc.util.sleep(2000000)",
414-
[],
415-
TransactionConfiguration::default()->withTimeout(150)
416-
);
412+
->run("CALL apoc.util.sleep(2000000)", [], TransactionConfiguration::default()->withTimeout(150));
417413

418414
unset($result);
419415
}

0 commit comments

Comments
 (0)