Merge "Customise error message for invalid JSON, add hook"

RELEASE-NOTES-1.39

@@ -91,6 +91,8 @@ For notes on 1.38.x and older releases, see HISTORY.
 * …
 
 === New developer features in 1.39 ===
+* JsonValidateSaveHook has been added to allow extensions to set additional
+  pre-save validations for specific JSON pages (T313254)
 * …
 
 === External library changes in 1.39 ===

includes/HookContainer/HookRunner.php

@@ -6,9 +6,11 @@ use Article;
 use Config;
 use File;
 use IContextSource;
+use JsonContent;
 use ManualLogEntry;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\Linker\LinkTarget;
+use MediaWiki\Page\PageIdentity;
 use MediaWiki\Page\ProperPageIdentity;
 use MediaWiki\Permissions\Authority;
 use MediaWiki\ResourceLoader as RL;
@@ -71,6 +73,7 @@ class HookRunner implements
 	\MediaWiki\Content\Hook\GetContentModelsHook,
 	\MediaWiki\Content\Hook\GetDifferenceEngineHook,
 	\MediaWiki\Content\Hook\GetSlotDiffRendererHook,
+	\MediaWiki\Content\Hook\JsonValidateSaveHook,
 	\MediaWiki\Content\Hook\PageContentLanguageHook,
 	\MediaWiki\Content\Hook\PlaceNewSectionHook,
 	\MediaWiki\Content\Hook\SearchDataForIndexHook,
@@ -2226,6 +2229,13 @@ class HookRunner implements
 		);
 	}
 
+	public function onJsonValidateSave( JsonContent $content, PageIdentity $pageIdentity, StatusValue $status ) {
+		return $this->container->run(
+			'JsonValidateSave',
+			[ $content, $pageIdentity, &$status ]
+		);
+	}
+
 	public function onLanguageGetNamespaces( &$namespaces ) {
 		return $this->container->run(
 			'LanguageGetNamespaces',

includes/content/Hook/JsonValidateSaveHook.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace MediaWiki\Content\Hook;
+
+use JsonContent;
+use MediaWiki\Page\PageIdentity;
+use StatusValue;
+
+/**
+ * This is a hook handler interface, see docs/Hooks.md.
+ * Use the hook name "JsonValidateSaveHook" to register handlers implementing this interface.
+ *
+ * @stable to implement
+ * @ingroup Hooks
+ */
+interface JsonValidateSaveHook {
+	/**
+	 * Use this hook to add additional validations for JSON content pages.
+	 * This hook is only called if JSON syntax validity and other contentmodel-specific validations
+	 * are passing.
+	 *
+	 * @since 1.39
+	 *
+	 * @param JsonContent $content
+	 * @param PageIdentity $pageIdentity
+	 * @param StatusValue $status Fatal errors only would trigger validation failure as $status is checked with isOK()
+	 * @return bool|void True or no return value to continue
+	 */
+	public function onJsonValidateSave( JsonContent $content, PageIdentity $pageIdentity, StatusValue $status );
+}

includes/content/JsonContentHandler.php

@@ -20,11 +20,12 @@
 
 use MediaWiki\Content\Renderer\ContentParseParams;
 use MediaWiki\Content\Transform\PreSaveTransformParams;
+use MediaWiki\Content\ValidationParams;
 
 /**
  * Content handler for JSON text.
  *
- * Useful for maintaining JSON that can be viewed and edit directly by users.
+ * Useful for maintaining JSON that can be viewed and edited directly by users.
  *
  * @author Ori Livneh <ori@wikimedia.org>
  * @author Kunal Mehta <legoktm@gmail.com>
@@ -65,6 +66,25 @@ class JsonContentHandler extends CodeContentHandler {
 		return true;
 	}
 
+	/**
+	 * @param Content $content
+	 * @param ValidationParams $validationParams
+	 * @return StatusValue
+	 */
+	public function validateSave( Content $content, ValidationParams $validationParams ) {
+		$status = parent::validateSave( $content, $validationParams );
+		'@phan-var JsonContent $content';
+		if ( !$status->isOK() ) {
+			if ( !$content->getData()->isGood() ) {
+				return StatusValue::newFatal( $content->getData()->getMessage( 'invalid-json-data' ) );
+			} else {
+				return $status;
+			}
+		}
+		$this->getHookRunner()->onJsonValidateSave( $content, $validationParams->getPageIdentity(), $status );
+		return $status;
+	}
+
 	public function preSaveTransform(
 		Content $content,
 		PreSaveTransformParams $pstParams

languages/i18n/en.json

@@ -786,6 +786,7 @@
 	"defaultmessagetext": "Default message text",
 	"content-failed-to-parse": "Failed to parse $2 content for $1 model: $3",
 	"invalid-content-data": "Invalid content data",
+	"invalid-json-data": "Invalid JSON: $1",
 	"content-not-allowed-here": "\"$1\" content is not allowed on page [[:$2]] in slot \"$3\"",
 	"confirmleave-warning": "Leaving this page may cause you to lose any changes you have made.",
 	"editpage-invalidcontentmodel-title": "Content model not supported",

languages/i18n/qqq.json

@@ -1026,6 +1026,7 @@
 	"defaultmessagetext": "Caption above the default message text shown on the left-hand side of a diff displayed after clicking \"Show changes\" when creating a new page in the MediaWiki: namespace",
 	"content-failed-to-parse": "Error message indicating that the page's content can not be saved because it is syntactically invalid. This may occurr for content types using serialization or a strict markup syntax.\n\nParameters:\n* $1 – content model, any one of the following messages:\n** {{msg-mw|Content-model-wikitext}}\n** {{msg-mw|Content-model-javascript}}\n** {{msg-mw|Content-model-css}}\n** {{msg-mw|Content-model-json}}\n** {{msg-mw|Content-model-text}}\n* $2 – content format as MIME type (e.g. <code>text/css</code>)\n* $3 – specific error message",
 	"invalid-content-data": "Error message indicating that the page's content can not be saved because it is invalid. This may occurr for content types with internal consistency constraints.",
+	"invalid-json-data": "Error message indicating the page's content is not valid JSON.\n\nParameters:\n* $1 - specific validation error message",
 	"content-not-allowed-here": "Error message indicating that the desired content model is not supported in given location.\n* $1 - the human readable name of the content model: {{msg-mw|Content-model-wikitext}}, {{msg-mw|Content-model-javascript}}, {{msg-mw|Content-model-json}}, {{msg-mw|Content-model-css}} or {{msg-mw|Content-model-text}}\n* $2 - the title of the page in question\n* $3 - the role name of the [[:mw:Manual:Slot|slot]] the content is not allowed in",
 	"confirmleave-warning": "Generic warning message that may be shown when attempting to leave some pages with unsaved changes.",
 	"editpage-invalidcontentmodel-title": "Title of error page shown when using an unrecognized content model on EditPage",

tests/phpunit/includes/api/ApiChangeContentModelTest.php

@@ -114,7 +114,7 @@ class ApiChangeContentModelTest extends ApiTestCase {
 			'`PageWithTextThatIsNotValidJSON` should be wikitext at first'
 		);
 
-		$this->setExpectedApiException( 'invalid-content-data' );
+		$this->setExpectedApiException( wfMessage( 'invalid-json-data', wfMessage( 'json-error-syntax' ) ) );
 		$this->doApiRequestWithToken( [
 				'action' => 'changecontentmodel',
 				'summary' => __METHOD__,

tests/phpunit/includes/content/ContentModelChangeTest.php

@@ -101,10 +101,7 @@ class ContentModelChangeTest extends MediaWikiIntegrationTestCase {
 			__METHOD__ . ' comment',
 			false
 		);
-		$this->assertSame(
-			'invalid-content-data',
-			$status->getErrors()[0]['message']
-		);
+		$this->assertStatusError( 'invalid-json-data', $status );
 	}
 
 	/**

tests/phpunit/includes/content/JsonContentHandlerIntegrationTest.php

@@ -1,5 +1,9 @@
 <?php
 
+use MediaWiki\Content\ValidationParams;
+use MediaWiki\Page\PageIdentity;
+use MediaWiki\Page\PageIdentityValue;
+
 class JsonContentHandlerIntegrationTest extends MediaWikiLangTestCase {
 
 	public function provideDataAndParserText() {
@@ -64,4 +68,38 @@ class JsonContentHandlerIntegrationTest extends MediaWikiLangTestCase {
 		$this->assertInstanceOf( ParserOutput::class, $parserOutput );
 		$this->assertEquals( $expected, $parserOutput->getText() );
 	}
+
+	/**
+	 * @covers JsonContentHandler::validateSave
+	 */
+	public function testValidateSave() {
+		$handler = new JsonContentHandler();
+		$validationParams = new ValidationParams(
+			PageIdentityValue::localIdentity( 123, NS_MEDIAWIKI, 'Config.json' ),
+			0
+		);
+
+		$validJson = new JsonContent( FormatJson::encode( [ 'test' => 'value' ] ) );
+		$invalidJson = new JsonContent( '{"key":' );
+
+		$this->assertStatusGood( $handler->validateSave( $validJson, $validationParams ) );
+		$this->assertStatusNotOK( $handler->validateSave( $invalidJson, $validationParams ) );
+
+		$this->setTemporaryHook(
+			'JsonValidateSave',
+			static function ( JsonContent $content, PageIdentity $pageIdentity, StatusValue $status )
+			{
+				if ( $pageIdentity->getDBkey() === 'Config.json' &&
+					!isset( $content->getData()->getValue()->foo ) ) {
+					$status->fatal( 'missing-key-foo' );
+				}
+			}
+		);
+
+		$this->assertStatusNotOK( $handler->validateSave( $validJson, $validationParams ) );
+		$this->assertStatusError( 'invalid-json-data',
+			$handler->validateSave( $invalidJson, $validationParams ) );
+		$this->assertStatusError( 'missing-key-foo',
+			$handler->validateSave( $validJson, $validationParams ) );
+	}
 }