Skip to content

Commit 238c52c

Browse files
p123-stackp123-stack
committed
resolved merge conflicts
2 parents 5fed5eb + 5c6e5ab commit 238c52c

File tree

7 files changed

+172
-87
lines changed

7 files changed

+172
-87
lines changed

‎Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
ARG PHP_VERSION
1+
ARG PHP_VERSION=8.1
22

33
FROM php:${PHP_VERSION}-cli
44
RUN apt-get update \

‎src/Bolt/Session.php‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
use Exception;
1717
use Laudis\Neo4j\Common\GeneratorHelper;
1818
use Laudis\Neo4j\Common\Neo4jLogger;
19-
use Laudis\Neo4j\Common\TransactionHelper;
2019
use Laudis\Neo4j\Contracts\ConnectionPoolInterface;
20+
use Laudis\Neo4j\Contracts\CypherSequence;
2121
use Laudis\Neo4j\Contracts\SessionInterface;
2222
use Laudis\Neo4j\Contracts\TransactionInterface;
2323
use Laudis\Neo4j\Contracts\UnmanagedTransactionInterface;
@@ -41,6 +41,7 @@ final class Session implements SessionInterface
4141
{
4242
/** @psalm-readonly */
4343
private readonly BookmarkHolder $bookmarkHolder;
44+
private const ROLLBACK_CLASSIFICATIONS = ['ClientError', 'TransientError', 'DatabaseError'];
4445

4546
/**
4647
* @param ConnectionPool|Neo4jConnectionPool $pool
@@ -100,21 +101,59 @@ public function writeTransaction(callable $tsxHandler, ?TransactionConfiguration
100101
$this->getLogger()?->log(LogLevel::INFO, 'Beginning write transaction', ['config' => $config]);
101102
$config = $this->mergeTsxConfig($config);
102103

103-
return TransactionHelper::retry(
104-
fn () => $this->startTransaction($config, $this->config->withAccessMode(AccessMode::WRITE())),
105-
$tsxHandler
106-
);
104+
return $this->retry($tsxHandler, false, $config);
107105
}
108106

109107
public function readTransaction(callable $tsxHandler, ?TransactionConfiguration $config = null)
110108
{
111109
$this->getLogger()?->log(LogLevel::INFO, 'Beginning read transaction', ['config' => $config]);
112110
$config = $this->mergeTsxConfig($config);
113111

114-
return TransactionHelper::retry(
115-
fn () => $this->startTransaction($config, $this->config->withAccessMode(AccessMode::READ())),
116-
$tsxHandler
117-
);
112+
return $this->retry($tsxHandler, true, $config);
113+
}
114+
115+
/**
116+
* @template U
117+
*
118+
* @param callable(TransactionInterface):U $tsxHandler
119+
*
120+
* @return U
121+
*/
122+
private function retry(callable $tsxHandler, bool $read, TransactionConfiguration $config)
123+
{
124+
while (true) {
125+
$transaction = null;
126+
try {
127+
if ($read) {
128+
$transaction = $this->startTransaction($config, $this->config->withAccessMode(AccessMode::READ()));
129+
} else {
130+
$transaction = $this->startTransaction($config, $this->config->withAccessMode(AccessMode::WRITE()));
131+
}
132+
$tbr = $tsxHandler($transaction);
133+
self::triggerLazyResult($tbr);
134+
$transaction->commit();
135+
136+
return $tbr;
137+
} catch (Neo4jException $e) {
138+
if ($transaction && !in_array($e->getClassification(), self::ROLLBACK_CLASSIFICATIONS)) {
139+
$transaction->rollback();
140+
}
141+
142+
if ($e->getTitle() === 'NotALeader') {
143+
// By closing the pool, we force the connection to be re-acquired and the routing table to be refetched
144+
$this->pool->close();
145+
} elseif ($e->getClassification() !== 'TransientError') {
146+
throw $e;
147+
}
148+
}
149+
}
150+
}
151+
152+
private static function triggerLazyResult(mixed $tbr): void
153+
{
154+
if ($tbr instanceof CypherSequence) {
155+
$tbr->preload();
156+
}
118157
}
119158

120159
public function transaction(callable $tsxHandler, ?TransactionConfiguration $config = null)

‎src/Common/TransactionHelper.php‎

Lines changed: 0 additions & 62 deletions
This file was deleted.

‎src/Formatter/SummarizedResultFormatter.php‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@
8686
* constraints-added?: int,
8787
* constraints-removed?: int,
8888
* contains-updates?: bool,
89-
* contains-system-updates?: bool,
90-
* system-updates?: int,
89+
* contains-system-updates?: bool|int,
90+
* system-updates?: int|bool,
9191
* db?: string
9292
* }
9393
* @psalm-type CypherError = array{code: string, message: string}
@@ -138,6 +138,21 @@ public function formatBoltStats(array $response): SummaryCounters
138138
}
139139
}
140140

141+
$systemUpdates = $stats['system-updates'] ?? 0;
142+
if (is_bool($systemUpdates)) {
143+
$systemUpdates = (int) $systemUpdates;
144+
}
145+
146+
$containsSystemUpdates = $stats['contains-system-updates'] ?? null;
147+
148+
if ($containsSystemUpdates === null) {
149+
$containsSystemUpdates = $systemUpdates > 0;
150+
} else {
151+
if (!is_bool($containsSystemUpdates)) {
152+
$containsSystemUpdates = (bool) $containsSystemUpdates;
153+
}
154+
}
155+
141156
return new SummaryCounters(
142157
$stats['nodes-created'] ?? 0,
143158
$stats['nodes-deleted'] ?? 0,
@@ -151,8 +166,8 @@ public function formatBoltStats(array $response): SummaryCounters
151166
$stats['constraints-added'] ?? 0,
152167
$stats['constraints-removed'] ?? 0,
153168
$updateCount > 0,
154-
($stats['contains-system-updates'] ?? $stats['system-updates'] ?? 0) >= 1,
155-
$stats['system-updates'] ?? 0
169+
$containsSystemUpdates,
170+
$systemUpdates
156171
);
157172
}
158173

@@ -195,10 +210,11 @@ function (mixed $response) use ($connection, $statement, $runStart, $resultAvail
195210

196211
$formattedResult = $this->processBoltResult($meta, $result, $connection, $holder);
197212

198-
/**
199-
* @var SummarizedResult<CypherMap<OGMTypes>>
200-
*/
201-
return new SummarizedResult($summary, (new CypherList($formattedResult))->withCacheLimit($result->getFetchSize()));
213+
/** @var SummarizedResult */
214+
$result = (new CypherList($formattedResult))->withCacheLimit($result->getFetchSize());
215+
// $keys = $meta['fields'];
216+
217+
return new SummarizedResult($summary, $result);
202218
}
203219

204220
public function formatArgs(array $profiledPlanData): PlanArguments
@@ -255,7 +271,7 @@ private function formatProfiledPlan(array $profiledPlanData): ProfiledQueryPlan
255271
pageCacheHitRatio: (float) ($profiledPlanData['pageCacheHitRatio'] ?? 0.0),
256272
time: (int) ($profiledPlanData['time'] ?? 0),
257273
operatorType: $profiledPlanData['operatorType'] ?? '',
258-
children: array_map([$this, 'formatProfiledPlan'], $profiledPlanData['children'] ?? []),
274+
children: array_values(array_map([$this, 'formatProfiledPlan'], $profiledPlanData['children'] ?? [])),
259275
identifiers: $profiledPlanData['identifiers'] ?? []
260276
);
261277
}
@@ -309,7 +325,7 @@ private function formatPlan(array $plan): Plan
309325
{
310326
return new Plan(
311327
$this->formatArgs($plan['args']),
312-
array_map($this->formatPlan(...), $plan['children'] ?? []),
328+
array_values(array_map($this->formatPlan(...), $plan['children'] ?? [])),
313329
$plan['identifiers'] ?? [],
314330
$plan['operatorType'] ?? ''
315331
);

‎testkit-backend/src/MainRepository.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,4 @@ public function getTsxIdFromSession(Uuid $sessionId): Uuid
179179
return $this->sessionToTransactions[$sessionId->toRfc4122()];
180180
}
181181

182-
public function addBufferedRecords(string $id, array $records): void
183-
{
184-
$this->records[$id] = $records;
185-
}
186182
}

‎testkit-backend/testkit.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,4 @@ python3 -m unittest tests.neo4j.test_tx_run.TestTxRun.test_parallel_queries ||
110110
python3 -m unittest tests.neo4j.test_tx_run.TestTxRun.test_interwoven_queries || EXIT_CODE=1
111111
python3 -m unittest tests.neo4j.test_tx_run.TestTxRun.test_unconsumed_result || EXIT_CODE=1
112112

113-
113+
exit $EXIT_CODE

‎tests/Integration/SummarizedResultFormatterTest.php‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
use Laudis\Neo4j\Contracts\TransactionInterface;
2525
use Laudis\Neo4j\Databags\SummarizedResult;
2626
use Laudis\Neo4j\Databags\SummaryCounters;
27+
use Laudis\Neo4j\Formatter\Specialised\BoltOGMTranslator;
28+
use Laudis\Neo4j\Formatter\SummarizedResultFormatter;
2729
use Laudis\Neo4j\Tests\EnvironmentAwareIntegrationTest;
2830
use Laudis\Neo4j\Types\CartesianPoint;
2931
use Laudis\Neo4j\Types\CypherList;
@@ -521,4 +523,98 @@ private function articlesQuery(): string
521523
article.readingTime = duration(articleProperties.readingTime)
522524
CYPHER;
523525
}
526+
527+
public function testFormatBoltStatsWithFalseSystemUpdates(): void
528+
{
529+
$formatter = new SummarizedResultFormatter(new BoltOGMTranslator());
530+
531+
$response = [
532+
'stats' => [
533+
'nodes-created' => 1,
534+
'nodes-deleted' => 0,
535+
'relationships-created' => 0,
536+
'relationships-deleted' => 0,
537+
'properties-set' => 2,
538+
'labels-added' => 1,
539+
'labels-removed' => 0,
540+
'indexes-added' => 0,
541+
'indexes-removed' => 0,
542+
'constraints-added' => 0,
543+
'constraints-removed' => 0,
544+
'contains-updates' => true,
545+
'contains-system-updates' => false,
546+
'system-updates' => false,
547+
],
548+
];
549+
550+
$counters = $formatter->formatBoltStats($response);
551+
552+
self::assertInstanceOf(SummaryCounters::class, $counters);
553+
self::assertEquals(1, $counters->nodesCreated());
554+
self::assertEquals(2, $counters->propertiesSet());
555+
self::assertSame(0, $counters->systemUpdates());
556+
}
557+
558+
public function testSystemUpdatesWithPotentialFalseValues(): void
559+
{
560+
$this->getSession()->run('CREATE INDEX duplicate_test_index IF NOT EXISTS FOR (n:TestSystemUpdates) ON (n.duplicateProperty)');
561+
$result = $this->getSession()->run('CREATE INDEX duplicate_test_index IF NOT EXISTS FOR (n:TestSystemUpdates) ON (n.duplicateProperty)');
562+
563+
$summary = $result->getSummary();
564+
$counters = $summary->getCounters();
565+
566+
// For duplicate index creation (IF NOT EXISTS), might not create system updates
567+
$this->assertGreaterThanOrEqual(0, $counters->systemUpdates());
568+
// containsSystemUpdates should be consistent with systemUpdates count
569+
$this->assertEquals($counters->systemUpdates() > 0, $counters->containsSystemUpdates());
570+
571+
$result2 = $this->getSession()->run('DROP INDEX non_existent_test_index IF EXISTS');
572+
573+
$summary2 = $result2->getSummary();
574+
$counters2 = $summary2->getCounters();
575+
576+
// Dropping non-existent index should not create system updates
577+
$this->assertEquals(0, $counters2->systemUpdates());
578+
$this->assertFalse($counters2->containsSystemUpdates());
579+
580+
$this->getSession()->run('DROP INDEX duplicate_test_index IF EXISTS');
581+
}
582+
583+
public function testMultipleSystemOperationsForBug(): void
584+
{
585+
$operations = [
586+
'CREATE INDEX multi_test_1 IF NOT EXISTS FOR (n:MultiTestNode) ON (n.prop1)',
587+
'CREATE INDEX multi_test_2 IF NOT EXISTS FOR (n:MultiTestNode) ON (n.prop2)',
588+
'CREATE CONSTRAINT multi_test_constraint IF NOT EXISTS FOR (n:MultiTestNode) REQUIRE n.id IS UNIQUE',
589+
'DROP INDEX multi_test_1 IF EXISTS',
590+
'DROP INDEX multi_test_2 IF EXISTS',
591+
'DROP CONSTRAINT multi_test_constraint IF EXISTS',
592+
];
593+
594+
foreach ($operations as $operation) {
595+
$result = $this->getSession()->run($operation);
596+
597+
$summary = $result->getSummary();
598+
$counters = $summary->getCounters();
599+
600+
// Test that system operations properly track system updates
601+
$this->assertGreaterThanOrEqual(0, $counters->systemUpdates());
602+
// Verify consistency between systemUpdates count and containsSystemUpdates flag
603+
$this->assertEquals($counters->systemUpdates() > 0, $counters->containsSystemUpdates());
604+
}
605+
}
606+
607+
public function testRegularDataOperationsStillWork(): void
608+
{
609+
$result = $this->getSession()->run('CREATE (n:RegularTestNode {name: "test", id: $id}) RETURN n', ['id' => bin2hex(random_bytes(8))]);
610+
611+
$summary = $result->getSummary();
612+
$counters = $summary->getCounters();
613+
614+
// Regular data operations should not involve system updates
615+
$this->assertEquals(0, $counters->systemUpdates());
616+
$this->assertFalse($counters->containsSystemUpdates());
617+
618+
$this->getSession()->run('MATCH (n:RegularTestNode) DELETE n');
619+
}
524620
}

0 commit comments

Comments
 (0)