Yurik has uploaded a new change for review.
https://gerrit.wikimedia.org/r/160575
Change subject: abstract CodeContent, reworked JsonContent
......................................................................
abstract CodeContent, reworked JsonContent
* Merged common code from Css, JavaScript, Json Content objects into
an abstract CodeContent class
* Reworked JsonContent object:
** Support JSON decoding options
** Decoding produces a status object that may contain decoded object
** Saves optimally formatted, no spaces blob
** Removed beautifyJSON() - don't think we need it - encoding is different
for different use-cases, plus the errors should be handled properly.
** Table rendering is done the same as other code content - with a <pre>
Change-Id: Idbca6c091d61bf33174110dd3637a9a913e3503b
---
M includes/AutoLoader.php
M includes/content/CssContent.php
M includes/content/JavaScriptContent.php
M includes/content/JsonContent.php
M tests/phpunit/includes/content/JsonContentTest.php
5 files changed, 84 insertions(+), 150 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/75/160575/1
diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 7c5de2a..14ffd89 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -375,6 +375,7 @@
# includes/content
'AbstractContent' => 'includes/content/AbstractContent.php',
+ 'CodeContent' => 'includes/content/CodeContent.php',
'CodeContentHandler' => 'includes/content/CodeContentHandler.php',
'Content' => 'includes/content/Content.php',
'ContentHandler' => 'includes/content/ContentHandler.php',
diff --git a/includes/content/CssContent.php b/includes/content/CssContent.php
index 8290603..753986e 100644
--- a/includes/content/CssContent.php
+++ b/includes/content/CssContent.php
@@ -30,7 +30,7 @@
*
* @ingroup Content
*/
-class CssContent extends TextContent {
+class CssContent extends CodeContent {
/**
* @param string $text CSS code.
@@ -41,37 +41,10 @@
}
/**
- * Returns a Content object with pre-save transformations applied using
- * Parser::preSaveTransform().
- *
- * @param Title $title
- * @param User $user
- * @param ParserOptions $popts
- *
- * @return CssContent
- *
- * @see TextContent::preSaveTransform
- */
- public function preSaveTransform( Title $title, User $user,
ParserOptions $popts ) {
- global $wgParser;
- // @todo Make pre-save transformation optional for script pages
-
- $text = $this->getNativeData();
- $pst = $wgParser->preSaveTransform( $text, $title, $user,
$popts );
-
- return new static( $pst );
- }
-
- /**
* @return string CSS wrapped in a <pre> tag.
*/
protected function getHtml() {
- $html = "";
- $html .= "<pre class=\"mw-code mw-css\" dir=\"ltr\">\n";
- $html .= htmlspecialchars( $this->getNativeData() );
- $html .= "\n</pre>\n";
-
- return $html;
+ return $this->codeToPreElement( 'mw-css' );
}
}
diff --git a/includes/content/JavaScriptContent.php
b/includes/content/JavaScriptContent.php
index c0194c2..f044d11 100644
--- a/includes/content/JavaScriptContent.php
+++ b/includes/content/JavaScriptContent.php
@@ -30,7 +30,7 @@
*
* @ingroup Content
*/
-class JavaScriptContent extends TextContent {
+class JavaScriptContent extends CodeContent {
/**
* @param string $text JavaScript code.
@@ -41,36 +41,10 @@
}
/**
- * Returns a Content object with pre-save transformations applied using
- * Parser::preSaveTransform().
- *
- * @param Title $title
- * @param User $user
- * @param ParserOptions $popts
- *
- * @return JavaScriptContent
- */
- public function preSaveTransform( Title $title, User $user,
ParserOptions $popts ) {
- global $wgParser;
- // @todo Make pre-save transformation optional for script pages
- // See bug #32858
-
- $text = $this->getNativeData();
- $pst = $wgParser->preSaveTransform( $text, $title, $user,
$popts );
-
- return new static( $pst );
- }
-
- /**
* @return string JavaScript wrapped in a <pre> tag.
*/
protected function getHtml() {
- $html = "";
- $html .= "<pre class=\"mw-code mw-js\" dir=\"ltr\">\n";
- $html .= htmlspecialchars( $this->getNativeData() );
- $html .= "\n</pre>\n";
-
- return $html;
+ return $this->codeToPreElement( 'mw-js' );
}
}
diff --git a/includes/content/JsonContent.php b/includes/content/JsonContent.php
index b36827c..4b0becd 100644
--- a/includes/content/JsonContent.php
+++ b/includes/content/JsonContent.php
@@ -6,45 +6,70 @@
*
* @author Ori Livneh <[email protected]>
* @author Kunal Mehta <[email protected]>
+ * @author Yuri Astrakhan <yurik !@! wikimedia ! org>
*/
/**
* Represents the content of a JSON content.
* @since 1.24
*/
-class JsonContent extends TextContent {
+class JsonContent extends CodeContent {
+
+ /** @var \Status */
+ private $status;
+
+ /** @var int */
+ private $decodeOpts;
public function __construct( $text, $modelId = CONTENT_MODEL_JSON ) {
parent::__construct( $text, $modelId );
}
/**
- * Decodes the JSON into a PHP associative array.
- * @return array
- */
- public function getJsonData() {
- return FormatJson::decode( $this->getNativeData(), true );
- }
-
- /**
* @return bool Whether content is valid JSON.
*/
public function isValid() {
- return $this->getJsonData() !== null;
+ // It is possible for the valid JSON to be ok but not good,
+ // implying that it decodes ok but did not pass some additional
validation
+ return $this->getJson()->isGood();
+ }
+
+ public function decodeOptions( $options = false ) {
+ if ( $options !== false ) {
+ $this->decodeOpts = $options;
+ }
+ return $this->decodeOpts;
}
/**
- * Pretty-print JSON
- *
- * @return bool|null|string
+ * Decodes JSON content into the PHP data structure.
+ * @deprecated use getJson()
+ * @return array
*/
- public function beautifyJSON() {
- $decoded = FormatJson::decode( $this->getNativeData(), true );
- if ( !is_array( $decoded ) ) {
- return null;
- }
- return FormatJson::encode( $decoded, true );
+ public function getJsonData() {
+ return $this->getJson()->getValue();
+ }
+ /**
+ * Return the status object that will contain parsed data if ok, or the
error otherwise
+ * Note to inheritors: override parseJson() instead of this method
+ * @return Status
+ */
+ public function getJson() {
+ if ( $this->status === null ) {
+ $this->status = $this->parseJson();
+ }
+ return $this->status;
+ }
+
+ /**
+ * Convert native data string into the Status object.
+ * On success, status' value is set to the decoded value.
+ * Derived classes may override this method to add additional validation
+ * @return Status
+ */
+ protected function parseJson() {
+ return FormatJson::parse( $this->getNativeData(),
$this->decodeOptions() );
}
/**
@@ -55,66 +80,30 @@
* @return JsonContent
*/
public function preSaveTransform( Title $title, User $user,
ParserOptions $popts ) {
- return new static( $this->beautifyJSON() );
+ $data = $this->getJson();
+ if ( !$data->isOK() ) {
+ return $this;
+ }
+ // When saving, we are ok to save optimally-encoded,
non-pretty-printed data
+ $newText = FormatJson::encode( $data->getValue(), false,
FormatJson::ALL_OK );
+ if ( $this->getNativeData() === $newText ) {
+ return $this;
+ }
+ $new = new static( $newText, $this->getModel() );
+ $new->decodeOptions( $this->decodeOptions() );
+ return $new;
}
/**
- * Set the HTML and add the appropriate styles
- *
- *
- * @param Title $title
- * @param int $revId
- * @param ParserOptions $options
- * @param bool $generateHtml
- * @param ParserOutput $output
+ * @return string JavaScript wrapped in a <pre> tag.
*/
- protected function fillParserOutput( Title $title, $revId,
- ParserOptions $options, $generateHtml, ParserOutput &$output
- ) {
- if ( $generateHtml ) {
- $output->setText( $this->objectTable(
$this->getJsonData() ) );
- $output->addModuleStyles( 'mediawiki.content.json' );
- } else {
- $output->setText( '' );
- }
- }
- /**
- * Constructs an HTML representation of a JSON object.
- * @param array $mapping
- * @return string HTML
- */
- protected function objectTable( $mapping ) {
- $rows = array();
-
- foreach ( $mapping as $key => $val ) {
- $rows[] = $this->objectRow( $key, $val );
- }
- return Xml::tags( 'table', array( 'class' => 'mw-json' ),
- Xml::tags( 'tbody', array(), join( "\n", $rows ) )
- );
- }
-
- /**
- * Constructs HTML representation of a single key-value pair.
- * @param string $key
- * @param mixed $val
- * @return string HTML.
- */
- protected function objectRow( $key, $val ) {
- $th = Xml::elementClean( 'th', array(), $key );
- if ( is_array( $val ) ) {
- $td = Xml::tags( 'td', array(), self::objectTable( $val
) );
- } else {
- if ( is_string( $val ) ) {
- $val = '"' . $val . '"';
- } else {
- $val = FormatJson::encode( $val );
- }
-
- $td = Xml::elementClean( 'td', array( 'class' =>
'value' ), $val );
- }
-
- return Xml::tags( 'tr', array(), $th . $td );
+ protected function getHtml() {
+ $data = $this->getJson();
+ // Return original data if this is not a valid JSON
+ $text = !$data->isOK()
+ ? $this->getNativeData()
+ : FormatJson::encode( $data->getValue(), true,
FormatJson::UTF8_OK );
+ return $this->codeToPreElement( 'mw-json', $text );
}
}
diff --git a/tests/phpunit/includes/content/JsonContentTest.php
b/tests/phpunit/includes/content/JsonContentTest.php
index 6c77d1a..5f4ab20 100644
--- a/tests/phpunit/includes/content/JsonContentTest.php
+++ b/tests/phpunit/includes/content/JsonContentTest.php
@@ -8,27 +8,32 @@
/**
* @dataProvider provideValidConstruction
+ * @param $text
+ * @param $isValid
+ * @param $expected
*/
- public function testValidConstruct( $text, $modelId, $isValid,
$expected ) {
- $obj = new JsonContent( $text, $modelId );
- $this->assertEquals( $isValid, $obj->isValid() );
- $this->assertEquals( $expected, $obj->getJsonData() );
+ public function testValidConstruct( $text, $isValid, $expected ) {
+ $obj = new JsonContent( $text, CONTENT_MODEL_JSON );
+ $status = $obj->getJson();
+ $this->assertEquals( $isValid, $status->isOK() );
+ $this->assertEquals( $expected, $status->getValue() );
}
public function provideValidConstruction() {
return array(
- array( 'foo', CONTENT_MODEL_JSON, false, null ),
- array( FormatJson::encode( array() ),
CONTENT_MODEL_JSON, true, array() ),
- array( FormatJson::encode( array( 'foo' ) ),
CONTENT_MODEL_JSON, true, array( 'foo' ) ),
+ array( 'foo', false, null ),
+ array( FormatJson::encode( array() ), true, array() ),
+ array( FormatJson::encode( array( 'foo' ) ), true,
array( 'foo' ) ),
);
}
/**
* @dataProvider provideDataToEncode
*/
- public function testBeautifyUsesFormatJson( $data ) {
+ public function testPreSaveTransform( $data ) {
$obj = new JsonContent( FormatJson::encode( $data ) );
- $this->assertEquals( FormatJson::encode( $data, true ),
$obj->beautifyJSON() );
+ $newObj = $obj->preSaveTransform( $this->getMockTitle(),
$this->getMockUser(), $this->getMockParserOptions() );
+ $this->assertTrue( $newObj->equals( new JsonContent(
FormatJson::encode( $data, false, FormatJson::ALL_OK ) ) ) );
}
public function provideDataToEncode() {
@@ -39,15 +44,6 @@
array( array( 'baz' => 'foo', 'bar' ) ),
array( array( 'baz' => 1000, 'bar' ) ),
);
- }
-
- /**
- * @dataProvider provideDataToEncode
- */
- public function testPreSaveTransform( $data ) {
- $obj = new JsonContent( FormatJson::encode( $data ) );
- $newObj = $obj->preSaveTransform( $this->getMockTitle(),
$this->getMockUser(), $this->getMockParserOptions() );
- $this->assertTrue( $newObj->equals( new JsonContent(
FormatJson::encode( $data, true ) ) ) );
}
private function getMockTitle() {
@@ -61,6 +57,7 @@
->disableOriginalConstructor()
->getMock();
}
+
private function getMockParserOptions() {
return $this->getMockBuilder( 'ParserOptions' )
->disableOriginalConstructor()
--
To view, visit https://gerrit.wikimedia.org/r/160575
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idbca6c091d61bf33174110dd3637a9a913e3503b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Yurik <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
