Skip to content

Commit 9a93741

Browse files
p123-stackp123-stack
committed
only re-enabled bookmark tests
1 parent 2eb998c commit 9a93741

File tree

8 files changed

+49
-159
lines changed

8 files changed

+49
-159
lines changed

‎docker-compose.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ services:
127127
- neo4j
128128
environment:
129129
TEST_NEO4J_HOST: neo4j
130-
TEST_NEO4J_PORT: 7687 # Add this if your testkit uses it
131130
TEST_NEO4J_USER: neo4j
132131
TEST_NEO4J_PASS: testtest
133132
TEST_DRIVER_NAME: php

‎src/Bolt/BoltConnection.php‎

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

6968
/**
7069
* @return array{0: V4_4|V5|V5_1|V5_2|V5_3|V5_4|null, 1: Connection}
@@ -162,10 +161,6 @@ public function isOpen(): bool
162161

163162
public function isStreaming(): bool
164163
{
165-
if (!isset($this->boltProtocol)) {
166-
return false;
167-
}
168-
169164
return in_array(
170165
$this->protocol()->serverState,
171166
[ServerState::STREAMING, ServerState::TX_STREAMING],
@@ -211,13 +206,6 @@ public function reset(): void
211206
$this->subscribedResults = [];
212207
}
213208

214-
private function prepareForBegin(): void
215-
{
216-
if (in_array($this->getServerState(), ['STREAMING', 'TX_STREAMING'], true)) {
217-
$this->discardUnconsumedResults();
218-
}
219-
}
220-
221209
/**
222210
* Begins a transaction.
223211
*
@@ -227,23 +215,12 @@ private function prepareForBegin(): void
227215
*/
228216
public function begin(?string $database, ?float $timeout, BookmarkHolder $holder, ?iterable $txMetaData): void
229217
{
230-
$this->logger?->log(LogLevel::DEBUG, 'BEGIN transaction');
231-
232-
// Only consume results if we're actually streaming
233-
if ($this->isStreaming()) {
234-
$this->logger?->log(LogLevel::DEBUG, 'Consuming results before BEGIN');
235-
$this->consumeResults();
236-
}
237-
238-
$extra = $this->buildRunExtra($database, $timeout, $holder, $this->getAccessMode(), $txMetaData);
239-
$this->logger?->log(LogLevel::DEBUG, 'BEGIN with extra', $extra);
218+
$this->consumeResults();
240219

220+
$extra = $this->buildRunExtra($database, $timeout, $holder, AccessMode::WRITE(), $txMetaData);
241221
$message = $this->messageFactory->createBeginMessage($extra);
242222
$response = $message->send()->getResponse();
243223
$this->assertNoFailure($response);
244-
$this->inTransaction = true;
245-
246-
$this->logger?->log(LogLevel::DEBUG, 'BEGIN successful');
247224
}
248225

249226
/**
@@ -276,11 +253,7 @@ public function run(
276253
?AccessMode $mode,
277254
?iterable $tsxMetadata,
278255
): array {
279-
if ($this->isInTransaction()) {
280-
$extra = [];
281-
} else {
282-
$extra = $this->buildRunExtra($database, $timeout, $holder, $mode, $tsxMetadata);
283-
}
256+
$extra = $this->buildRunExtra($database, $timeout, $holder, $mode, $tsxMetadata);
284257
$message = $this->messageFactory->createRunMessage($text, $parameters, $extra);
285258
$response = $message->send()->getResponse();
286259
$this->assertNoFailure($response);
@@ -296,14 +269,11 @@ public function run(
296269
*/
297270
public function rollback(): void
298271
{
299-
if ($this->isStreaming()) {
300-
$this->consumeResults();
301-
}
272+
$this->consumeResults();
302273

303274
$message = $this->messageFactory->createRollbackMessage();
304275
$response = $message->send()->getResponse();
305276
$this->assertNoFailure($response);
306-
$this->inTransaction = false;
307277
}
308278

309279
public function protocol(): V4_4|V5|V5_1|V5_2|V5_3|V5_4
@@ -361,30 +331,27 @@ public function close(): void
361331
}
362332
}
363333

364-
private function buildRunExtra(
365-
?string $database,
366-
?float $timeout,
367-
BookmarkHolder $holder,
368-
?AccessMode $mode,
369-
?iterable $metadata,
370-
): array {
334+
private function buildRunExtra(?string $database, ?float $timeout, BookmarkHolder $holder, ?AccessMode $mode, ?iterable $metadata): array
335+
{
371336
$extra = [];
372-
373337
if ($database !== null) {
374338
$extra['db'] = $database;
375339
}
376340
if ($timeout !== null) {
377341
$extra['tx_timeout'] = (int) ($timeout * 1000);
378342
}
379343

380-
$bookmarks = $holder->getBookmark()->values();
381-
if (!empty($bookmarks)) {
382-
$extra['bookmarks'] = $bookmarks;
344+
if (!$holder->getBookmark()->isEmpty()) {
345+
$extra['bookmarks'] = $holder->getBookmark()->values();
346+
}
347+
348+
if ($mode) {
349+
$extra['mode'] = AccessMode::WRITE() === $mode ? 'w' : 'r';
383350
}
384351

385352
if ($metadata !== null) {
386353
$metadataArray = $metadata instanceof Traversable ? iterator_to_array($metadata) : $metadata;
387-
if (!empty($metadataArray)) {
354+
if (count($metadataArray) > 0) {
388355
$extra['tx_metadata'] = $metadataArray;
389356
}
390357
}
@@ -395,13 +362,11 @@ private function buildRunExtra(
395362
private function buildResultExtra(?int $fetchSize, ?int $qid): array
396363
{
397364
$extra = [];
398-
$fetchSize = 1000;
399-
/** @psalm-suppress RedundantCondition */
400365
if ($fetchSize !== null) {
401366
$extra['n'] = $fetchSize;
402367
}
403368

404-
if ($qid !== null && $qid >= 0) {
369+
if ($qid !== null) {
405370
$extra['qid'] = $qid;
406371
}
407372

@@ -462,38 +427,17 @@ public function discardUnconsumedResults(): void
462427
static fn (WeakReference $ref): bool => $ref->get() !== null
463428
));
464429

465-
if (empty($this->subscribedResults)) {
466-
$this->logger?->log(LogLevel::DEBUG, 'No unconsumed results to discard');
467-
468-
return;
469-
}
470-
471-
$state = $this->getServerState();
472-
$this->logger?->log(LogLevel::DEBUG, "Server state before discard: {$state}");
473-
474-
try {
475-
if (in_array($state, ['STREAMING', 'TX_STREAMING'], true)) {
430+
if (!empty($this->subscribedResults)) {
431+
try {
476432
$this->discard(null);
477433
$this->logger?->log(LogLevel::DEBUG, 'Sent DISCARD ALL for unconsumed results');
478-
} else {
479-
$this->logger?->log(LogLevel::DEBUG, 'Skipping discard - server not in streaming state');
434+
} catch (Throwable $e) {
435+
$this->logger?->log(LogLevel::ERROR, 'Failed to discard results', [
436+
'exception' => $e->getMessage(),
437+
]);
480438
}
481-
} catch (Throwable $e) {
482-
$this->logger?->log(LogLevel::ERROR, 'Failed to discard results', [
483-
'exception' => $e->getMessage(),
484-
]);
485439
}
486440

487441
$this->subscribedResults = [];
488442
}
489-
490-
private function isInTransaction(): bool
491-
{
492-
return $this->inTransaction;
493-
}
494-
495-
public function setTransactionFinished(): void
496-
{
497-
$this->inTransaction = false;
498-
}
499443
}

‎src/Bolt/Messages/BoltCommitMessage.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@ public function getResponse(): Response
5656
$bookmark = $content['bookmark'] ?? '';
5757

5858
if (trim($bookmark) !== '') {
59-
$this->logger?->log(LogLevel::DEBUG, 'Setting bookmark after commit', ['bookmark' => $bookmark]);
6059
$this->bookmarks->setBookmark(new Bookmark([$bookmark]));
6160
}
6261

6362
$this->connection->protocol()->serverState = ServerState::READY;
64-
$this->connection->setTransactionFinished();
6563

6664
return $response;
6765
}

‎src/Bolt/Session.php‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
use Laudis\Neo4j\Neo4j\Neo4jConnectionPool;
3434
use Laudis\Neo4j\Types\CypherList;
3535
use Psr\Log\LogLevel;
36-
use Throwable;
3736

3837
/**
3938
* A session using bolt connections.
@@ -169,14 +168,11 @@ public function transaction(callable $tsxHandler, ?TransactionConfiguration $con
169168
*/
170169
public function beginTransaction(?iterable $statements = null, ?TransactionConfiguration $config = null): UnmanagedTransactionInterface
171170
{
172-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::beginTransaction - START');
171+
$this->getLogger()?->log(LogLevel::INFO, 'Beginning transaction', ['statements' => $statements, 'config' => $config]);
173172
$config = $this->mergeTsxConfig($config);
174-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::beginTransaction - Calling startTransaction');
175173
$tsx = $this->startTransaction($config, $this->config);
176-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::beginTransaction - startTransaction returned');
177174

178175
$tsx->runStatements($statements ?? []);
179-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::beginTransaction - DONE');
180176

181177
return $tsx;
182178
}
@@ -230,32 +226,18 @@ private function acquireConnection(TransactionConfiguration $config, SessionConf
230226

231227
private function startTransaction(TransactionConfiguration $config, SessionConfiguration $sessionConfig): UnmanagedTransactionInterface
232228
{
233-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - START');
229+
$this->getLogger()?->log(LogLevel::INFO, 'Starting transaction', ['config' => $config, 'sessionConfig' => $sessionConfig]);
234230
try {
235-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - Acquiring connection');
236231
$connection = $this->acquireConnection($config, $sessionConfig);
237-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - Connection acquired');
238232

239-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - Calling connection->begin()');
240233
$connection->begin($this->config->getDatabase(), $config->getTimeout(), $this->bookmarkHolder, $config->getMetaData());
241-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - connection->begin() returned');
242234
} catch (Neo4jException $e) {
243-
$this->getLogger()?->log(LogLevel::ERROR, 'Session::startTransaction - Neo4jException', ['message' => $e->getMessage()]);
244235
if (isset($connection) && $connection->getServerState() === 'FAILED') {
245236
$connection->reset();
246237
}
247238
throw $e;
248-
} catch (Throwable $e) {
249-
$this->getLogger()?->log(LogLevel::ERROR, 'Session::startTransaction - Throwable', [
250-
'type' => get_class($e),
251-
'message' => $e->getMessage(),
252-
'trace' => $e->getTraceAsString(),
253-
]);
254-
throw $e;
255239
}
256240

257-
$this->getLogger()?->log(LogLevel::DEBUG, 'Session::startTransaction - Creating BoltUnmanagedTransaction');
258-
259241
return new BoltUnmanagedTransaction(
260242
$this->config->getDatabase(),
261243
$this->formatter,

‎testkit-backend/src/Handlers/SessionBeginTransaction.php‎

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

1414
namespace Laudis\Neo4j\TestkitBackend\Handlers;
1515

16-
use Laudis\Neo4j\Databags\Neo4jError;
1716
use Laudis\Neo4j\Databags\TransactionConfiguration;
1817
use Laudis\Neo4j\Exception\Neo4jException;
1918
use Laudis\Neo4j\TestkitBackend\Contracts\RequestHandlerInterface;
@@ -24,7 +23,6 @@
2423
use Laudis\Neo4j\TestkitBackend\Responses\TransactionResponse;
2524
use Psr\Log\LoggerInterface;
2625
use Symfony\Component\Uid\Uuid;
27-
use Throwable;
2826

2927
/**
3028
* @implements RequestHandlerInterface<SessionBeginTransactionRequest>
@@ -45,62 +43,38 @@ public function __construct(MainRepository $repository, LoggerInterface $logger)
4543
*/
4644
public function handle($request): TestkitResponseInterface
4745
{
48-
$this->logger->debug('SessionBeginTransaction: Starting', ['sessionId' => $request->getSessionId()]);
46+
$session = $this->repository->getSession($request->getSessionId());
4947

50-
try {
51-
$session = $this->repository->getSession($request->getSessionId());
52-
$this->logger->debug('SessionBeginTransaction: Got session');
53-
54-
$config = TransactionConfiguration::default();
48+
$config = TransactionConfiguration::default();
5549

56-
if ($request->getTimeout()) {
57-
$config = $config->withTimeout($request->getTimeout());
58-
}
50+
if ($request->getTimeout()) {
51+
$config = $config->withTimeout($request->getTimeout());
52+
}
5953

60-
if ($request->getTxMeta()) {
61-
$metaData = $request->getTxMeta();
62-
$actualMeta = [];
63-
if ($metaData !== null) {
64-
foreach ($metaData as $key => $meta) {
65-
$actualMeta[$key] = AbstractRunner::decodeToValue($meta);
66-
}
54+
if ($request->getTxMeta()) {
55+
$metaData = $request->getTxMeta();
56+
$actualMeta = [];
57+
if ($metaData !== null) {
58+
foreach ($metaData as $key => $meta) {
59+
$actualMeta[$key] = AbstractRunner::decodeToValue($meta);
6760
}
68-
$config = $config->withMetaData($actualMeta);
6961
}
62+
$config = $config->withMetaData($actualMeta);
63+
}
7064

71-
$this->logger->debug('SessionBeginTransaction: About to call beginTransaction');
72-
// TODO - Create beginReadTransaction and beginWriteTransaction
65+
// TODO - Create beginReadTransaction and beginWriteTransaction
66+
try {
7367
$transaction = $session->beginTransaction(null, $config);
74-
$this->logger->debug('SessionBeginTransaction: beginTransaction returned successfully');
75-
76-
$id = Uuid::v4();
77-
78-
$this->repository->addTransaction($id, $transaction);
79-
$this->repository->bindTransactionToSession($request->getSessionId(), $id);
80-
81-
$this->logger->debug('SessionBeginTransaction: Returning TransactionResponse', ['id' => $id]);
82-
83-
return new TransactionResponse($id);
8468
} catch (Neo4jException $exception) {
85-
$this->logger->error('SessionBeginTransaction: Neo4jException', [
86-
'exception' => $exception->getMessage(),
87-
'trace' => $exception->getTraceAsString(),
88-
]);
69+
$this->logger->debug($exception->__toString());
8970

9071
return new DriverErrorResponse($request->getSessionId(), $exception);
91-
} catch (Throwable $exception) {
92-
$this->logger->error('SessionBeginTransaction: Unexpected exception', [
93-
'type' => get_class($exception),
94-
'message' => $exception->getMessage(),
95-
'trace' => $exception->getTraceAsString(),
96-
]);
72+
}
73+
$id = Uuid::v4();
9774

98-
$neo4jException = new Neo4jException([new Neo4jError(
99-
'PHP.ClientError',
100-
$exception->getMessage()
101-
)]);
75+
$this->repository->addTransaction($id, $transaction);
76+
$this->repository->bindTransactionToSession($request->getSessionId(), $id);
10277

103-
return new DriverErrorResponse($request->getSessionId(), $neo4jException);
104-
}
78+
return new TransactionResponse($id);
10579
}
10680
}

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

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

1414
namespace Laudis\Neo4j\TestkitBackend\Requests;
1515

16+
use Laudis\Neo4j\Databags\TransactionConfiguration;
1617
use Symfony\Component\Uid\Uuid;
1718

1819
final class SessionBeginTransactionRequest
@@ -48,8 +49,8 @@ public function getTxMeta(): iterable
4849
return $this->txMeta ?? [];
4950
}
5051

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

0 commit comments

Comments
 (0)