Skip to content

Commit 1a42a5a

Browse files
p123-stackp123-stack
committed
Fixed issue TypeError in SummaryCounters constructor when systemUpdates is false
1 parent de6a404 commit 1a42a5a

File tree

2 files changed

+122
-10
lines changed

2 files changed

+122
-10
lines changed

‎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
);

‎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)