Skip to content

Commit 75a64de

Browse files
alistair3149claude
andcommitted
Cover main/child flips, self-referencing removal, and relation ids
Follow-up to #1080. - testFlippingASubjectBetweenMainAndChildLeavesASingleHasSubjectEdge locks detachSubjectsFromPage: without it, flipping a subject's isMain flag leaves a duplicate HasSubject edge (verified red when the call is removed). - testRemovingASelfReferencingSubjectDeletesItRatherThanStubbingIt guards the self-loop deletion. - assertRelationExists now optionally asserts the preserved relation's id, so a regression that keeps an incoming relation by endpoints and type but drops its id is caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 172037f commit 75a64de

1 file changed

Lines changed: 69 additions & 7 deletions

File tree

tests/phpunit/GraphDatabasePlugins/Neo4j/Persistence/Neo4jProjectionStoreTest.php

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ public function testSavingPageWithoutAReferencedSubjectPreservesIncomingRelation
305305

306306
$store->savePage( TestPage::build( id: 1 ) );
307307

308-
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1 );
308+
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1, 'rTestNQS1111rr1' );
309309
}
310310

311311
public function testSavingPageWithoutAReferencedSubjectReducesItToAStub(): void {
@@ -361,7 +361,7 @@ public function testDeletingPageReducesAReferencedSubjectToAStub(): void {
361361
$store->deletePage( new PageId( 1 ) );
362362

363363
$this->assertSubjectIsStub( self::GUID_1 );
364-
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1 );
364+
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1, 'rTestNQS1111rr1' );
365365
}
366366

367367
public function testSavingASubjectUpgradesItsStubInPlace(): void {
@@ -390,7 +390,7 @@ public function testSavingASubjectUpgradesItsStubInPlace(): void {
390390

391391
$this->assertSame( [ 'Subject', TestSubject::DEFAULT_SCHEMA_ID ], $labels );
392392
$this->assertSame( 'Real subject', $row['name'] );
393-
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1 );
393+
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1, 'rTestNQS1111rr1' );
394394
}
395395

396396
public function testReducingReferencedSubjectToStubKeepsIncomingRelationsButStripsOutgoingRelationsAndProperties(): void {
@@ -435,8 +435,55 @@ public function testReducingReferencedSubjectToStubKeepsIncomingRelationsButStri
435435

436436
$this->assertSubjectIsStub( self::GUID_1 );
437437
$this->assertOutgoingRelationCount( self::GUID_1, 0 );
438-
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1 );
439-
$this->assertRelationExists( self::GUID_5, 'LocatedIn', self::GUID_1 );
438+
$this->assertRelationExists( self::GUID_2, 'LocatedIn', self::GUID_1, 'rTestNQS1111rCA' );
439+
$this->assertRelationExists( self::GUID_5, 'LocatedIn', self::GUID_1, 'rTestNQS1111rDA' );
440+
}
441+
442+
public function testFlippingASubjectBetweenMainAndChildLeavesASingleHasSubjectEdge(): void {
443+
$store = $this->newProjectionStore();
444+
445+
$store->savePage( TestPage::build(
446+
id: 42,
447+
mainSubject: TestSubject::build( id: self::GUID_1 ),
448+
childSubjects: new SubjectMap(
449+
TestSubject::build( id: self::GUID_2 ),
450+
)
451+
) );
452+
453+
// Swap the roles: GUID_2 becomes the main subject and GUID_1 becomes a child.
454+
// The HasSubject relation carries the isMain flag, so re-saving must not leave a
455+
// second, stale HasSubject edge behind for either subject.
456+
$store->savePage( TestPage::build(
457+
id: 42,
458+
mainSubject: TestSubject::build( id: self::GUID_2 ),
459+
childSubjects: new SubjectMap(
460+
TestSubject::build( id: self::GUID_1 ),
461+
)
462+
) );
463+
464+
$this->assertPageHasSubjects(
465+
[
466+
[ 'id' => self::GUID_1, 'hs' => [ 'isMain' => false ] ],
467+
[ 'id' => self::GUID_2, 'hs' => [ 'isMain' => true ] ],
468+
],
469+
42
470+
);
471+
}
472+
473+
public function testRemovingASelfReferencingSubjectDeletesItRatherThanStubbingIt(): void {
474+
$store = $this->newProjectionStoreWithLocationRelation();
475+
476+
// GUID_1 holds a relation to itself and nothing else references it.
477+
$store->savePage( TestPage::build(
478+
id: 1,
479+
mainSubject: $this->buildSubjectWithLocationRelation( self::GUID_1, self::GUID_1, 'rTestNQS1111rr1' ),
480+
) );
481+
482+
// Removing it from its page must delete it: a self-loop is not an external reference,
483+
// so keeping it as a stub would leave an unreachable orphan node.
484+
$store->savePage( TestPage::build( id: 1 ) );
485+
486+
$this->assertSubjectDoesNotExist( self::GUID_1 );
440487
}
441488

442489
private function newProjectionStoreWithLocationRelation( string $wikiId = self::WIKI_ID ): GraphDatabasePlugin {
@@ -481,16 +528,31 @@ private function buildSubjectWithLocationRelation( string $id, string $targetId,
481528
);
482529
}
483530

484-
private function assertRelationExists( string $fromSubjectId, string $relationType, string $toSubjectId ): void {
531+
private function assertRelationExists(
532+
string $fromSubjectId,
533+
string $relationType,
534+
string $toSubjectId,
535+
?string $expectedRelationId = null
536+
): void {
485537
$result = $this->readGraph(
486-
'MATCH ({id: $from})-[relation:' . $relationType . ']->({id: $to}) RETURN relation',
538+
'MATCH ({id: $from})-[relation:' . $relationType . ']->({id: $to}) RETURN relation.id AS id',
487539
[ 'from' => $fromSubjectId, 'to' => $toSubjectId ]
488540
);
489541

490542
$this->assertFalse(
491543
$result->isEmpty(),
492544
"Relation {$fromSubjectId}-[{$relationType}]->{$toSubjectId} should exist"
493545
);
546+
547+
// Readers reconcile relations by their id, so a relation that survives by endpoints and type
548+
// but loses its id is broken. Assert the id is preserved when the caller pins it down.
549+
if ( $expectedRelationId !== null ) {
550+
$this->assertSame(
551+
$expectedRelationId,
552+
$result->first()->toRecursiveArray()['id'],
553+
"Relation {$fromSubjectId}-[{$relationType}]->{$toSubjectId} should keep its id"
554+
);
555+
}
494556
}
495557

496558
private function subjectHasProperty( string $subjectId, string $property ): bool {

0 commit comments

Comments
 (0)