MaxSem has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/316011

Change subject: Refactor tag handling
......................................................................

Refactor tag handling

Rename the class, make it non-static. Passing some parameters
to constructor only improves readability, as well as separating concerns.
In, process, make sure error messages use page language.

Bug: T148082
Change-Id: Ifd8edc722755e4a85af3a4a316e0cee16fe88554
---
M extension.json
M includes/ApiGraph.php
A includes/Content.php
M includes/Graph.hooks.php
R includes/ParserTag.php
A includes/Store.php
6 files changed, 140 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Graph 
refs/changes/11/316011/1

diff --git a/extension.json b/extension.json
index 620032e..c83ee03 100644
--- a/extension.json
+++ b/extension.json
@@ -17,9 +17,10 @@
        "AutoloadClasses": {
                "Graph\\ApiGraph": "includes/ApiGraph.php",
                "Graph\\DataModule": "includes/DataModule.php",
-               "Graph\\Singleton": "includes/Graph.body.php",
+               "Graph\\ParserTag": "includes/ParserTag.php",
                "Graph\\Sandbox": "includes/Sandbox.php",
-               "Graph\\Content": "includes/Graph.body.php",
+               "Graph\\Store": "includes/Store.php",
+               "Graph\\Content": "includes/Content.php",
                "Graph\\Hooks": "includes/Graph.hooks.php"
        },
        "APIModules": {
diff --git a/includes/ApiGraph.php b/includes/ApiGraph.php
index 905b909..1c08046 100644
--- a/includes/ApiGraph.php
+++ b/includes/ApiGraph.php
@@ -101,7 +101,7 @@
 
                // NOTE: Very strange wgMemc feature: Even though we store the 
data structure into memcached
                // by JSON-encoding and gzip-ing it, when we get it out it is 
already in the original form.
-               $graph = Singleton::getDataFromCache( $hash );
+               $graph = Store::getFromCache( $hash );
                if ( !$graph ) {
                        $title = Title::newFromText( $title );
                        if ( !$title || !$title->exists() || !$title->userCan( 
'read', $this->getUser() ) ) {
diff --git a/includes/Content.php b/includes/Content.php
new file mode 100644
index 0000000..4a4d2f6
--- /dev/null
+++ b/includes/Content.php
@@ -0,0 +1,52 @@
+<?php
+/**
+ *
+ * @license MIT
+ * @file
+ *
+ * @author Dan Andreescu, Yuri Astrakhan, Frédéric Bolduc
+ */
+
+namespace Graph;
+
+use JsonConfig\JCContent;
+use Parser;
+use ParserOptions;
+use ParserOutput;
+use Title;
+
+/**
+ * Class Content represents JSON content that Graph understands
+ * as the definition of a visualization.
+ *
+ * This is based on TextContent, and represents JSON as a string.
+ *
+ * TODO: determine if a different representation makes more sense and 
implement it with
+ * ContentHandler::serializeContent() and ContentHandler::unserializeContent()
+ *
+ * TODO: create a visual editor for Graph definitions that introspects what is 
allowed
+ * in each part of the definition and presents documentation to aid with 
discovery.
+ *
+ */
+class Content extends JCContent  {
+
+       public function getWikitextForTransclusion() {
+               return '<graph>' . $this->getNativeData() . '</graph>';
+       }
+
+       protected function fillParserOutput( Title $title, $revId, 
ParserOptions $options, $generateHtml,
+                                                                               
 ParserOutput &$output ) {
+               /** @var $wgParser Parser */
+               global $wgParser;
+               $text = $this->getNativeData();
+               $parser = $wgParser->getFreshParser();
+               $text = $parser->preprocess( $text, $title, $options, $revId );
+
+               $html = !$generateHtml ? '' : Singleton::buildHtml( $text, 
$title, $revId, $output,
+                       $options->getIsPreview() );
+               $output->setText( $html );
+
+               // Since we invoke parser manually, the ParserAfterParse never 
gets called, do it manually
+               ParserTag::finalizeParserOutput( $parser, $title, $output );
+       }
+}
diff --git a/includes/Graph.hooks.php b/includes/Graph.hooks.php
index 27cecda..18c2b49 100644
--- a/includes/Graph.hooks.php
+++ b/includes/Graph.hooks.php
@@ -53,7 +53,7 @@
         * @return bool
         */
        public static function onParserFirstCallInit( Parser $parser ) {
-               $parser->setHook( 'graph', 'Graph\Singleton::onGraphTag' );
+               $parser->setHook( 'graph', 'Graph\ParserTag::onGraphTag' );
                return true;
        }
 
@@ -64,7 +64,7 @@
         * @return bool
         */
        public static function onParserAfterParse( Parser $parser ) {
-               Singleton::finalizeParserOutput( $parser, $parser->getTitle(), 
$parser->getOutput() );
+               ParserTag::finalizeParserOutput( $parser, $parser->getTitle(), 
$parser->getOutput() );
                return true;
        }
 }
diff --git a/includes/Graph.body.php b/includes/ParserTag.php
similarity index 64%
rename from includes/Graph.body.php
rename to includes/ParserTag.php
index 879eb33..8e8a608 100644
--- a/includes/Graph.body.php
+++ b/includes/ParserTag.php
@@ -11,15 +11,36 @@
 
 use FormatJson;
 use Html;
-use JsonConfig\JCContent;
-use ObjectCache;
+use Language;
+use Message;
 use Parser;
 use ParserOptions;
 use ParserOutput;
 use PPFrame;
+use Status;
 use Title;
 
-class Singleton {
+class ParserTag {
+       /** @var ParserOptions */
+       private $parserOptions;
+
+       /** @var ParserOutput */
+       private $parserOutput;
+
+       /** @var Language */
+       private $language;
+
+       /**
+        * ParserTag constructor.
+        * @param Parser $parser
+        * @param ParserOptions $parserOptions
+        * @param ParserOutput $parserOutput
+        */
+       public function __construct( Parser $parser, ParserOptions 
$parserOptions, ParserOutput $parserOutput ) {
+               $this->parserOptions = $parserOptions;
+               $this->parserOutput = $parserOutput;
+               $this->language = $parser->getTargetLanguage();
+       }
 
        /**
         * @param $input
@@ -29,8 +50,8 @@
         * @return string
         */
        public static function onGraphTag( $input, array $args, Parser $parser, 
PPFrame $frame ) {
-               return Singleton::buildHtml( $input, $parser->getTitle(), 
$parser->getRevisionId(),
-                       $parser->getOutput(), 
$parser->getOptions()->getIsPreview(), $args );
+               $tag = new self( $parser, $parser->getOptions(), 
$parser->getOutput() );
+               return $tag->buildHtml( $input, $parser->getTitle(), 
$parser->getRevisionId(), $args );
        }
 
        public static function finalizeParserOutput( Parser $parser, $title, 
ParserOutput $output ) {
@@ -80,7 +101,7 @@
         * @param string $hash
         * @return array
         */
-       public static function buildDivAttributes( $mode = '', $data = false, 
$hash = '' ) {
+       private function buildDivAttributes( $mode = '', $data = false, $hash = 
'' ) {
                $attribs = array( 'class' => 'mw-graph' );
 
                if ( is_object( $data ) ) {
@@ -101,23 +122,29 @@
                return $attribs;
        }
 
+       private function formatError( Message $msg ) {
+               $this->parserOutput->setExtensionData( 'graph_specs_broken', 
true );
+               $error = $msg->inLanguage( $this->language )->parse();
+               return "<span class=\"error\">{$error}</span>";
+       }
+
+       private function formatStatus( Status $status ) {
+               return $this->formatError( $status->getMessage( false, false, 
$this->language ) );
+       }
+
        /**
         * @param string $jsonText
         * @param Title $title
         * @param int $revid
-        * @param ParserOutput $parserOutput
-        * @param bool $isPreview
         * @param array $args
         * @return string
         */
-       public static function buildHtml( $jsonText, $title, $revid, 
$parserOutput, $isPreview,
-                                                                         $args 
= null ) {
+       public function buildHtml( $jsonText, Title $title, $revid, $args = 
null ) {
                global $wgGraphImgServiceUrl, $wgServerName;
 
                $status = FormatJson::parse( $jsonText, FormatJson::TRY_FIXING 
| FormatJson::STRIP_COMMENTS );
                if ( !$status->isOK() ) {
-                       $parserOutput->setExtensionData( 'graph_specs_broken', 
true );
-                       return "<span 
class=\"error\">{$status->getWikiText()}</span>";
+                       return $this->formatStatus( $status );
                }
 
                $isInteractive = isset( $args['mode'] ) && $args['mode'] === 
'interactive';
@@ -136,26 +163,25 @@
                        $data->version = $wgGraphDefaultVegaVer;
                }
                if ( $data->version === 2 ) {
-                       $parserOutput->setExtensionData( 'graph_vega2', true );
+                       $this->parserOutput->setExtensionData( 'graph_vega2', 
true );
                } else {
-                       $parserOutput->setExtensionData( 
'graph_specs_obsolete', true );
+                       $this->parserOutput->setExtensionData( 
'graph_specs_obsolete', true );
                }
 
                // Calculate hash and store graph definition in graph_specs 
extension data
-               $specs = $parserOutput->getExtensionData( 'graph_specs' ) ?: 
array();
+               $specs = $this->parserOutput->getExtensionData( 'graph_specs' ) 
?: array();
                // Make sure that multiple json blobs that only differ in 
spacing hash the same
                $hash = sha1( FormatJson::encode( $data, false, 
FormatJson::ALL_OK ) );
                $specs[$hash] = $data;
-               $parserOutput->setExtensionData( 'graph_specs', $specs );
+               $this->parserOutput->setExtensionData( 'graph_specs', $specs );
+               Store::saveToCache( $hash, $data );
 
-               self::saveDataToCache( $hash, $data );
-
-               if ( $isPreview || !$wgGraphImgServiceUrl ) {
+               if ( $this->parserOptions->getIsPreview() || 
!$wgGraphImgServiceUrl ) {
                        // Always do client-side rendering
                        $attribs = self::buildDivAttributes( 'always', $data, 
$hash );
-                       $liveSpecs = $parserOutput->getExtensionData( 
'graph_live_specs' ) ?: array();
+                       $liveSpecs = $this->parserOutput->getExtensionData( 
'graph_live_specs' ) ?: array();
                        $liveSpecs[$hash] = $data;
-                       $parserOutput->setExtensionData( 'graph_live_specs', 
$liveSpecs );
+                       $this->parserOutput->setExtensionData( 
'graph_live_specs', $liveSpecs );
                        $html = ''; // will be injected with a <canvas> tag
                } else {
 
@@ -176,7 +202,7 @@
 
                        if ( $isInteractive ) {
                                // Allow image to interactive switchover
-                               $parserOutput->setExtensionData( 
'graph_interact', true );
+                               $this->parserOutput->setExtensionData( 
'graph_interact', true );
                                $attribs = self::buildDivAttributes( 
'interactable', $data, $hash );
 
                                // add the overlay title
@@ -200,61 +226,5 @@
                }
 
                return Html::rawElement( 'div', $attribs, $html );
-       }
-
-       /**
-        * Store graph data in the memcached
-        * @param $hash string
-        * @param $data string Graph spec after json encoding
-        */
-       private static function saveDataToCache( $hash, $data ) {
-               $cache = ObjectCache::getLocalClusterInstance();
-               $cache->add( $cache->makeKey( 'graph-data', $hash ), $data );
-       }
-
-       /**
-        * Get graph data from the memcached
-        * @param $hash
-        * @return mixed
-        */
-       public static function getDataFromCache( $hash ) {
-               $cache = ObjectCache::getLocalClusterInstance();
-               return $cache->get( $cache->makeKey( 'graph-data', $hash ) );
-       }
-}
-
-/**
- * Class Content represents JSON content that Graph understands
- * as the definition of a visualization.
- *
- * This is based on TextContent, and represents JSON as a string.
- *
- * TODO: determine if a different representation makes more sense and 
implement it with
- * ContentHandler::serializeContent() and ContentHandler::unserializeContent()
- *
- * TODO: create a visual editor for Graph definitions that introspects what is 
allowed
- * in each part of the definition and presents documentation to aid with 
discovery.
- *
- */
-class Content extends JCContent {
-
-       public function getWikitextForTransclusion() {
-               return '<graph>' . $this->getNativeData() . '</graph>';
-       }
-
-       protected function fillParserOutput( Title $title, $revId, 
ParserOptions $options, $generateHtml,
-                                            ParserOutput &$output ) {
-               /** @var $wgParser Parser */
-               global $wgParser;
-               $text = $this->getNativeData();
-               $parser = $wgParser->getFreshParser();
-               $text = $parser->preprocess( $text, $title, $options, $revId );
-
-               $html = !$generateHtml ? '' : Singleton::buildHtml( $text, 
$title, $revId, $output,
-                       $options->getIsPreview() );
-               $output->setText( $html );
-
-               // Since we invoke parser manually, the ParserAfterParse never 
gets called, do it manually
-               Singleton::finalizeParserOutput( $parser, $title, $output );
        }
 }
diff --git a/includes/Store.php b/includes/Store.php
new file mode 100644
index 0000000..285bdf5
--- /dev/null
+++ b/includes/Store.php
@@ -0,0 +1,34 @@
+<?php
+/**
+ *
+ * @license MIT
+ * @file
+ *
+ * @author Yuri Astrakhan
+ */
+
+namespace Graph;
+
+use ObjectCache;
+
+class Store {
+       /**
+        * Store graph data in the memcached
+        * @param string $hash
+        * @param string $data Graph spec after json encoding
+        */
+       public static function saveToCache( $hash, $data ) {
+               $cache = ObjectCache::getLocalClusterInstance();
+               $cache->add( $cache->makeKey( 'graph-data', $hash ), $data );
+       }
+
+       /**
+        * Get graph data from memcached
+        * @param string $hash
+        * @return mixed
+        */
+       public static function getFromCache( $hash ) {
+               $cache = ObjectCache::getLocalClusterInstance();
+               return $cache->get( $cache->makeKey( 'graph-data', $hash ) );
+       }
+}

-- 
To view, visit https://gerrit.wikimedia.org/r/316011
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifd8edc722755e4a85af3a4a316e0cee16fe88554
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Graph
Gerrit-Branch: master
Gerrit-Owner: MaxSem <maxsem.w...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to