Merge "Split validation and normalization into separate services"

includes/TemplateDataBlob.php

@@ -94,8 +94,17 @@ class TemplateDataBlob {
 	 * @return Status
 	 */
 	protected function parse(): Status {
-		$validator = new TemplateDataValidator();
+		$deprecatedTypes = array_keys( TemplateDataNormalizer::DEPRECATED_PARAMETER_TYPES );
-		return $validator->validate( $this->data );
+		$validator = new TemplateDataValidator( $deprecatedTypes );
+		$status = $validator->validate( $this->data );
+
+		if ( $status->isOK() ) {
+			$lang = MediaWikiServices::getInstance()->getContentLanguage();
+			$normalizer = new TemplateDataNormalizer( $lang->getCode() );
+			$normalizer->normalize( $this->data );
+		}
+
+		return $status;
 	}
 
 	/**

includes/TemplateDataNormalizer.php

@@ -0,0 +1,100 @@
+<?php
+
+namespace MediaWiki\Extension\TemplateData;
+
+use stdClass;
+
+class TemplateDataNormalizer {
+
+	public const DEPRECATED_PARAMETER_TYPES = [
+		'string/line' => 'line',
+		'string/wiki-page-name' => 'wiki-page-name',
+		'string/wiki-user-name' => 'wiki-user-name',
+		'string/wiki-file-name' => 'wiki-file-name',
+	];
+
+	/** @var string */
+	private string $contentLanguageCode;
+
+	/**
+	 * @param string $contentLanguageCode
+	 */
+	public function __construct( string $contentLanguageCode ) {
+		$this->contentLanguageCode = $contentLanguageCode;
+	}
+
+	/**
+	 * @param stdClass $data Expected to be valid according to the {@see TemplateDataValidator}
+	 */
+	public function normalize( stdClass $data ) {
+		$data->description ??= null;
+		$data->sets ??= [];
+		$data->maps ??= (object)[];
+		$data->format ??= null;
+
+		$this->normaliseInterfaceText( $data->description );
+		foreach ( $data->sets as $setObj ) {
+			$this->normaliseInterfaceText( $setObj->label );
+		}
+
+		if ( isset( $data->params ) ) {
+			foreach ( $data->params as $param ) {
+				if ( isset( $param->inherits ) && isset( $data->params->{ $param->inherits } ) ) {
+					$parent = $data->params->{ $param->inherits };
+					foreach ( $parent as $key => $value ) {
+						if ( !isset( $param->$key ) ) {
+							$param->$key = is_object( $parent->$key ) ?
+								clone $parent->$key :
+								$parent->$key;
+						}
+					}
+					unset( $param->inherits );
+				}
+				$this->normalizeParameter( $param );
+			}
+		}
+	}
+
+	/**
+	 * @param stdClass $paramObj
+	 */
+	private function normalizeParameter( stdClass $paramObj ) {
+		$paramObj->label ??= null;
+		$paramObj->description ??= null;
+		$paramObj->required ??= false;
+		$paramObj->suggested ??= false;
+		$paramObj->deprecated ??= false;
+		$paramObj->aliases ??= [];
+		$paramObj->type ??= 'unknown';
+		$paramObj->autovalue ??= null;
+		$paramObj->default ??= null;
+		$paramObj->suggestedvalues ??= [];
+		$paramObj->example ??= null;
+
+		$this->normaliseInterfaceText( $paramObj->label );
+		$this->normaliseInterfaceText( $paramObj->description );
+		$this->normaliseInterfaceText( $paramObj->default );
+		$this->normaliseInterfaceText( $paramObj->example );
+
+		foreach ( $paramObj->aliases as &$alias ) {
+			if ( is_int( $alias ) ) {
+				$alias = (string)$alias;
+			}
+		}
+
+		// Map deprecated types to newer versions
+		if ( isset( self::DEPRECATED_PARAMETER_TYPES[$paramObj->type] ) ) {
+			$paramObj->type = self::DEPRECATED_PARAMETER_TYPES[$paramObj->type];
+		}
+	}
+
+	/**
+	 * @param string|stdClass &$text
+	 */
+	private function normaliseInterfaceText( &$text ) {
+		if ( is_string( $text ) ) {
+			$text = (object)[ $this->contentLanguageCode => $text ];
+		}
+	}
+
+}

includes/TemplateDataValidator.php

@@ -2,7 +2,6 @@
 
 namespace MediaWiki\Extension\TemplateData;
 
-use MediaWiki\MediaWikiServices;
 use Status;
 use stdClass;
 
@@ -53,12 +52,15 @@ class TemplateDataValidator {
 		'wiki-template-name',
 	];
 
-	private const DEPRECATED_TYPES_MAP = [
+	/** @var string[] */
-		'string/line' => 'line',
+	private $validParameterTypes;
-		'string/wiki-page-name' => 'wiki-page-name',
+
-		'string/wiki-user-name' => 'wiki-user-name',
+	/**
-		'string/wiki-file-name' => 'wiki-file-name',
+	 * @param string[] $additionalParameterTypes
-	];
+	 */
+	public function __construct( array $additionalParameterTypes ) {
+		$this->validParameterTypes = array_merge( self::VALID_TYPES, $additionalParameterTypes );
+	}
 
 	/**
 	 * @param mixed $data
@@ -86,21 +88,17 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type', 'description',
 					'string|object' );
 			}
-			$data->description = $this->normaliseInterfaceText( $data->description );
-		} else {
-			$data->description = null;
 		}
 
 		// Root.format
-		if ( !isset( $data->format ) ) {
+		if ( isset( $data->format ) ) {
-			$data->format = null;
+			if ( !is_string( $data->format ) ||
-		} elseif ( !is_string( $data->format ) ||
+				!( isset( self::PREDEFINED_FORMATS[$data->format] ) ||
-			// @phan-suppress-next-line PhanImpossibleCondition
+					$this->isValidCustomFormatString( $data->format )
-			!( isset( self::PREDEFINED_FORMATS[$data->format] ) ||
+				)
-				$this->isValidCustomFormatString( $data->format )
+			) {
-			)
+				return Status::newFatal( 'templatedata-invalid-format', 'format' );
-		) {
+			}
-			return Status::newFatal( 'templatedata-invalid-format', 'format' );
 		}
 
 		// Root.params
@@ -112,31 +110,10 @@ class TemplateDataValidator {
 			return Status::newFatal( 'templatedata-invalid-type', 'params', 'object' );
 		}
 
-		// Deep clone
+		return $this->validateParameters( $data->params ) ??
-		// We need this to determine whether a property was originally set
+			$this->validateParameterOrder( $data->paramOrder ?? null, $data->params ) ??
-		// to decide whether 'inherits' will add it or not.
+			$this->validateSets( $data->sets ?? [], $data->params ) ??
-		$unnormalizedParams = unserialize( serialize( $data->params ) );
+			$this->validateMaps( $data->maps ?? (object)[], $data->params ) ??
-
-		$status = $this->validateParameters( $data->params );
-		if ( $status ) {
-			return $status;
-		}
-
-		foreach ( $data->params as $paramName => $param ) {
-			if ( isset( $param->inherits ) ) {
-				$parentParam = $data->params->{ $param->inherits };
-				foreach ( $parentParam as $key => $value ) {
-					if ( !isset( $unnormalizedParams->$paramName->$key ) ) {
-						$param->$key = is_object( $value ) ? clone $value : $value;
-					}
-				}
-				unset( $param->inherits );
-			}
-		}
-
-		return $this->validateParameterOrder( $data->paramOrder ?? null, $data->params ) ??
-			$this->validateSets( $data->sets, $data->params ) ??
-			$this->validateMaps( $data->maps, $data->params ) ??
 			Status::newGood();
 	}
 
@@ -184,9 +161,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.label", 'string|object' );
 			}
-			$param->label = $this->normaliseInterfaceText( $param->label );
-		} else {
-			$param->label = null;
 		}
 
 		// Param.required
@@ -195,8 +169,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.required", 'boolean' );
 			}
-		} else {
-			$param->required = false;
 		}
 
 		// Param.suggested
@@ -205,8 +177,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.suggested", 'boolean' );
 			}
-		} else {
-			$param->suggested = false;
 		}
 
 		// Param.description
@@ -215,9 +185,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.description", 'string|object' );
 			}
-			$param->description = $this->normaliseInterfaceText( $param->description );
-		} else {
-			$param->description = null;
 		}
 
 		// Param.example
@@ -226,9 +193,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.example", 'string|object' );
 			}
-			$param->example = $this->normaliseInterfaceText( $param->example );
-		} else {
-			$param->example = null;
 		}
 
 		// Param.deprecated
@@ -237,8 +201,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.deprecated", 'boolean|string' );
 			}
-		} else {
-			$param->deprecated = false;
 		}
 
 		// Param.aliases
@@ -247,16 +209,12 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.aliases", 'array' );
 			}
-			foreach ( $param->aliases as $i => &$alias ) {
+			foreach ( $param->aliases as $i => $alias ) {
-				if ( is_int( $alias ) ) {
+				if ( !is_int( $alias ) && !is_string( $alias ) ) {
-					$alias = (string)$alias;
-				} elseif ( !is_string( $alias ) ) {
 					return Status::newFatal( 'templatedata-invalid-type',
 						"params.{$paramName}.aliases[$i]", 'int|string' );
 				}
 			}
-		} else {
-			$param->aliases = [];
 		}
 
 		// Param.autovalue
@@ -266,8 +224,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.autovalue", 'string' );
 			}
-		} else {
-			$param->autovalue = null;
 		}
 
 		// Param.default
@@ -276,9 +232,6 @@ class TemplateDataValidator {
 				return Status::newFatal( 'templatedata-invalid-type',
 					"params.{$paramName}.default", 'string|object' );
 			}
-			$param->default = $this->normaliseInterfaceText( $param->default );
-		} else {
-			$param->default = null;
 		}
 
 		// Param.type
@@ -288,17 +241,10 @@ class TemplateDataValidator {
 					"params.{$paramName}.type", 'string' );
 			}
 
-			// Map deprecated types to newer versions
+			if ( !in_array( $param->type, $this->validParameterTypes ) ) {
-			if ( isset( self::DEPRECATED_TYPES_MAP[ $param->type ] ) ) {
-				$param->type = self::DEPRECATED_TYPES_MAP[ $param->type ];
-			}
-
-			if ( !in_array( $param->type, self::VALID_TYPES ) ) {
 				return Status::newFatal( 'templatedata-invalid-value',
 					'params.' . $paramName . '.type' );
 			}
-		} else {
-			$param->type = 'unknown';
 		}
 
 		// Param.suggestedvalues
@@ -313,8 +259,6 @@ class TemplateDataValidator {
 						"params.{$paramName}.suggestedvalues[$i]", 'string' );
 				}
 			}
-		} else {
-			$param->suggestedvalues = [];
 		}
 
 		return null;
@@ -354,16 +298,13 @@ class TemplateDataValidator {
 	}
 
 	/**
-	 * @param mixed &$sets
+	 * @param mixed $sets
 	 * @param stdClass $params
 	 *
 	 * @return Status|null
 	 */
-	private function validateSets( &$sets, stdClass $params ): ?Status {
+	private function validateSets( $sets, stdClass $params ): ?Status {
-		if ( $sets === null ) {
+		if ( !is_array( $sets ) ) {
-			$sets = [];
-			return null;
-		} elseif ( !is_array( $sets ) ) {
 			return Status::newFatal( 'templatedata-invalid-type', 'sets', 'array' );
 		}
 
@@ -382,8 +323,6 @@ class TemplateDataValidator {
 					'string|object' );
 			}
 
-			$setObj->label = $this->normaliseInterfaceText( $setObj->label );
-
 			if ( !isset( $setObj->params ) ) {
 				return Status::newFatal( 'templatedata-invalid-missing', "sets.{$setNr}.params",
 					'array' );
@@ -411,16 +350,13 @@ class TemplateDataValidator {
 	}
 
 	/**
-	 * @param mixed &$maps
+	 * @param mixed $maps
 	 * @param stdClass $params
 	 *
 	 * @return Status|null
 	 */
-	private function validateMaps( &$maps, stdClass $params ): ?Status {
+	private function validateMaps( $maps, stdClass $params ): ?Status {
-		if ( $maps === null ) {
+		if ( !( $maps instanceof stdClass ) ) {
-			$maps = (object)[];
-			return null;
-		} elseif ( !( $maps instanceof stdClass ) ) {
 			return Status::newFatal( 'templatedata-invalid-type', 'maps', 'object' );
 		}
 
@@ -504,17 +440,4 @@ class TemplateDataValidator {
 		return is_string( $text );
 	}
 
-	/**
-	 * Normalise a InterfaceText field in the TemplateData blob.
-	 * @param stdClass|string $text
-	 * @return stdClass
-	 */
-	private function normaliseInterfaceText( $text ): stdClass {
-		if ( is_string( $text ) ) {
-			$contLang = MediaWikiServices::getInstance()->getContentLanguage();
-			return (object)[ $contLang->getCode() => $text ];
-		}
-		return $text;
-	}
-
 }

tests/phpunit/TemplateDataBlobTest.php

@@ -11,6 +11,7 @@ use Wikimedia\TestingAccessWrapper;
  * @group Database
  * @covers \MediaWiki\Extension\TemplateData\TemplateDataBlob
  * @covers \MediaWiki\Extension\TemplateData\TemplateDataCompressedBlob
+ * @covers \MediaWiki\Extension\TemplateData\TemplateDataNormalizer
  * @covers \MediaWiki\Extension\TemplateData\TemplateDataValidator
  */
 class TemplateDataBlobTest extends MediaWikiIntegrationTestCase {
@@ -819,7 +820,7 @@ class TemplateDataBlobTest extends MediaWikiIntegrationTestCase {
 	public function testIsValidInterfaceText( $text, bool $expected ) {
 		/** @var TemplateDataValidator $validator */
 		$validator = TestingAccessWrapper::newFromObject(
-			new TemplateDataValidator()
+			new TemplateDataValidator( [] )
 		);
 		$this->assertSame( $expected, $validator->isValidInterfaceText( $text ) );
 	}