Skip to content

Commit f077289

Browse files
JeroenDeDauwclaude
JeroenDeDauw
and
claude
committed
Merge origin/master into docs/text-pass-fleet
Resolve the graph-model.md conflict from #1080 (stub Subject nodes). The branch's rewrite claimed a removed-but-referenced node "keeps its Schema labels and dynamic properties"; #1080 changed the code to strip such nodes to id + wiki_id + Subject-label stubs, making that claim false. Take master's Stub-Subject-nodes section and its corrected one-line deletion paragraph; the rest of the branch's rewrite stands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents b90dccb + cdba2dd commit f077289

10 files changed

Lines changed: 626 additions & 126 deletions

File tree

‎docs/api/graph-model.md‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,23 @@ Values a PropertyType cannot represent are dropped from the projection: a proper
8282
PropertyType produces no node property, and non-ISO 8601 `date`/`dateTime` parts are omitted from the stored value.
8383
The revision slot stays authoritative.
8484

85+
### Stub Subject nodes
86+
87+
A Subject node can exist as a *stub*: a node stripped down to only its `id` and `wiki_id` properties and the
88+
`Subject` label — no `name`, no Schema label, and no Statement-derived properties. A stub keeps a Subject's identity
89+
available for incoming relations while carrying none of its data.
90+
91+
Stubs arise in two ways:
92+
93+
- **Referenced but removed.** When a Subject is removed from its page (or its page is deleted) but other Subjects
94+
still hold relations to it, its node is reduced to a stub rather than deleted, and its `HasSubject` and outgoing
95+
relationships are removed. This keeps the incoming references valid.
96+
- **Referenced but not yet created.** When a Relation targets a Subject that does not exist yet, a stub target node
97+
is created so the relationship can be stored.
98+
99+
When the real Subject is later saved, its node is upgraded in place — matched by `id` alone, so the stub gains its
100+
properties and Schema label without creating a duplicate node.
101+
85102
## Relationships
86103

87104
### HasSubject
@@ -106,10 +123,8 @@ backtick-escaped.
106123
| `id` | string | Relation ID, 15 characters starting with `r` |
107124
| *(additional)* | scalar | Any properties from the Relation's property map |
108125

109-
When a Subject is deleted but still has incoming relations from other Subjects, its outgoing relationships and
110-
`HasSubject` relationship are removed, but the node itself is kept so incoming references stay valid. Such a retained
111-
node keeps its Schema labels and dynamic properties; the absence of an incoming `HasSubject` relationship distinguishes
112-
it from a live Subject.
126+
When a Subject with incoming relations from other Subjects is removed, its node is kept as a
127+
[stub](#stub-subject-nodes) so those incoming references remain valid.
113128

114129
## Constraints
115130

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,16 +195,6 @@ parameters:
195195
count: 1
196196
path: src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jQueryStore.php
197197

198-
-
199-
message: "#^Cannot call method get\\(\\) on mixed\\.$#"
200-
count: 1
201-
path: src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jSubjectUpdater.php
202-
203-
-
204-
message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\GraphDatabasePlugins\\\\Neo4j\\\\Persistence\\\\Neo4jSubjectUpdater\\:\\:getNodeLabels\\(\\) should return array\\<string\\> but returns array\\<int, mixed\\>\\.$#"
205-
count: 1
206-
path: src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jSubjectUpdater.php
207-
208198
-
209199
message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#"
210200
count: 1

‎src/EntryPoints/Content/SubjectContentHandler.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use MediaWiki\Content\Content;
88
use MediaWiki\Content\JsonContentHandler;
99
use MediaWiki\Content\Renderer\ContentParseParams;
10+
use MediaWiki\Title\Title;
1011
use MediaWiki\Parser\ParserOutput;
1112

1213
class SubjectContentHandler extends JsonContentHandler {
@@ -27,4 +28,13 @@ protected function fillParserOutput(
2728
$parserOutput->setRawText( '' );
2829
}
2930

31+
/**
32+
* Subject content is only ever valid inside the dedicated subject slot, whose fixed content model is
33+
* enforced by the slot role rather than by this method. It is never a page's main content model, so
34+
* it must not be selectable as one.
35+
*/
36+
public function canBeUsedOn( Title $title ): bool {
37+
return false;
38+
}
39+
3040
}

‎src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jNodeLabels.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare( strict_types = 1 );
4+
5+
namespace ProfessionalWiki\NeoWiki\GraphDatabasePlugins\Neo4j\Persistence;
6+
7+
use Laudis\Neo4j\Contracts\TransactionInterface;
8+
use Laudis\Neo4j\Databags\SummarizedResult;
9+
10+
/**
11+
* Reads and removes the labels of a Neo4j node identified by its id, within a given transaction.
12+
*
13+
* Shared by Neo4jSubjectUpdater (reconciling a subject's labels with its schema) and
14+
* Neo4jProjectionStore (stripping a subject down to a stub). Those classes hold their transaction
15+
* differently, so the transaction is passed in per call rather than owned here.
16+
*/
17+
class Neo4jNodeLabels {
18+
19+
/**
20+
* @return string[]
21+
*/
22+
public static function read( TransactionInterface $transaction, string $nodeId ): array {
23+
/**
24+
* @var SummarizedResult $result
25+
*/
26+
$result = $transaction->run(
27+
'MATCH (n {id: $id}) RETURN labels(n) AS labels',
28+
[ 'id' => $nodeId ]
29+
);
30+
31+
if ( $result->isEmpty() ) {
32+
return [];
33+
}
34+
35+
return $result->first()->get( 'labels' )->toArray();
36+
}
37+
38+
/**
39+
* @param string[] $labels
40+
*/
41+
public static function remove( TransactionInterface $transaction, string $nodeId, array $labels ): void {
42+
if ( $labels === [] ) {
43+
return;
44+
}
45+
46+
$transaction->run(
47+
'MATCH (n {id: $id}) REMOVE n:' . Cypher::buildLabelList( $labels ),
48+
[ 'id' => $nodeId ]
49+
);
50+
}
51+
52+
}

‎src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jProjectionStore.php‎

Lines changed: 145 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -32,52 +32,66 @@ public function initialize(): void {
3232

3333
public function savePage( Page $page ): void {
3434
$this->client->writeTransaction( function ( TransactionInterface $transaction ) use ( $page ): void {
35-
$properties = $page->getProperties()->asArray();
36-
[ $typedSetClauses, $typedParams, $properties ] = $this->extractTypedValues( $properties );
37-
38-
$cypher = '
39-
// Create or update the page. Page identity is scoped per wiki so that
40-
// pages from different wikis sharing the same id do not collide in a shared graph.
41-
// The wiki_id is persisted by the MERGE pattern itself.
42-
MERGE (page:Page {id: $pageId, wiki_id: $wikiId})
43-
SET page += $properties
44-
SET page.id = $pageId';
45-
46-
if ( $typedSetClauses !== '' ) {
47-
$cypher .= ',' . $typedSetClauses;
48-
}
49-
50-
$cypher .= '
51-
52-
// Delete subjects that are no longer present on the page
53-
WITH page
54-
MATCH (page)-[r:HasSubject]->(subject)
55-
WHERE NOT subject.id IN $subjectIds
56-
DETACH DELETE subject
57-
58-
// Remove all existing HasSubject relations
59-
WITH page
60-
MATCH (page)-[r:HasSubject]->()
61-
DELETE r
62-
';
63-
64-
$transaction->run(
65-
$cypher,
66-
array_merge(
67-
[
68-
'pageId' => $page->getId()->id,
69-
'wikiId' => $this->wikiId,
70-
'subjectIds' => $page->getSubjects()->getAllSubjects()->getIdsAsTextArray(),
71-
'properties' => new CypherMap( $properties ),
72-
],
73-
$typedParams,
74-
)
75-
);
76-
35+
$this->upsertPageNode( $transaction, $page );
36+
$this->removeAbsentSubjects( $transaction, $page );
37+
$this->detachSubjectsFromPage( $transaction, $page->getId() );
7738
$this->updateSubjects( $transaction, $page );
7839
} );
7940
}
8041

42+
private function upsertPageNode( TransactionInterface $transaction, Page $page ): void {
43+
$properties = $page->getProperties()->asArray();
44+
[ $typedSetClauses, $typedParams, $properties ] = $this->extractTypedValues( $properties );
45+
46+
// Create or update the page. Page identity is scoped per wiki so that pages from different
47+
// wikis sharing the same id do not collide in a shared graph. The wiki_id is persisted by
48+
// the MERGE pattern itself.
49+
$cypher = '
50+
MERGE (page:Page {id: $pageId, wiki_id: $wikiId})
51+
SET page += $properties
52+
SET page.id = $pageId';
53+
54+
if ( $typedSetClauses !== '' ) {
55+
$cypher .= ',' . $typedSetClauses;
56+
}
57+
58+
$transaction->run(
59+
$cypher,
60+
array_merge(
61+
[
62+
'pageId' => $page->getId()->id,
63+
'wikiId' => $this->wikiId,
64+
'properties' => new CypherMap( $properties ),
65+
],
66+
$typedParams,
67+
)
68+
);
69+
}
70+
71+
/**
72+
* Removes the subjects that are attached to the page in the graph but are no longer present on the
73+
* page. This reuses removeSubjects so that a removed subject still referenced by other subjects is
74+
* kept as a stub instead of being deleted, keeping the incoming relations valid.
75+
*/
76+
private function removeAbsentSubjects( TransactionInterface $transaction, Page $page ): void {
77+
$presentSubjectIds = $page->getSubjects()->getAllSubjects()->getIdsAsTextArray();
78+
79+
$absentSubjectIds = array_values( array_filter(
80+
$this->getSubjectIdsByPageId( $transaction, $page->getId() ),
81+
fn( string $subjectId ) => !in_array( $subjectId, $presentSubjectIds, true )
82+
) );
83+
84+
$this->removeSubjects( $transaction, $absentSubjectIds );
85+
}
86+
87+
private function detachSubjectsFromPage( TransactionInterface $transaction, PageId $pageId ): void {
88+
$transaction->run(
89+
'MATCH (page:Page {id: $pageId, wiki_id: $wikiId})-[hasSubject:HasSubject]->()
90+
DELETE hasSubject',
91+
[ 'pageId' => $pageId->id, 'wikiId' => $this->wikiId ]
92+
);
93+
}
94+
8195
/**
8296
* Extracts PageValue instances from the property map and converts them
8397
* to Cypher SET clauses with parameterized values.
@@ -140,9 +154,7 @@ private function updateSubjects( TransactionInterface $transaction, Page $page )
140154

141155
public function deletePage( PageId $pageId ): void {
142156
$this->client->writeTransaction( function ( TransactionInterface $transaction ) use ( $pageId ): void {
143-
foreach ( $this->getSubjectIdsByPageId( $transaction, $pageId ) as $subjectId ) {
144-
$this->deleteSubject( $transaction, new SubjectId( $subjectId ) );
145-
}
157+
$this->removeSubjects( $transaction, $this->getSubjectIdsByPageId( $transaction, $pageId ) );
146158

147159
$this->deletePageNode( $transaction, $pageId );
148160
} );
@@ -157,7 +169,6 @@ private function deletePageNode( TransactionInterface $transaction, PageId $page
157169
}
158170

159171
/**
160-
* FIXME: tests still pass if this function returns an empty array
161172
* @return string[]
162173
*/
163174
private function getSubjectIdsByPageId( TransactionInterface $transaction, PageId $pageId ): array {
@@ -166,7 +177,7 @@ private function getSubjectIdsByPageId( TransactionInterface $transaction, PageI
166177
*/
167178
$results = $transaction->run(
168179
'MATCH (page:Page {id: $pageId, wiki_id: $wikiId})-[:HasSubject]->(subject:Subject)
169-
RETURN subject.id AS id, subject AS properties, labels(subject) AS labels',
180+
RETURN subject.id AS id',
170181
[ 'pageId' => $pageId->id, 'wikiId' => $this->wikiId ]
171182
);
172183

@@ -176,37 +187,97 @@ private function getSubjectIdsByPageId( TransactionInterface $transaction, PageI
176187
);
177188
}
178189

179-
private function deleteSubject( TransactionInterface $transaction, SubjectId $subjectId ): void {
180-
if ( $this->subjectHasIncomingRelations( $transaction, $subjectId ) ) {
181-
// Only remove HasSubject relations and outgoing relations.
182-
// Keep the subject node itself because other subjects still reference it.
183-
$transaction->run(
184-
'
185-
MATCH ()-[hsRelation:HasSubject]->(subject {id: $subjectId})
186-
OPTIONAL MATCH (subject)-[outgoingSubjectRelation]->(o)
187-
DELETE hsRelation, outgoingSubjectRelation
188-
',
189-
[ 'subjectId' => $subjectId->text ]
190-
);
191-
// TODO: clear properties?
192-
// TODO: clear labels?
190+
/**
191+
* Removes the given subjects from the graph. A subject still referenced by an incoming relation from
192+
* another subject is reduced to a stub so that reference stays valid; the rest are deleted outright.
193+
*
194+
* The referenced/unreferenced split is computed with a single query and the unreferenced subjects are
195+
* deleted with a single query, rather than one round trip per subject.
196+
*
197+
* @param string[] $subjectIds
198+
*/
199+
private function removeSubjects( TransactionInterface $transaction, array $subjectIds ): void {
200+
if ( $subjectIds === [] ) {
201+
return;
193202
}
194-
else {
195-
$transaction->run(
196-
'MATCH (subject {id: $subjectId})
197-
DETACH DELETE subject',
198-
[ 'subjectId' => $subjectId->text ]
199-
);
203+
204+
$referencedSubjectIds = $this->subjectIdsWithIncomingRelations( $transaction, $subjectIds );
205+
206+
$this->deleteSubjects( $transaction, array_values( array_diff( $subjectIds, $referencedSubjectIds ) ) );
207+
208+
foreach ( $referencedSubjectIds as $subjectId ) {
209+
$this->reduceSubjectToStub( $transaction, new SubjectId( $subjectId ) );
200210
}
201211
}
202212

203-
private function subjectHasIncomingRelations( TransactionInterface $transaction, SubjectId $subjectId ): bool {
204-
return $transaction->run(
205-
'MATCH (subject {id: $subjectId})<-[incomingRelation]-()
206-
WHERE NOT incomingRelation:HasSubject
207-
RETURN incomingRelation',
208-
[ 'subjectId' => $subjectId->text ]
209-
)->isEmpty() === false;
213+
/**
214+
* Returns the subset of the given subject ids that still have an incoming relation from a *different*
215+
* subject. HasSubject relations do not count, and neither does a self-loop: a subject whose only
216+
* incoming relation is its own outgoing self-reference has no external referrer, so it is deleted
217+
* rather than kept as an unreachable stub.
218+
*
219+
* @param string[] $subjectIds
220+
* @return string[]
221+
*/
222+
private function subjectIdsWithIncomingRelations( TransactionInterface $transaction, array $subjectIds ): array {
223+
/**
224+
* @var SummarizedResult $result
225+
*/
226+
$result = $transaction->run(
227+
'UNWIND $subjectIds AS subjectId
228+
MATCH (subject {id: subjectId})<-[incomingRelation]-(other)
229+
WHERE NOT incomingRelation:HasSubject AND other <> subject
230+
RETURN DISTINCT subject.id AS id',
231+
[ 'subjectIds' => $subjectIds ]
232+
);
233+
234+
return array_map(
235+
fn( $record ) => $record->get( 'id' ),
236+
$result->toArray()
237+
);
238+
}
239+
240+
/**
241+
* @param string[] $subjectIds
242+
*/
243+
private function deleteSubjects( TransactionInterface $transaction, array $subjectIds ): void {
244+
if ( $subjectIds === [] ) {
245+
return;
246+
}
247+
248+
$transaction->run(
249+
'MATCH (subject) WHERE subject.id IN $subjectIds DETACH DELETE subject',
250+
[ 'subjectIds' => $subjectIds ]
251+
);
252+
}
253+
254+
/**
255+
* Reduces a subject node to a stub: it is detached from its page and its outgoing relations, and
256+
* stripped down to only the id and wiki_id properties and the Subject label. The incoming relations
257+
* from other subjects are kept, so the stub keeps those references valid. The stub is upgraded back
258+
* to a full subject in place if the subject is saved again.
259+
*/
260+
private function reduceSubjectToStub( TransactionInterface $transaction, SubjectId $subjectId ): void {
261+
$transaction->run(
262+
'MATCH (subject {id: $subjectId})
263+
OPTIONAL MATCH ()-[hasSubject:HasSubject]->(subject)
264+
OPTIONAL MATCH (subject)-[outgoingRelation]->()
265+
DELETE hasSubject, outgoingRelation
266+
WITH DISTINCT subject
267+
SET subject = {id: $subjectId, wiki_id: $wikiId}
268+
SET subject:Subject',
269+
[ 'subjectId' => $subjectId->text, 'wikiId' => $this->wikiId ]
270+
);
271+
272+
$this->removeNonStubLabels( $transaction, $subjectId );
273+
}
274+
275+
private function removeNonStubLabels( TransactionInterface $transaction, SubjectId $subjectId ): void {
276+
Neo4jNodeLabels::remove(
277+
$transaction,
278+
$subjectId->text,
279+
array_diff( Neo4jNodeLabels::read( $transaction, $subjectId->text ), [ 'Subject' ] )
280+
);
210281
}
211282

212283
}

0 commit comments

Comments
 (0)