Skip to content

Commit 2116ae6

Browse files
malbertsclaude
andcommitted
Bind the context parse to the revision and clarify provider docs
The parse that fills categories and parserProperties ran without a revision id, i.e. as a preview-like parse: revision-context magic words ({{REVISIONID}}, {{REVISIONUSER}}, ...) and parser hooks that read the revision while recording page properties resolved against nothing, so the recorded values could diverge from what core stored for the same revision. That mattered little while the parse only fed categories, but the parse products are now published to providers as the recommended derivation path. The fix passes the revision id through ContentParseParams. The new test pins the behavior with a {{DEFAULTSORT:Rev{{REVISIONID}}}} page: it records Rev without the fix and Rev<id> with it, and a second stored revision ensures resolving the page's latest revision instead of the provided one also fails. The content-unavailable sentinels (content and contentModel empty, parserProperties and categories empty when the main-slot content is hidden) had no covering test, so type-clean drift such as switching the builder to getContentOrThrow would pass the whole suite while making undeletion of a page with rev-deleted history fatal in production; the new test suppresses a real revision and asserts what a provider receives. The contentModel test only exercised wikitext, the one value a hardcode would also produce, so a JSON page now pins that the model comes from the actual content. In extending.md, the sentence introducing the context now names the content and parse products, and the section states when providers actually run: when a revision is stored for a page carrying Subject data (including undeletions) and during RebuildGraphDatabases runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ec599b9 commit 2116ae6

3 files changed

Lines changed: 48 additions & 7 deletions

File tree

docs/reference/extending.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ $registrar->addNeo4jValueBuilder( ColorType::NAME, static fn ( $value ) => $valu
7474
### Page Property Providers
7575

7676
Page Property Providers contribute key/value metadata to the Page node in the graph (queryable via Cypher;
77-
Neo4j is currently the only graph backend). Implement `PagePropertyProvider`:
77+
Neo4j is currently the only graph backend). They run when a revision is stored for a page carrying Subject
78+
data (including undeletions), and again for such pages when the graph is rebuilt with the
79+
`RebuildGraphDatabases` maintenance script. Pages without Subject data are not stored in the graph, so
80+
providers are never invoked for them. Implement `PagePropertyProvider`:
7881

7982
```php
8083
class StaticPagePropertyProvider implements PagePropertyProvider {
@@ -87,14 +90,14 @@ class StaticPagePropertyProvider implements PagePropertyProvider {
8790
```
8891

8992
Register with `NeoWikiRegistrar::addPagePropertyProvider()`. The context exposes the page id, title,
90-
creation and modification times, categories, and last editor, so providers can derive Page Properties
91-
from the page content without re-fetching or re-parsing it.
93+
creation and modification times, categories, and last editor, as well as the page content and its parse
94+
products, so providers can derive Page Properties from the page content without re-fetching or re-parsing it.
9295

93-
To derive Page Properties from the content, prefer the parse products: `categories`, and
96+
To derive Page Properties from the content, prefer the parse products: `categories` and
9497
`parserProperties` — the MediaWiki page properties recorded during parsing (e.g. those a parser hook
9598
sets via `ParserOutput::setPageProperty`). These are template-expansion-safe and robust. (Note that
9699
`parserProperties` are an input from MediaWiki's parse; they are not the NeoWiki Page Properties this
97-
provider returns.) The raw main slot `content` and its `contentModel` are also exposed, but scraping
100+
provider returns.) The raw main slot `content` and its `contentModel` are the fallback: scraping
98101
raw wikitext is fragile — reach for them mainly when handling a custom, non-wikitext content model that
99102
the parse products do not cover. Example:
100103
[`src/StaticPagePropertyProvider.php`](https://github.com/ProfessionalWiki/NeoWiki/blob/master/tests/RedHerb/src/StaticPagePropertyProvider.php).

src/PagePropertiesBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private function buildContext( RevisionRecord $revision, ?UserIdentity $user ):
6161

6262
private function parse( Content $content, RevisionRecord $revision ): ParserOutput {
6363
return $this->contentHandlerFactory->getContentHandler( $content->getModel() )
64-
->getParserOutput( $content, new ContentParseParams( $revision->getPage() ) );
64+
->getParserOutput( $content, new ContentParseParams( $revision->getPage(), $revision->getId() ) );
6565
}
6666

6767
private function getCreationTime( RevisionRecord $revision ): string {

tests/phpunit/PagePropertiesBuilderTest.php

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace ProfessionalWiki\NeoWiki\Tests;
66

7+
use MediaWiki\Revision\RevisionRecord;
78
use ProfessionalWiki\NeoWiki\Domain\Page\PagePropertyProviderContext;
89
use ProfessionalWiki\NeoWiki\Domain\Page\PagePropertyProviderRegistry;
910
use ProfessionalWiki\NeoWiki\PagePropertiesBuilder;
@@ -39,9 +40,46 @@ public function testProviderReceivesCategoriesFromParsedContent(): void {
3940
$this->assertSame( [ 'Cats' ], $context->categories );
4041
}
4142

43+
public function testParseIsBoundToTheProvidedRevision(): void {
44+
$revision = $this->editPage( 'PagePropertiesBuilderTestPage', '{{DEFAULTSORT:Rev{{REVISIONID}}}}' )->getNewRevision();
45+
$this->editPage( 'PagePropertiesBuilderTestPage', 'Newer revision without a defaultsort' );
46+
47+
$this->assertSame(
48+
'Rev' . $revision->getId(),
49+
$this->getContextForRevision( $revision )->parserProperties['defaultsort']
50+
);
51+
}
52+
53+
public function testProviderReceivesContentModelOfNonWikitextContent(): void {
54+
$revision = $this->editPage( 'MediaWiki:PagePropertiesBuilderTest.json', '{ "answer": 42 }' )->getNewRevision();
55+
56+
$this->assertSame( CONTENT_MODEL_JSON, $this->getContextForRevision( $revision )->contentModel );
57+
}
58+
59+
public function testContextHasEmptySentinelsWhenContentIsUnavailable(): void {
60+
$revision = $this->editPage( 'PagePropertiesBuilderTestPage', 'Hidden [[Category:Cats]] {{DEFAULTSORT:Zebra}}' )->getNewRevision();
61+
$this->editPage( 'PagePropertiesBuilderTestPage', 'Newer public revision' );
62+
$this->revisionDelete( $revision );
63+
64+
$context = $this->getContextForRevision( $this->getSuppressedRevision( $revision->getId() ) );
65+
66+
$this->assertSame( '', $context->content );
67+
$this->assertSame( '', $context->contentModel );
68+
$this->assertSame( [], $context->parserProperties );
69+
$this->assertSame( [], $context->categories );
70+
}
71+
72+
private function getSuppressedRevision( int $revisionId ): RevisionRecord {
73+
return $this->getServiceContainer()->getRevisionStore()->getRevisionById( $revisionId );
74+
}
75+
4276
private function getContextForNewPageWithContent( string $wikitext ): PagePropertyProviderContext {
43-
$revision = $this->editPage( 'PagePropertiesBuilderTestPage', $wikitext )->getNewRevision();
77+
return $this->getContextForRevision(
78+
$this->editPage( 'PagePropertiesBuilderTestPage', $wikitext )->getNewRevision()
79+
);
80+
}
4481

82+
private function getContextForRevision( RevisionRecord $revision ): PagePropertyProviderContext {
4583
$spy = new SpyPagePropertyProvider();
4684

4785
$this->newPagePropertiesBuilder( $spy )->getPagePropertiesFor( $revision, null );

0 commit comments

Comments
 (0)