jenkins-bot has submitted this change and it was merged.
Change subject: Factor out extension data manipulation
......................................................................
Factor out extension data manipulation
Change-Id: I1aa83bbbecf6432468739c60a937cde991563157
---
M extension.json
M includes/Hooks.php
A includes/State.php
M includes/Tag/MapFrame.php
M includes/Tag/MapLink.php
M includes/Tag/TagHandler.php
M tests/phpunit/KartographerTest.php
7 files changed, 154 insertions(+), 34 deletions(-)
Approvals:
Yurik: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 7d1f9c9..9a0d3cd 100644
--- a/extension.json
+++ b/extension.json
@@ -23,6 +23,7 @@
"Kartographer\\DataModule": "includes/DataModule.php",
"Kartographer\\Hooks": "includes/Hooks.php",
"Kartographer\\SimpleStyleParser":
"includes/SimpleStyleParser.php",
+ "Kartographer\\State": "includes/State.php",
"Kartographer\\Tag\\MapFrame": "includes/Tag/MapFrame.php",
"Kartographer\\Tag\\MapLink": "includes/Tag/MapLink.php",
"Kartographer\\Tag\\TagHandler": "includes/Tag/TagHandler.php"
diff --git a/includes/Hooks.php b/includes/Hooks.php
index 8343ee5..94a6e66 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -11,9 +11,10 @@
use Kartographer\Tag\TagHandler;
use Parser;
+use ParserOutput;
class Hooks {
- static $tags = [
+ private static $tags = [
'mapframe' => 'Kartographer\Tag\MapFrame::entryPoint',
'maplink' => 'Kartographer\Tag\MapLink::entryPoint',
];
@@ -59,4 +60,21 @@
TagHandler::finalParseStep( $parser );
return true;
}
+
+ /**
+ * RejectParserCacheValue hook handler. Rejects output with old
versions of map data
+ * structures. To be enabled at a later date.
+ * @see
https://www.mediawiki.org/wiki/Manual:Hooks/RejectParserCacheValue
+ * @param ParserOutput $po
+ * @return bool
+ */
+ /*public static function onRejectParserCacheValue( ParserOutput $po ) {
+ // One of these should be prsent in any output with old version
of data
+ if ( $po->getExtensionData( 'kartographer_valid' )
+ || $po->getExtensionData( 'kartographer_broken' )
+ ) {
+ return false;
+ }
+ return true;
+ }*/
}
diff --git a/includes/State.php b/includes/State.php
new file mode 100644
index 0000000..f90741a
--- /dev/null
+++ b/includes/State.php
@@ -0,0 +1,103 @@
+<?php
+
+namespace Kartographer;
+
+use ParserOutput;
+use stdClass;
+
+/**
+ * Stores information about map tags on page in ParserOutput
+ */
+class State {
+ const DATA_KEY = 'kartographer';
+ const VERSION = 1;
+
+ /** @var int Version of this class, for checking after deserialization
*/
+ private $version = self::VERSION;
+
+ private $valid = false;
+ private $broken = false;
+ private $interactiveGroups = [];
+ private $requestedGroups = [];
+ private $counters;
+ private $data = [];
+
+ /**
+ * Retrieves an instance of self from ParserOutout, if present
+ *
+ * @param ParserOutput $output
+ * @return self|null
+ */
+ public static function getState( ParserOutput $output ) {
+ return $output->getExtensionData( self::DATA_KEY );
+ }
+
+ /**
+ * Retrieves an instance of self from ParserOutput, initializing it
anew if not present
+ *
+ * @param ParserOutput $output
+ * @return State
+ */
+ public static function getOrCreate( ParserOutput $output ) {
+ $result = self::getState( $output );
+ if ( !$result ) {
+ $result = new self;
+ $output->setExtensionData( self::DATA_KEY, $result );
+ }
+
+ return $result;
+ }
+
+ public function hasValidTags() {
+ return $this->valid;
+ }
+
+ public function setValidTags() {
+ $this->valid = true;
+ }
+
+ public function hasBrokenTags() {
+ return $this->broken;
+ }
+
+ public function setBrokenTags() {
+ $this->broken = true;
+ }
+
+ public function addInteractiveGroups( array $groups ) {
+ $this->interactiveGroups += array_flip( $groups );
+ }
+
+ public function getInteractiveGroups() {
+ return array_keys( $this->interactiveGroups );
+ }
+
+ public function addRequestedGroups( array $groups ) {
+ $this->requestedGroups += array_flip( $groups );
+ }
+
+ public function getRequestedGroups() {
+ return $this->requestedGroups;
+ }
+
+ public function getCounters() {
+ return $this->counters ?: new stdClass();
+ }
+
+ public function setCounters( stdClass $counters ) {
+ $this->counters = $counters;
+ }
+
+ public function addData( $key, $data ) {
+ $this->data = $this->data ?: new stdClass();
+ if ( property_exists( $this->data, $key ) ) {
+ $this->data->$key = array_merge( $this->data->$key,
$data );
+ } else {
+ $this->data->$key = $data;
+ }
+ }
+
+ public function getData() {
+ return $this->data;
+ }
+}
diff --git a/includes/Tag/MapFrame.php b/includes/Tag/MapFrame.php
index 7d9fa27..f84cf7c 100644
--- a/includes/Tag/MapFrame.php
+++ b/includes/Tag/MapFrame.php
@@ -103,10 +103,7 @@
FormatJson::ALL_OK );
}
- $groups = $output->getExtensionData(
'kartographer_interact' );
- $groups = $groups ?: [];
- $groups = array_merge( $groups,
$this->showGroups );
- $output->setExtensionData(
'kartographer_interact', $groups );
+ $this->state->addInteractiveGroups(
$this->showGroups );
return Html::rawElement( 'div', $attrs );
break;
diff --git a/includes/Tag/MapLink.php b/includes/Tag/MapLink.php
index 7a9a2b2..65c9390 100644
--- a/includes/Tag/MapLink.php
+++ b/includes/Tag/MapLink.php
@@ -22,10 +22,6 @@
protected function render() {
$output = $this->parser->getOutput();
$output->addModules( 'ext.kartographer.link' );
- $interact = $output->getExtensionData( 'kartographer_interact'
);
- if ( $interact === null ) {
- $output->setExtensionData( 'kartographer_interact', []
);
- }
// @todo: Mapbox markers don't support localized numbers yet
$text = $this->getText( 'text', null, '/\S+/' );
diff --git a/includes/Tag/TagHandler.php b/includes/Tag/TagHandler.php
index 695c50d..f0ba225 100644
--- a/includes/Tag/TagHandler.php
+++ b/includes/Tag/TagHandler.php
@@ -13,6 +13,7 @@
use FormatJson;
use Html;
use Kartographer\SimpleStyleParser;
+use Kartographer\State;
use Language;
use Parser;
use ParserOutput;
@@ -63,6 +64,9 @@
/** @var PPFrame */
protected $frame;
+ /** @var State */
+ protected $state;
+
/** @var stdClass */
protected $markerProperties;
@@ -107,6 +111,7 @@
$this->frame = $frame;
$output = $parser->getOutput();
$output->addModuleStyles( 'ext.kartographer.style' );
+ $this->state = State::getOrCreate( $output );
$this->status = Status::newGood();
$this->args = $args;
@@ -121,7 +126,7 @@
$this->saveData( $output );
- $output->setExtensionData( 'kartographer_valid', true );
+ $this->state->setValidTags();
return $this->render();
}
@@ -241,9 +246,7 @@
protected function saveData( ParserOutput $output ) {
- $requestedGroups = $output->getExtensionData(
'kartographer_requested' ) ?: [];
- $requestedGroups = array_merge( $requestedGroups,
$this->showGroups );
- $output->setExtensionData( 'kartographer_requested',
$requestedGroups );
+ $this->state->addRequestedGroups( $this->showGroups );
if ( !$this->geometries ) {
return;
@@ -253,12 +256,12 @@
// For all GeoJSON items whose marker-symbol value begins with
'-counter' and '-letter',
// recursively replace them with an automatically incremented
marker icon.
- $counters = $output->getExtensionData( 'kartographer_counters'
) ?: new stdClass();
+ $counters = $this->state->getCounters();
$marker = SimpleStyleParser::doCountersRecursive(
$this->geometries, $counters );
if ( $marker ) {
list( $this->counter, $this->markerProperties ) =
$marker;
}
- $output->setExtensionData( 'kartographer_counters', $counters );
+ $this->state->setCounters( $counters );
if ( $this->groupName === null ) {
$group = '_' . sha1( FormatJson::encode(
$this->geometries, false, FormatJson::ALL_OK ) );
@@ -269,13 +272,7 @@
$group = $this->groupName;
}
- $data = $output->getExtensionData( 'kartographer_data' ) ?: new
stdClass();
- if ( isset( $data->$group ) ) {
- $data->$group = array_merge( $data->$group,
$this->geometries );
- } else {
- $data->$group = $this->geometries;
- }
- $output->setExtensionData( 'kartographer_data', $data );
+ $this->state->addData( $group, $this->geometries );
}
/**
@@ -284,22 +281,27 @@
*/
public static function finalParseStep( Parser $parser ) {
$output = $parser->getOutput();
+ $state = State::getState( $output );
- $data = $output->getExtensionData( 'kartographer_data' );
+ if ( !$state ) {
+ return;
+ }
+
+ $data = $state->getData();
if ( $data ) {
$json = FormatJson::encode( $data, false,
FormatJson::ALL_OK );
$output->setProperty( 'kartographer', gzencode( $json )
);
}
- if ( $output->getExtensionData( 'kartographer_broken' ) ) {
+ if ( $state->hasBrokenTags() ) {
$output->addTrackingCategory(
'kartographer-broken-category', $parser->getTitle() );
}
- if ( $output->getExtensionData( 'kartographer_valid' ) ) {
+ if ( $state->hasValidTags() ) {
$output->addTrackingCategory(
'kartographer-tracking-category', $parser->getTitle() );
}
- $interact = $output->getExtensionData( 'kartographer_interact'
) ?: [];
- $requested = $output->getExtensionData(
'kartographer_requested' ) ?: [];
+ $interact = $state->getInteractiveGroups();
+ $requested = $state->getRequestedGroups();
if ( $interact || $requested ) {
$interact = array_flip( $interact );
$liveData = array_intersect_key( (array)$data,
$interact );
@@ -319,7 +321,7 @@
* @throws Exception
*/
private function reportError() {
- $this->parser->getOutput()->setExtensionData(
'kartographer_broken', true );
+ $this->state->setBrokenTags();
$errors = array_merge( $this->status->getErrorsByType( 'error'
),
$this->status->getErrorsByType( 'warning' )
);
diff --git a/tests/phpunit/KartographerTest.php
b/tests/phpunit/KartographerTest.php
index e2cfc35..7fe468f 100644
--- a/tests/phpunit/KartographerTest.php
+++ b/tests/phpunit/KartographerTest.php
@@ -1,6 +1,7 @@
<?php
namespace Kartographer\Tests;
+use Kartographer\State;
use MediaWikiTestCase;
use Parser;
use ParserOptions;
@@ -40,18 +41,20 @@
*/
public function testTagData( $expected, $input, $message ) {
$output = $this->parse( $input );
+ $state = State::getState( $output );
if ( $expected === false ) {
- $this->assertTrue( $output->getExtensionData(
'kartographer_broken' ), $message . ' Parse is expected to fail' );
+ $this->assertTrue( $state->hasBrokenTags(), $message .
' Parse is expected to fail' );
$this->assertTrue( $this->hasTrackingCategory( $output,
'kartographer-broken-category' ), $message . ' Category for failed maps should
be added' );
return;
}
- $this->assertFalse( !!$output->getExtensionData(
'kartographer_broken' ), $message . ' Parse is expected to succeed' );
+ $this->assertFalse( $state->hasBrokenTags(), $message . ' Parse
is expected to succeed' );
+ $this->assertTrue( $state->hasValidTags(), $message . ' State
is expected to have valid tags' );
$this->assertFalse( $this->hasTrackingCategory( $output,
'kartographer-broken-category' ), $message . ' No tracking category' );
$expected = json_encode( json_decode( $expected ) ); //
Normalize JSON
- $this->assertEquals( $expected, json_encode(
$output->getExtensionData( 'kartographer_data' ) ), $message );
+ $this->assertEquals( $expected, json_encode( $state->getData()
), $message );
}
public function provideTagData() {
@@ -104,8 +107,8 @@
"description":"<a
href=\"\/w\/index.php?title=Link_to_nowhere&action=edit&redlink=1\"
class=\"new\" title=\"Link to nowhere (page does not exist)\">Link to
nowhere<\/a>","marker-symbol":"1"}}
]}';
return [
- [ 'null', '<mapframe width=700 height=400 zoom=13
longitude=-122.3988 latitude=37.8013/>', '<mapframe> without JSON' ],
- [ 'null', '<mapframe width=700 height=400 zoom=13
longitude=-122.3988 latitude=37.8013></mapframe>', '<mapframe> without JSON 2'
],
+ [ '[]', '<mapframe width=700 height=400 zoom=13
longitude=-122.3988 latitude=37.8013/>', '<mapframe> without JSON' ],
+ [ '[]', '<mapframe width=700 height=400 zoom=13
longitude=-122.3988 latitude=37.8013></mapframe>', '<mapframe> without JSON 2'
],
[
"{\"_4622d19afa2e6480c327846395ed932ba6fa56d4\":[$validJson]}", "<mapframe
width=700 height=400 zoom=13 longitude=-122.3988
latitude=37.8013>$validJson</mapframe>", '<mapframe> with GeoJSON' ],
[
"{\"_4622d19afa2e6480c327846395ed932ba6fa56d4\":[$validJson]}", "<mapframe
width=700 height=400 zoom=13 longitude=-122.3988
latitude=37.8013>[$validJson]</mapframe>", '<mapframe> with GeoJSON array' ],
[ false, '<mapframe width=700 height=400 zoom=13
longitude=-122.3988 latitude=37.8013>123</mapframe>', 'Invalid JSON' ],
@@ -129,7 +132,7 @@
[ $wikitextJsonParsed, "<maplink zoom=13
longitude=-122.3988 latitude=37.8013>[{$this->wikitextJson}]</maplink>",
'<maplink> with parsable text and description' ],
// Bugs
- [ 'null', "<maplink zoom=13 longitude=-122.3988
latitude=37.8013>\t\r\n </maplink>", 'T127345: whitespace-only tag content,
<maplink>' ],
+ [ '[]', "<maplink zoom=13 longitude=-122.3988
latitude=37.8013>\t\r\n </maplink>", 'T127345: whitespace-only tag content,
<maplink>' ],
[ $xssJsonSanitized, "<maplink zoom=13 longitude=10
latitude=20>$xssJson</maplink>", 'T134719: XSS via __proto__' ],
];
}
--
To view, visit https://gerrit.wikimedia.org/r/297110
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1aa83bbbecf6432468739c60a937cde991563157
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Kartographer
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: Yurik <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
