Skip to content

Commit fb4f610

Browse files
gesinn-it-geagesinn-it-gea
committed
refactor(api): extract shared trait for the three KnowledgeGraphApiLoad* endpoints
KnowledgeGraphApiLoadNodes, KnowledgeGraphApiLoadProperties, and KnowledgeGraphApiLoadCategories duplicated the same execute() shape, which had already produced identical copy-paste bugs (see #92) across two of the three classes. New KnowledgeGraphApiLoadTrait centralizes the shared init/loop/response logic; each class now only implements getTitlesToLoad() and getPropertiesForTitle().
1 parent 2471c2d commit fb4f610

6 files changed

Lines changed: 176 additions & 167 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](https://semver.org/) and
1313
- `$wgKnowledgeGraphListSeparator` config variable (default `,`): the `nodes=`/`properties=` parser-function parameters were always split on a hard-coded comma via `KnowledgeGraph::applyDefaultParams()`, which incorrectly fragmented page titles or property names that themselves contain a comma (e.g. legal citations, bibliographic entries). The separator is now configurable per wiki, analogous to `$wgPageFormsListSeparator` ([#33](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/33), [#48](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/48)). Note: PR [#50](https://github.com/SemanticMediaWiki/KnowledgeGraph/pull/50) proposed a different fix for #48 (reassembling comma-split fragments by probing `Title::isKnown()`), which only papers over the symptom for titles that already exist and doesn't address the underlying hard-coded separator raised in #33; this change addresses the root cause instead.
1414

1515
### Changed
16+
- `KnowledgeGraphApiLoadNodes`, `KnowledgeGraphApiLoadProperties`, and `KnowledgeGraphApiLoadCategories` (`includes/api/`) shared the same `execute()` shape (init SMW, resolve each requested title/category-member to semantic data via `KnowledgeGraph::setSemanticDataFromApi()`, `json_encode()` the result, `addValue()`) independently in three places — the same structure that had already produced identical copy-paste bugs across two of the three classes (both referencing the undeclared `self::$data` instead of `\KnowledgeGraph::$data`, see #92 above). Extracted the shared shape into a new `KnowledgeGraphApiLoadTrait` (`isWriteMode()`, `mustBePosted()`, `needsToken()`, `execute()`); each class now implements only two small abstract hooks, `getTitlesToLoad()` (which titles/category members to process) and `getPropertiesForTitle()` (which properties to load for a given title), with `KnowledgeGraphApiLoadCategories` also keeping its per-class `buildPropertiesList()` and its running properties accumulator (now an instance property instead of a value threaded through local variables) needed to carry accumulated properties across category members within one request. Purely structural — no externally observable change to any endpoint's response shape ([#93](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/93))
1617
- `KnowledgeGraph::setSemanticDataFromApi()` no longer reads/writes the public static `KnowledgeGraph::$data` or the private static `KnowledgeGraph::$relationsSeen` as an implicit communication channel with its callers; it now takes `$data`/`$relationsSeen` as by-reference parameters (accumulating across its own recursion) and also returns `$data`, so a call's result is explicit rather than something callers had to read back out of shared static state afterwards. `KnowledgeGraphApiLoadNodes`, `KnowledgeGraphApiLoadProperties`, and `KnowledgeGraphApiLoadCategories` (as well as `KnowledgeGraph::parserFunctionKnowledgeGraph()`) now each thread their own local `$data`/`$relationsSeen` through the call instead of relying on `\KnowledgeGraph::$data`/external reset discipline; this also fixes `KnowledgeGraphApiLoadCategories::execute()`'s pre-existing `self::$data` reference (a class with no such declared property, working only because the read is guarded by `isset()`) and removes the class of fatal ("Access to undeclared static property") this pattern had already caused twice before (see #68, #69 above). The now-unused `KnowledgeGraph::resetSeenRelations()` test-support method (and its dedicated test) was removed, since resetting shared state between calls/tests is no longer necessary ([#92](https://github.com/SemanticMediaWiki/KnowledgeGraph/issues/92))
1718

1819
### Fixed

‎extension.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"AutoloadClasses": {
2222
"KnowledgeGraph":"includes/KnowledgeGraph.php",
2323
"SpecialKnowledgeGraphDesigner":"includes/specials/SpecialKnowledgeGraphDesigner.php",
24+
"KnowledgeGraphApiLoadTrait": "includes/api/KnowledgeGraphApiLoadTrait.php",
2425
"KnowledgeGraphApiLoadNodes": "includes/api/KnowledgeGraphApiLoadNodes.php",
2526
"KnowledgeGraphApiLoadProperties": "includes/api/KnowledgeGraphApiLoadProperties.php",
2627
"KnowledgeGraphApiLoadCategories": "includes/api/KnowledgeGraphApiLoadCategories.php"

‎includes/api/KnowledgeGraphApiLoadCategories.php‎

Lines changed: 50 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
class KnowledgeGraphApiLoadCategories extends ApiBase {
1313

14+
use KnowledgeGraphApiLoadTrait;
15+
1416
/**
1517
* Store instance for Semantic MediaWiki data.
1618
*
@@ -26,29 +28,28 @@ class KnowledgeGraphApiLoadCategories extends ApiBase {
2628
protected static $SMWDataValueFactory = null;
2729

2830
/**
29-
* @inheritDoc
31+
* Names (without namespace prefix) of all known Property: pages, used by
32+
* buildPropertiesList() to find properties for which a category member is
33+
* a target value. Populated once per execute() call.
34+
*
35+
* @var string[]
3036
*/
31-
public function isWriteMode() {
32-
return false;
33-
}
37+
private $propertyNames = [];
3438

3539
/**
36-
* @inheritDoc
40+
* Running list of properties accumulated across category members already
41+
* processed in the current execute() call; each member's properties are
42+
* merged into this list, and the merged result is used for every
43+
* subsequent member. Reset at the start of getTitlesToLoad().
44+
*
45+
* @var array
3746
*/
38-
public function mustBePosted(): bool {
39-
return true;
40-
}
47+
private $accumulatedProperties = [];
4148

4249
/**
4350
* @inheritDoc
4451
*/
45-
public function execute() {
46-
$result = $this->getResult();
47-
$params = $this->extractRequestParams();
48-
$context = $this->getContext();
49-
$output = $context->getOutput();
50-
51-
\KnowledgeGraph::initSMW();
52+
protected function getTitlesToLoad( array $params ): iterable {
5253
self::$SMWStore = \SMW\StoreFactory::getStore();
5354
self::$SMWDataValueFactory = SMW\DataValueFactory::getInstance();
5455

@@ -60,66 +61,59 @@ public function execute() {
6061
'format' => 'json'
6162
];
6263

63-
$api = new ApiMain( \KnowledgeGraph::newDerivativeApiContext( $context, $queryParams, false ) );
64+
$api = new ApiMain(
65+
\KnowledgeGraph::newDerivativeApiContext( $this->getContext(), $queryParams, false )
66+
);
6467
$api->execute();
65-
$data = $api->getResult()->getResultData();
68+
$allPagesData = $api->getResult()->getResultData();
6669

67-
$propertyTitles = $data['query']['allpages'] ?? [];
70+
$propertyTitles = $allPagesData['query']['allpages'] ?? [];
6871
$propertyTitles = array_column( $propertyTitles, 'title' );
69-
$propertyNames = array_map( static function ( $title ) {
72+
$this->propertyNames = array_map( static function ( $title ) {
7073
return substr( $title, strrpos( $title, ':' ) + 1 );
7174
}, $propertyTitles );
7275

73-
$params['properties'] = ( !empty( $params['properties'] ) ?
76+
$this->accumulatedProperties = ( !empty( $params['properties'] ) ?
7477
json_decode( $params['properties'], true ) : [] );
7578

7679
$categories = explode( '|', $params['categories'] );
7780

78-
$data = [];
79-
$relationsSeen = [];
80-
$titles = [];
8181
foreach ( $categories as $categoryText ) {
8282
$category_ = Title::makeTitleSafe( NS_CATEGORY, $categoryText );
8383
// && $category_->isKnown()
84-
if ( $category_ ) {
85-
$titles_ = \KnowledgeGraph::articlesInCategories(
86-
$categoryText,
87-
$params['limit'],
88-
$params['offset']
84+
if ( !$category_ ) {
85+
continue;
86+
}
87+
88+
$titles_ = \KnowledgeGraph::articlesInCategories(
89+
$categoryText,
90+
$params['limit'],
91+
$params['offset']
92+
);
93+
94+
foreach ( $titles_ as $title_ ) {
95+
$titleText = str_replace( '_', ' ', $title_->getDbKey() );
96+
97+
$this->accumulatedProperties = $this->buildPropertiesList(
98+
$this->propertyNames,
99+
$title_,
100+
$titleText,
101+
$this->accumulatedProperties,
102+
$params['limit']
89103
);
90104

91-
foreach ( $titles_ as $title_ ) {
92-
$titles[$title_->getFullText()] = $title_;
93-
94-
$titleText = $title_->getDbKey();
95-
$titleText = str_replace( '_', ' ', $titleText );
96-
97-
$params['properties'] = $this->buildPropertiesList(
98-
$propertyNames,
99-
$title_,
100-
$titleText,
101-
$params['properties'],
102-
$params['limit']
103-
);
104-
105-
if ( $title_ && $title_->isKnown() ) {
106-
if ( !isset( $data[$title_->getFullText()] ) ) {
107-
\KnowledgeGraph::setSemanticDataFromApi(
108-
$title_,
109-
$params['properties'],
110-
0,
111-
$params['depth'],
112-
$data,
113-
$relationsSeen
114-
);
115-
}
116-
}
105+
if ( $title_->isKnown() ) {
106+
yield $titleText => $title_;
117107
}
118108
}
119109
}
110+
}
120111

121-
$res = json_encode( $data );
122-
$result->addValue( [ $this->getModuleName() ], 'data', $res, ApiResult::NO_VALIDATE );
112+
/**
113+
* @inheritDoc
114+
*/
115+
protected function getPropertiesForTitle( array $params, Title $title_, string $titleText ): array {
116+
return $this->accumulatedProperties;
123117
}
124118

125119
/**
@@ -209,13 +203,6 @@ public function getAllowedParams() {
209203
];
210204
}
211205

212-
/**
213-
* @inheritDoc
214-
*/
215-
public function needsToken() {
216-
return 'csrf';
217-
}
218-
219206
/**
220207
* @inheritDoc
221208
*/

‎includes/api/KnowledgeGraphApiLoadNodes.php‎

Lines changed: 10 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -11,71 +11,27 @@
1111

1212
class KnowledgeGraphApiLoadNodes extends ApiBase {
1313

14-
/**
15-
* Store instance for Semantic MediaWiki data.
16-
*
17-
* @var SMW\Store|null
18-
*/
19-
protected static $SMWStore = null;
20-
21-
/**
22-
* Factory instance for creating Semantic MediaWiki data values.
23-
*
24-
* @var SMW\DataValueFactory|null
25-
*/
26-
protected static $SMWDataValueFactory = null;
27-
28-
/**
29-
* @inheritDoc
30-
*/
31-
public function isWriteMode() {
32-
return false;
33-
}
34-
35-
/**
36-
* @inheritDoc
37-
*/
38-
public function mustBePosted(): bool {
39-
return true;
40-
}
14+
use KnowledgeGraphApiLoadTrait;
4115

4216
/**
4317
* @inheritDoc
4418
*/
45-
public function execute() {
46-
$result = $this->getResult();
47-
$params = $this->extractRequestParams();
48-
$context = $this->getContext();
49-
50-
\KnowledgeGraph::initSMW();
51-
self::$SMWStore = \SMW\StoreFactory::getStore();
52-
self::$SMWDataValueFactory = SMW\DataValueFactory::getInstance();
53-
54-
$data = [];
55-
$relationsSeen = [];
56-
$titles = explode( '|', $params['titles'] );
57-
foreach ( $titles as $titleText ) {
19+
protected function getTitlesToLoad( array $params ): iterable {
20+
foreach ( explode( '|', $params['titles'] ) as $titleText ) {
5821
$title_ = Title::newFromText( $titleText );
5922
if ( !$title_ || !$title_->isKnown() ) {
6023
continue;
6124
}
6225

63-
$listOfProps = \KnowledgeGraph::getAllPropertiesForNode( $titleText );
64-
65-
if ( !isset( $data[$title_->getFullText()] ) ) {
66-
\KnowledgeGraph::setSemanticDataFromApi(
67-
$title_,
68-
$listOfProps,
69-
0,
70-
$params['depth'],
71-
$data,
72-
$relationsSeen
73-
);
74-
}
26+
yield $titleText => $title_;
7527
}
28+
}
7629

77-
$res = json_encode( $data );
78-
$result->addValue( [ $this->getModuleName() ], 'data', $res, ApiResult::NO_VALIDATE );
30+
/**
31+
* @inheritDoc
32+
*/
33+
protected function getPropertiesForTitle( array $params, Title $title_, string $titleText ): array {
34+
return \KnowledgeGraph::getAllPropertiesForNode( $titleText );
7935
}
8036

8137
/**
@@ -99,13 +55,6 @@ public function getAllowedParams() {
9955
];
10056
}
10157

102-
/**
103-
* @inheritDoc
104-
*/
105-
public function needsToken() {
106-
return 'csrf';
107-
}
108-
10958
/**
11059
* @inheritDoc
11160
*/

‎includes/api/KnowledgeGraphApiLoadProperties.php‎

Lines changed: 12 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,54 +11,30 @@
1111

1212
class KnowledgeGraphApiLoadProperties extends ApiBase {
1313

14-
/**
15-
* @inheritDoc
16-
*/
17-
public function isWriteMode() {
18-
return false;
19-
}
14+
use KnowledgeGraphApiLoadTrait;
2015

2116
/**
2217
* @inheritDoc
2318
*/
24-
public function mustBePosted(): bool {
25-
return true;
19+
protected function getTitlesToLoad( array $params ): iterable {
20+
foreach ( explode( '|', $params['nodes'] ) as $titleText ) {
21+
$title_ = Title::newFromText( $titleText );
22+
if ( !$title_ || !$title_->isKnown() ) {
23+
continue;
24+
}
25+
26+
yield $titleText => $title_;
27+
}
2628
}
2729

2830
/**
2931
* @inheritDoc
3032
*/
31-
public function execute() {
32-
$result = $this->getResult();
33-
$params = $this->extractRequestParams();
34-
35-
\KnowledgeGraph::initSMW();
36-
$params['properties'] = self::expandInverseProperties(
33+
protected function getPropertiesForTitle( array $params, Title $title_, string $titleText ): array {
34+
return self::expandInverseProperties(
3735
explode( '|', $params['properties'] ),
3836
(bool)$params['inversePropsIncluded']
3937
);
40-
41-
$data = [];
42-
$relationsSeen = [];
43-
$params['nodes'] = explode( '|', $params['nodes'] );
44-
foreach ( $params['nodes'] as $titleText ) {
45-
$title_ = Title::newFromText( $titleText );
46-
if ( $title_ && $title_->isKnown() ) {
47-
if ( !isset( $data[$title_->getFullText()] ) ) {
48-
\KnowledgeGraph::setSemanticDataFromApi(
49-
$title_,
50-
$params['properties'],
51-
0,
52-
$params['depth'],
53-
$data,
54-
$relationsSeen
55-
);
56-
}
57-
}
58-
}
59-
60-
$res = json_encode( $data );
61-
$result->addValue( [ $this->getModuleName() ], 'data', $res, ApiResult::NO_VALIDATE );
6238
}
6339

6440
/**
@@ -112,13 +88,6 @@ public function getAllowedParams() {
11288
];
11389
}
11490

115-
/**
116-
* @inheritDoc
117-
*/
118-
public function needsToken() {
119-
return 'csrf';
120-
}
121-
12291
/**
12392
* @inheritDoc
12493
*/

0 commit comments

Comments
 (0)