jenkins-bot has submitted this change and it was merged.
Change subject: When a CollaborationListContent or CollaborationHubContent page
does not meet the schema, instead of throwing an exception it returns the
offending JSON.
......................................................................
When a CollaborationListContent or CollaborationHubContent page does not meet
the schema, instead of throwing an exception it returns the offending JSON.
This introduces a new top-level property in the object called "displaymode"
which is currently optional but will be required soon. The options are "normal"
(the default), "error" (used for outputting the error) and "members"
(CollaborationListContent only; not currently used but will replace the
ismemberlist bool).
Bug: T148085
Change-Id: I4f76f8f43405cf81eb84108a848a76e0255a76b7
---
M extension.json
M i18n/en.json
M i18n/qqq.json
M includes/content/CollaborationHubContent.php
M includes/content/CollaborationHubContentHandler.php
M includes/content/CollaborationHubContentSchema.php
M includes/content/CollaborationListContent.php
M includes/content/CollaborationListContentHandler.php
M includes/content/CollaborationListContentSchema.php
M tests/phpunit/CollaborationHubContentHandlerTest.php
10 files changed, 159 insertions(+), 116 deletions(-)
Approvals:
Isarra: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 1946eee..335ff7e 100644
--- a/extension.json
+++ b/extension.json
@@ -3,7 +3,7 @@
"version": "0.1",
"author": [ "Kim Schoonover", "Brian Wolff", "James Hare" ],
"url": "https://www.mediawiki.org/wiki/Extension:CollaborationKit";,
- "descriptionmsg": "colaborationkit-desc",
+ "descriptionmsg": "collaborationkit-desc",
"type": "other",
"license-name": "GPL-2.0+",
"MessagesDirs": {
diff --git a/i18n/en.json b/i18n/en.json
index 22e18d9..806b387 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -2,7 +2,7 @@
"@metadata": {
"authors": [ "Isarra", "Harej", "bawolff" ]
},
- "colaborationkit-desc": "Next-generation WikiProjects and collaborative
workspaces for wikis",
+ "collaborationkit-desc": "Next-generation WikiProjects and
collaborative workspaces for wikis",
"createcollaborationhub": "Create new Collaboration Hub project",
"createhubfeature": "Create a new Collaboration Hub feature",
"collaborationkit-createhub-title": "Page title",
@@ -35,6 +35,7 @@
"collaborationkit-createhubfeature-freetext": "Generic wiki page",
"collaborationkit-createhubfeature-articlelist": "List of pages",
"collaborationkit-list-isempty": "This list has no items in it.",
+ "collaborationkit-list-invalid": "This content does not meet the
requirements of the CollaborationListContent schema. This may happen as a
result of a software update. The content is reproduced below.",
"collaborationkit-list-notlist": "[[$1]] is not a CollaborationKit list
page",
"collaborationkit-list-taglist": "'''{{PLURAL:$2|Tagged:|Tags:}}''' $1
",
"collaborationkit-list-toomanytags": "You are not allowed to specify
more than {{PLURAL:$1|one tag|$1 tags}}",
@@ -77,6 +78,7 @@
"collaborationkit-hub-toc-label": "Project contents",
"collaborationkit-hub-addpage": "Add feature",
"collaborationkit-hub-manage": "Manage project",
+ "collaborationkit-hub-invalid": "This content does not meet the
requirements of the CollaborationHubContent schema. This may happen as a result
of a software update. The content is reproduced below.",
"collaborationkit-subpage-toc-label": "Part of a project:",
"collaborationkit-red1": "Dark red",
"collaborationkit-red2": "Red",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index a83df17..5fb5b83 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -2,7 +2,7 @@
"@metadata": {
"authors": [ "Isarra", "bawolff" ]
},
- "colaborationkit-desc":
"{{desc|name=CollaborationKit|url=https://www.mediawiki.org/wiki/Extension:CollaborationKit}}";,
+ "collaborationkit-desc":
"{{desc|name=CollaborationKit|url=https://www.mediawiki.org/wiki/Extension:CollaborationKit}}";,
"createcollaborationhub": "Header for Special:CreateCollaborationHub",
"createhubfeature": "Header for Special:CreateHubFeature",
"collaborationkit-createhub-title": "Label for target page title input
on Special:CreateCollaborationHub",
@@ -35,6 +35,7 @@
"collaborationkit-createhubfeature-invalidcontenttype": "Error message
on Special:CreateHubFeature in the unlikely event that an invalid content type
is specified for a feature",
"collaborationkit-createhubfeature-editsummary": "Edit summary used by
Special:CreateHubFeature when creating a new feature",
"collaborationkit-list-isempty": "Shown on lists that are empty
immediately after the description",
+ "collaborationkit-list-invalid": "Message shown on lists that do not
comply with the JSON Schema; shown before displaying the offending content",
"collaborationkit-list-notlist": "Error when
<nowiki>{{#transcludelist:page}}</nowiki> is given a page that is not a
CollaborationKit list page. $1 Name of page given.",
"collaborationkit-list-taglist": "Box for showing tags a specific item
has on a list. $1 = comma separated list of tags. $2 = number of tags",
"collaborationkit-list-toomanytags": "Error if
<nowiki>{{#transcludelist:...}}</nowiki> is given more than the allowed number
of tag arguments. $1 - number of tags allowed. $2 - number of tags given.",
@@ -77,6 +78,7 @@
"collaborationkit-hub-toc-label": "Label for the toc on a Collaboration
Hub mainpage",
"collaborationkit-hub-addpage": "Label for button/link to add a new
subpage/feature to a Collaboration Hub",
"collaborationkit-hub-manage": "Label for extra button/link to edit
Collaboration Hub",
+ "collaborationkit-hub-invalid": "Message shown on hubs that do not
comply with the JSON Schema; shown before displaying the offending content",
"collaborationkit-subpage-toc-label": "Label for the toc on a
Collaboration Hub subpage",
"collaborationkit-red1": "Color label",
"collaborationkit-red2": "Color label",
diff --git a/includes/content/CollaborationHubContent.php
b/includes/content/CollaborationHubContent.php
index 91fa4a5..5197af0 100644
--- a/includes/content/CollaborationHubContent.php
+++ b/includes/content/CollaborationHubContent.php
@@ -42,6 +42,9 @@
/** @var string */
protected $themeColour;
+ /** @var $displaymode String How to display contents */
+ protected $displaymode;
+
/** @var bool Whether contents have been populated */
protected $decoded = false;
@@ -87,18 +90,19 @@
*/
public function isValid() {
$hubSchema = include __DIR__ .
'/CollaborationHubContentSchema.php';
- $this->decode();
- if ( $this->decoded ) {
- $jsonParse = $this->getData();
- if ( $jsonParse->isGood() ) {
- // Forcing the object to become an array
- $jsonAsArray = json_decode( json_encode(
$jsonParse->getValue() ), true );
- try {
- EventLogging::schemaValidate(
$jsonAsArray, $hubSchema );
- return true;
- } catch ( JsonSchemaException $e ) {
- return false;
- }
+ $jsonParse = $this->getData();
+ if ( $jsonParse->isGood() ) {
+ // TODO: The schema should be checking for required
fields but for some reason that doesn't work
+ if ( !isset( $jsonParse->value->content ) ) {
+ return false;
+ }
+ // Forcing the object to become an array
+ $jsonAsArray = json_decode( json_encode(
$jsonParse->getValue() ), true );
+ try {
+ EventLogging::schemaValidate( $jsonAsArray,
$hubSchema );
+ return true;
+ } catch ( JsonSchemaException $e ) {
+ return false;
}
return false;
}
@@ -115,31 +119,36 @@
$jsonParse = $this->getData();
$data = $jsonParse->isGood() ? $jsonParse->getValue() : null;
if ( $data ) {
- $this->displayName = isset( $data->display_name ) ?
$data->display_name : '';
- $this->introduction = isset( $data->introduction ) ?
$data->introduction : '';
- $this->footer = isset( $data->footer ) ? $data->footer
: '';
- $this->image = isset( $data->image ) ? $data->image :
'none';
-
- // Set colour to default if empty or missing
- if ( !isset( $data->colour ) || $data->colour == '' ) {
- $this->themeColour = 'blue5';
+ if ( !$this->isValid() ) {
+ $this->displaymode = 'error';
+ $this->errortext = FormatJson::encode( $data,
true, FormatJson::ALL_OK );
} else {
- $this->themeColour = $data->colour;
- }
+ $this->displayName = isset( $data->display_name
) ? $data->display_name : '';
+ $this->introduction = isset(
$data->introduction ) ? $data->introduction : '';
+ $this->footer = isset( $data->footer ) ?
$data->footer : '';
+ $this->image = isset( $data->image ) ?
$data->image : 'none';
- if ( isset( $data->content ) && is_array(
$data->content ) ) {
- $this->content = [];
- foreach ( $data->content as $itemObject ) {
- if ( !is_object( $itemObject ) ) { //
Malformed item
- $this->content = null;
- break;
+ // Set colour to default if empty or missing
+ if ( !isset( $data->colour ) || $data->colour
== '' ) {
+ $this->themeColour = 'blue5';
+ } else {
+ $this->themeColour = $data->colour;
+ }
+
+ if ( isset( $data->content ) && is_array(
$data->content ) ) {
+ $this->content = [];
+ foreach ( $data->content as $itemObject
) {
+ if ( !is_object( $itemObject )
) { // Malformed item
+ $this->content = null;
+ break;
+ }
+ $item = [];
+ $item['title'] = isset(
$itemObject->title ) ? $itemObject->title : null;
+ $item['image'] = isset(
$itemObject->image ) ? $itemObject->image : null;
+ $item['displayTitle'] = isset(
$itemObject->display_title ) ? $itemObject->display_title : null;
+
+ $this->content[] = $item;
}
- $item = [];
- $item['title'] = isset(
$itemObject->title ) ? $itemObject->title : null;
- $item['image'] = isset(
$itemObject->image ) ? $itemObject->image : null;
- $item['displayTitle'] = isset(
$itemObject->display_title ) ? $itemObject->display_title : null;
-
- $this->content[] = $item;
}
}
}
@@ -211,68 +220,76 @@
// Dummy parse intro and footer to get categories and whatnot
$output = $wgParser->parse( $this->getIntroduction() .
$this->getFooter(), $title, $options, true, true, $revId );
$html = '';
- // set up hub with theme stuff
- $html .= Html::openElement(
- 'div',
- [ 'class' => $this->getHubClasses() ]
- );
- // get page image
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-header-image' ],
- $this->getParsedImage( $this->getImage(), 200 )
- );
- // get members list
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-members' ],
- $this->getMembersBlock( $title, $options )
- );
- // get parsed intro
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-intro' ],
- $this->getParsedIntroduction( $title, $options )
- );
- // get announcements
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-announcements' ],
- $this->getParsedAnnouncements( $title, $options )
- );
- // get table of contents
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-toc' ],
- $this->getTableOfContents( $title, $options )
- );
- // get transcluded content
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-content' ],
- $this->getParsedContent( $title, $options, $output )
- );
- // get footer
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-footer' ],
- $this->getParsedFooter( $title, $options )
- );
- $html .= Html::rawElement(
- 'div',
- [ 'class' => 'wp-footeractions' ],
- $this->getSecondFooter( $title )
- );
- $html .= Html::closeElement( 'div' );
- $output->setText( $html );
+ // If error, then bypass all this and just show the offending
JSON
- // Add some style stuff
- $output->addModuleStyles( 'ext.CollaborationKit.hub.styles' );
- $output->addModules( 'ext.CollaborationKit.icons' );
- $output->addModules( 'ext.CollaborationKit.blots' );
- $output->addModules( 'ext.CollaborationKit.list.styles' );
- $output->setEnableOOUI( true );
+ if ( $this->displaymode == 'error' ) {
+ $html = '<div class=errorbox>' . wfMessage(
'collaborationkit-hub-invalid' ) . '</div>\n<pre>' . $this->errortext .
'</pre>';
+ $output->setText( $html );
+ } else {
+ // set up hub with theme stuff
+ $html .= Html::openElement(
+ 'div',
+ [ 'class' => $this->getHubClasses() ]
+ );
+ // get page image
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-header-image' ],
+ $this->getParsedImage( $this->getImage(), 200 )
+ );
+ // get members list
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-members' ],
+ $this->getMembersBlock( $title, $options )
+ );
+ // get parsed intro
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-intro' ],
+ $this->getParsedIntroduction( $title, $options )
+ );
+ // get announcements
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-announcements' ],
+ $this->getParsedAnnouncements( $title, $options
)
+ );
+ // get table of contents
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-toc' ],
+ $this->getTableOfContents( $title, $options )
+ );
+ // get transcluded content
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-content' ],
+ $this->getParsedContent( $title, $options,
$output )
+ );
+ // get footer
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-footer' ],
+ $this->getParsedFooter( $title, $options )
+ );
+ $html .= Html::rawElement(
+ 'div',
+ [ 'class' => 'wp-footeractions' ],
+ $this->getSecondFooter( $title )
+ );
+ $html .= Html::closeElement( 'div' );
+
+ $output->setText( $html );
+
+ // Add some style stuff
+ $output->addModuleStyles(
'ext.CollaborationKit.hub.styles' );
+ $output->addModules( 'ext.CollaborationKit.icons' );
+ $output->addModules( 'ext.CollaborationKit.blots' );
+ $output->addModules( 'ext.CollaborationKit.list.styles'
);
+ $output->setEnableOOUI( true );
+ }
}
/**
diff --git a/includes/content/CollaborationHubContentHandler.php
b/includes/content/CollaborationHubContentHandler.php
index cdf2b14..118c380 100644
--- a/includes/content/CollaborationHubContentHandler.php
+++ b/includes/content/CollaborationHubContentHandler.php
@@ -31,9 +31,7 @@
public function unserializeContent( $text, $format = null ) {
$this->checkFormat( $format );
$content = new CollaborationHubContent( $text );
- if ( !$content->isValid() ) {
- throw new MWContentSerializationException( 'The
collaborationhub content is invalid.' );
- }
+ // Deliberately not validating at this step; validation is done
later.
return $content;
}
diff --git a/includes/content/CollaborationHubContentSchema.php
b/includes/content/CollaborationHubContentSchema.php
index ee959c4..8f8d1de 100644
--- a/includes/content/CollaborationHubContentSchema.php
+++ b/includes/content/CollaborationHubContentSchema.php
@@ -2,6 +2,12 @@
return [
'type' => 'object',
'properties' => [
+ 'displaymode' => [
+ 'enum' => [
+ 0 => 'normal',
+ 1 => 'error'
+ ]
+ ],
'introduction' => [
'type' => 'string',
],
diff --git a/includes/content/CollaborationListContent.php
b/includes/content/CollaborationListContent.php
index b45422b..438db6c 100644
--- a/includes/content/CollaborationListContent.php
+++ b/includes/content/CollaborationListContent.php
@@ -22,6 +22,8 @@
protected $options;
/** @var $items Array List of items */
protected $items;
+ /** @var $displaymode String The variety of list */
+ protected $displaymode;
function __construct( $text, $type = 'CollaborationListContent' ) {
parent::__construct( $text, $type );
@@ -41,6 +43,10 @@
return false;
}
$data = $status->value;
+ // TODO: The schema should be checking for required fields but
for some reason that doesn't work
+ if ( !isset( $data->description ) || !isset( $data->items ) ||
!isset( $data->options ) ) {
+ return false;
+ }
$jsonAsArray = json_decode( json_encode( $data ), true );
try {
EventLogging::schemaValidate( $jsonAsArray, $listSchema
);
@@ -109,13 +115,16 @@
if ( $this->decoded ) {
return;
}
- if ( !$this->isValid() ) {
- throw new Exception( "Can't decode invalid content" );
- }
$data = $this->getData()->value;
- $this->description = $data->description;
- $this->options = $data->options;
- $this->items = $data->items;
+ if ( !$this->isValid() ) {
+ $this->displaymode = 'error';
+ $this->errortext = FormatJson::encode( $data, true,
FormatJson::ALL_OK );
+ } else {
+ $this->displaymode = 'normal'; // For now, while this
field is still optional
+ $this->description = $data->description;
+ $this->options = $data->options;
+ $this->items = $data->items;
+ }
$this->decoded = true;
}
@@ -182,6 +191,14 @@
$maxItems = $options['maxItems'];
$includeDesc = $options['includeDesc'];
+ // If this is an error-type list (i.e. a schema-violating blob),
+ // just return the plain JSON.
+
+ if ( $this->displaymode == 'error' ) {
+ $errorWikitext = '<div class=errorbox>' . wfMessage(
'collaborationkit-list-invalid' ) . '</div>\n<pre>' . $this->errortext .
'</pre>';
+ return $errorWikitext;
+ }
+
// Hack to force style loading even when we don't have a Parser
reference.
$text = "<collaborationkitloadliststyles/>\n";
diff --git a/includes/content/CollaborationListContentHandler.php
b/includes/content/CollaborationListContentHandler.php
index b0a98b0..674638e 100644
--- a/includes/content/CollaborationListContentHandler.php
+++ b/includes/content/CollaborationListContentHandler.php
@@ -45,9 +45,7 @@
$text = FormatJson::encode( $data );
}
$content = new CollaborationListContent( $text );
- if ( !$content->isValid() ) {
- throw new MWContentSerializationException( 'The
collaborationlist content is invalid.' );
- }
+ // Deliberately not validating at this step; validation is done
later.
return $content;
}
diff --git a/includes/content/CollaborationListContentSchema.php
b/includes/content/CollaborationListContentSchema.php
index 0766013..6dfa7ce 100644
--- a/includes/content/CollaborationListContentSchema.php
+++ b/includes/content/CollaborationListContentSchema.php
@@ -3,6 +3,16 @@
'type' => 'object',
'required' => [ 'description', 'items', 'options' ],
'properties' => [
+ 'displaymode' => [
+ 'enum' => [
+ 0 => 'normal',
+ 1 => 'members',
+ 2 => 'error'
+ ]
+ ],
+ 'errortext' => [
+ 'type' => 'string'
+ ],
'description' => [
'type' => 'string'
],
diff --git a/tests/phpunit/CollaborationHubContentHandlerTest.php
b/tests/phpunit/CollaborationHubContentHandlerTest.php
index 7be872c..cd59255 100644
--- a/tests/phpunit/CollaborationHubContentHandlerTest.php
+++ b/tests/phpunit/CollaborationHubContentHandlerTest.php
@@ -11,13 +11,6 @@
$this->handler = TestingAccessWrapper::newFromObject( $handler
);
}
- /**
- * @expectedException MWContentSerializationException
- */
- public function testUnserializeContent() {
- $this->handler->unserializeContent( 'There once was a horse
named bob.' );
- }
-
public function testMakeEmptyContent() {
$empty = $this->handler->makeEmptyContent();
$this->assertTrue( $empty->isValid() );
--
To view, visit https://gerrit.wikimedia.org/r/316196
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4f76f8f43405cf81eb84108a848a76e0255a76b7
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/CollaborationKit
Gerrit-Branch: master
Gerrit-Owner: Harej <jamesmh...@gmail.com>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Harej <jamesmh...@gmail.com>
Gerrit-Reviewer: Isarra <zhoris...@gmail.com>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
