Skip to content

Commit 7b8e061

Browse files
p123-stackp123-stack
committed
fixed bookmarks tests
1 parent 4e3dd50 commit 7b8e061

File tree

8 files changed

+84
-44
lines changed

8 files changed

+84
-44
lines changed

‎docker-compose.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ services:
125125
- neo4j
126126
environment:
127127
TEST_NEO4J_HOST: neo4j
128+
TEST_NEO4J_PORT: 7687 # Add this if your testkit uses it
128129
TEST_NEO4J_USER: neo4j
129130
TEST_NEO4J_PASS: testtest
130131
TEST_DRIVER_NAME: php

‎src/Bolt/BoltConnection.php

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ class BoltConnection implements ConnectionInterface
6464
* @var list<WeakReference<CypherList>>
6565
*/
6666
private array $subscribedResults = [];
67+
private bool $inTransaction = false;
6768

6869
/**
6970
* @return array{0: V4_4|V5|V5_1|V5_2|V5_3|V5_4|null, 1: Connection}
@@ -206,21 +207,27 @@ public function reset(): void
206207
$this->subscribedResults = [];
207208
}
208209

210+
private function prepareForBegin(): void
211+
{
212+
if (in_array($this->getServerState(), ['STREAMING', 'TX_STREAMING'], true)) {
213+
$this->discardUnconsumedResults();
214+
}
215+
}
216+
209217
/**
210218
* Begins a transaction.
211219
*
212220
* Any of the preconditioned states are: 'READY', 'INTERRUPTED'.
213221
*
214-
* @param iterable<string, scalar|array|null>|null $txMetaData
222+
* @param array<string, scalar|array|null>|null $txMetaData
215223
*/
216-
public function begin(?string $database, ?float $timeout, BookmarkHolder $holder, ?iterable $txMetaData): void
224+
public function begin(?string $database, ?float $timeout, BookmarkHolder $holder, ?array $txMetaData): void
217225
{
218-
$this->consumeResults();
219-
220-
$extra = $this->buildRunExtra($database, $timeout, $holder, AccessMode::WRITE(), $txMetaData);
226+
$extra = $this->buildRunExtra($database, $timeout, $holder, null, $txMetaData, true);
221227
$message = $this->messageFactory->createBeginMessage($extra);
222228
$response = $message->send()->getResponse();
223229
$this->assertNoFailure($response);
230+
$this->inTransaction = true;
224231
}
225232

226233
/**
@@ -253,7 +260,11 @@ public function run(
253260
?AccessMode $mode,
254261
?iterable $tsxMetadata,
255262
): array {
256-
$extra = $this->buildRunExtra($database, $timeout, $holder, $mode, $tsxMetadata);
263+
if ($this->isInTransaction()) {
264+
$extra = [];
265+
} else {
266+
$extra = $this->buildRunExtra($database, $timeout, $holder, $mode, $tsxMetadata, false);
267+
}
257268
$message = $this->messageFactory->createRunMessage($text, $parameters, $extra);
258269
$response = $message->send()->getResponse();
259270
$this->assertNoFailure($response);
@@ -321,7 +332,6 @@ public function close(): void
321332
if ($this->isStreaming()) {
322333
$this->discardUnconsumedResults();
323334
}
324-
325335
$message = $this->messageFactory->createGoodbyeMessage();
326336
$message->send();
327337

@@ -331,7 +341,7 @@ public function close(): void
331341
}
332342
}
333343

334-
private function buildRunExtra(?string $database, ?float $timeout, BookmarkHolder $holder, ?AccessMode $mode, ?iterable $metadata): array
344+
private function buildRunExtra(?string $database, ?float $timeout, BookmarkHolder $holder, ?AccessMode $mode, ?iterable $metadata, bool $forBegin = false): array
335345
{
336346
$extra = [];
337347
if ($database !== null) {
@@ -341,18 +351,26 @@ private function buildRunExtra(?string $database, ?float $timeout, BookmarkHolde
341351
$extra['tx_timeout'] = (int) ($timeout * 1000);
342352
}
343353

344-
if (!$holder->getBookmark()->isEmpty()) {
354+
$bookmarks = $holder->getBookmark()->values();
355+
if (!empty($bookmarks)) {
345356
$extra['bookmarks'] = $holder->getBookmark()->values();
346357
}
347358

348-
if ($mode) {
349-
$extra['mode'] = AccessMode::WRITE() === $mode ? 'w' : 'r';
350-
}
359+
if ($forBegin) {
360+
$bookmarks = $holder->getBookmark()->values();
361+
if (!empty($bookmarks)) {
362+
$extra['bookmarks'] = $bookmarks;
363+
}
351364

352-
if ($metadata !== null) {
353-
$metadataArray = $metadata instanceof Traversable ? iterator_to_array($metadata) : $metadata;
354-
if (count($metadataArray) > 0) {
355-
$extra['tx_metadata'] = $metadataArray;
365+
if ($mode !== null) {
366+
$extra['mode'] = $mode === AccessMode::WRITE() ? 'w' : 'r';
367+
}
368+
369+
if ($metadata !== null) {
370+
$metadataArray = $metadata instanceof Traversable ? iterator_to_array($metadata) : $metadata;
371+
if (!empty($metadataArray)) {
372+
$extra['tx_metadata'] = $metadataArray;
373+
}
356374
}
357375
}
358376

@@ -362,11 +380,13 @@ private function buildRunExtra(?string $database, ?float $timeout, BookmarkHolde
362380
private function buildResultExtra(?int $fetchSize, ?int $qid): array
363381
{
364382
$extra = [];
383+
$fetchSize = 1000;
384+
/** @psalm-suppress RedundantCondition */
365385
if ($fetchSize !== null) {
366386
$extra['n'] = $fetchSize;
367387
}
368388

369-
if ($qid !== null) {
389+
if ($qid !== null && $qid >= 0) {
370390
$extra['qid'] = $qid;
371391
}
372392

@@ -412,23 +432,38 @@ private function assertNoFailure(Response $response): void
412432
public function discardUnconsumedResults(): void
413433
{
414434
$this->logger?->log(LogLevel::DEBUG, 'Discarding unconsumed results');
415-
416435
$this->subscribedResults = array_values(array_filter(
417436
$this->subscribedResults,
418437
static fn (WeakReference $ref): bool => $ref->get() !== null
419438
));
420439

421-
if (!empty($this->subscribedResults)) {
422-
try {
440+
if (empty($this->subscribedResults)) {
441+
$this->logger?->log(LogLevel::DEBUG, 'No unconsumed results to discard');
442+
443+
return;
444+
}
445+
446+
$state = $this->getServerState();
447+
$this->logger?->log(LogLevel::DEBUG, "Server state before discard: {$state}");
448+
449+
try {
450+
if (in_array($state, ['STREAMING', 'TX_STREAMING'], true)) {
423451
$this->discard(null);
424452
$this->logger?->log(LogLevel::DEBUG, 'Sent DISCARD ALL for unconsumed results');
425-
} catch (Throwable $e) {
426-
$this->logger?->log(LogLevel::ERROR, 'Failed to discard results', [
427-
'exception' => $e->getMessage(),
428-
]);
453+
} else {
454+
$this->logger?->log(LogLevel::DEBUG, 'Skipping discard - server not in streaming state');
429455
}
456+
} catch (Throwable $e) {
457+
$this->logger?->log(LogLevel::ERROR, 'Failed to discard results', [
458+
'exception' => $e->getMessage(),
459+
]);
430460
}
431461

432462
$this->subscribedResults = [];
433463
}
464+
465+
private function isInTransaction(): bool
466+
{
467+
return $this->inTransaction;
468+
}
434469
}

‎src/Bolt/Session.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,14 +190,14 @@ private function startTransaction(TransactionConfiguration $config, SessionConfi
190190
$this->getLogger()?->log(LogLevel::INFO, 'Starting transaction', ['config' => $config, 'sessionConfig' => $sessionConfig]);
191191
try {
192192
$connection = $this->acquireConnection($config, $sessionConfig);
193-
194193
$connection->begin($this->config->getDatabase(), $config->getTimeout(), $this->bookmarkHolder, $config->getMetaData());
195194
} catch (Neo4jException $e) {
196195
if (isset($connection) && $connection->getServerState() === 'FAILED') {
197196
$connection->reset();
198197
}
199198
throw $e;
200199
}
200+
error_log('>>> EXIT startTransaction()');
201201

202202
return new BoltUnmanagedTransaction(
203203
$this->config->getDatabase(),

‎src/Databags/TransactionConfiguration.php

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ final class TransactionConfiguration
2424
public const DEFAULT_METADATA = '[]';
2525

2626
/**
27-
* @param float|null $timeout timeout in seconds
28-
* @param iterable<string, scalar|array|null>|null $metaData
27+
* @param float|null $timeout timeout in seconds
28+
* @param array<array-key, mixed>|null $metaData
2929
*/
3030
public function __construct(
3131
private ?float $timeout = null,
32-
private ?iterable $metaData = null,
32+
private ?array $metaData = null,
3333
) {
3434
}
3535

@@ -41,7 +41,7 @@ public function __construct(
4141
*/
4242
public static function create(?float $timeout = null, ?iterable $metaData = null): self
4343
{
44-
return new self($timeout, $metaData);
44+
return new self($timeout, $metaData !== null ? (array) $metaData : null);
4545
}
4646

4747
/**
@@ -53,11 +53,9 @@ public static function default(): self
5353
}
5454

5555
/**
56-
* Get the configured transaction metadata.
57-
*
58-
* @return iterable<string, scalar|array|null>|null
56+
* @return array<string, scalar|array|null>|null
5957
*/
60-
public function getMetaData(): ?iterable
58+
public function getMetaData(): ?array
6159
{
6260
return $this->metaData;
6361
}
@@ -87,7 +85,7 @@ public function withTimeout(?float $timeout): self
8785
*/
8886
public function withMetaData(?iterable $metaData): self
8987
{
90-
return new self($this->timeout, $metaData);
88+
return new self($this->timeout, $metaData !== null ? (array) $metaData : null);
9189
}
9290

9391
/**
@@ -101,6 +99,7 @@ public function merge(?TransactionConfiguration $config): self
10199

102100
$metaData = $config->metaData;
103101
if ($metaData !== null) {
102+
/** @psalm-suppress PossiblyInvalidArgument */
104103
$tsxConfig = $tsxConfig->withMetaData($metaData);
105104
}
106105
$timeout = $config->timeout;

‎testkit-backend/src/RequestFactory.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function create(string $name, iterable $data): object
8383
return new AuthorizationTokenRequest(
8484
$data['scheme'],
8585
$data['realm'] ?? '',
86-
$data['principal'],
86+
$data['principal'] ?? '',
8787
$data['credentials']
8888
);
8989
}

‎testkit-backend/src/Requests/SessionBeginTransactionRequest.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
namespace Laudis\Neo4j\TestkitBackend\Requests;
1515

16-
use Laudis\Neo4j\Databags\TransactionConfiguration;
1716
use Symfony\Component\Uid\Uuid;
1817

1918
final class SessionBeginTransactionRequest
@@ -49,8 +48,8 @@ public function getTxMeta(): iterable
4948
return $this->txMeta ?? [];
5049
}
5150

52-
public function getTimeout(): int
51+
public function getTimeout(): ?int
5352
{
54-
return (int) ($this->timeout ?? TransactionConfiguration::DEFAULT_TIMEOUT);
53+
return $this->timeout;
5554
}
5655
}

‎testkit-backend/testkit.sh

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,17 @@ EXIT_CODE=0
8282

8383

8484
#test-session-run
85-
python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_discard_on_session_close_untouched_result
86-
python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_discard_on_session_close_unfinished_result
87-
python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_no_discard_on_session_close_finished_result
88-
python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_raises_error_on_session_run
89-
85+
#python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_discard_on_session_close_untouched_result
86+
#python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_discard_on_session_close_unfinished_result
87+
#python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_no_discard_on_session_close_finished_result
88+
#python3 -m unittest tests.stub.session_run.test_session_run.TestSessionRun.test_raises_error_on_session_run
89+
90+
#bookmarks
91+
#TestBookmarksV4
92+
python3 -m unittest tests.stub.bookmarks.test_bookmarks_v4.TestBookmarksV4.test_bookmarks_on_unused_sessions_are_returned
93+
python3 -m unittest tests.stub.bookmarks.test_bookmarks_v4.TestBookmarksV4.test_bookmarks_session_run
94+
python3 -m unittest tests.stub.bookmarks.test_bookmarks_v4.TestBookmarksV4.test_bookmarks_tx_run
95+
python3 -m unittest tests.stub.bookmarks.test_bookmarks_v4.TestBookmarksV4.test_sequence_of_writing_and_reading_tx
9096

9197
exit $EXIT_CODE
9298

‎tests/Integration/ComplexQueryTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ public function testDiscardAfterTimeout(): void
268268

269269
try {
270270
$this->getSession(['bolt', 'neo4j'])
271-
->run('CALL apoc.util.sleep(2000000) RETURN 5 as x', [], TransactionConfiguration::default()->withTimeout(2))
271+
->run('CALL apoc.util.sleep(3000) RETURN 5 as x', [], TransactionConfiguration::default()->withTimeout(2))
272272
->first()
273273
->get('x');
274274
} catch (Neo4jException $e) {

0 commit comments

Comments
 (0)