Extract serialization methods into TemplateDataStatus class

This makes the large Hook class quite a bit smaller.

Change-Id: I55229116eb16ccd9be21d1f34de5e52826ece2bf

includes/Hooks.php

@@ -14,7 +14,6 @@ use MediaWiki\Revision\SlotRecord;
 use MediaWiki\User\UserIdentity;
 use OutputPage;
 use Parser;
-use ParserOutput;
 use PPFrame;
 use RequestContext;
 use ResourceLoader;
@@ -97,10 +96,10 @@ class Hooks {
 		// Revision hasn't been parsed yet, so parse to know if self::render got a
 		// valid tag (via inclusion and transclusion) and abort save if it didn't
 		$parserOutput = $renderedRevision->getRevisionParserOutput( [ 'generate-html' => false ] );
-		$templateDataStatus = self::getStatusFromParserOutput( $parserOutput );
+		$status = TemplateDataStatus::newFromJson( $parserOutput->getExtensionData( 'TemplateDataStatus' ) );
-		if ( $templateDataStatus instanceof Status && !$templateDataStatus->isOK() ) {
+		if ( $status && !$status->isOK() ) {
 			// Abort edit, show error message from TemplateDataBlob::getStatus
-			$hookStatus->merge( $templateDataStatus );
+			$hookStatus->merge( $status );
 			return false;
 		}
 
@@ -188,11 +187,12 @@ class Hooks {
 	 * @return string HTML to insert in the page.
 	 */
 	public static function render( $input, $args, Parser $parser, $frame ) {
+		$parserOutput = $parser->getOutput();
 		$ti = TemplateDataBlob::newFromJSON( wfGetDB( DB_REPLICA ), $input ?? '' );
 
 		$status = $ti->getStatus();
 		if ( !$status->isOK() ) {
-			self::setStatusToParserOutput( $parser->getOutput(), $status );
+			$parserOutput->setExtensionData( 'TemplateDataStatus', TemplateDataStatus::jsonSerialize( $status ) );
 			return Html::errorBox( $status->getHTML() );
 		}
 
@@ -205,16 +205,16 @@ class Hooks {
 		$title = $parser->getTitle();
 		$docPage = wfMessage( 'templatedata-doc-subpage' )->inContentLanguage();
 		if ( !$title->isSubpage() || $title->getSubpageText() !== $docPage->plain() ) {
-			$parser->getOutput()->setPageProperty( 'templatedata', $ti->getJSONForDatabase() );
+			$parserOutput->setPageProperty( 'templatedata', $ti->getJSONForDatabase() );
 		}
 
-		$parser->getOutput()->addModuleStyles( [
+		$parserOutput->addModuleStyles( [
 			'ext.templateData',
 			'ext.templateData.images',
 			'jquery.tablesorter.styles',
 		] );
-		$parser->getOutput()->addModules( [ 'jquery.tablesorter' ] );
+		$parserOutput->addModules( [ 'jquery.tablesorter' ] );
-		$parser->getOutput()->setEnableOOUI( true );
+		$parserOutput->setEnableOOUI( true );
 
 		$userLang = $parser->getOptions()->getUserLangObj();
 
@@ -319,66 +319,4 @@ class Hooks {
 		}
 	}
 
-	/**
-	 * Write the status to ParserOutput object.
-	 * @param ParserOutput $parserOutput
-	 * @param Status $status
-	 */
-	public static function setStatusToParserOutput( ParserOutput $parserOutput, Status $status ) {
-		$parserOutput->setExtensionData( 'TemplateDataStatus',
-			self::jsonSerializeStatus( $status ) );
-	}
-
-	/**
-	 * @param ParserOutput $parserOutput
-	 * @return Status|null
-	 */
-	public static function getStatusFromParserOutput( ParserOutput $parserOutput ) {
-		$status = $parserOutput->getExtensionData( 'TemplateDataStatus' );
-		if ( is_array( $status ) ) {
-			return self::newStatusFromJson( $status );
-		}
-		return $status;
-	}
-
-	/**
-	 * @param array $status contains StatusValue ok and errors fields (does not serialize value)
-	 * @return Status
-	 */
-	public static function newStatusFromJson( array $status ): Status {
-		if ( $status['ok'] ) {
-			return Status::newGood();
-		} else {
-			$statusObj = new Status();
-			$errors = $status['errors'];
-			foreach ( $errors as $error ) {
-				$statusObj->fatal( $error['message'], ...$error['params'] );
-			}
-			$warnings = $status['warnings'];
-			foreach ( $warnings as $warning ) {
-				$statusObj->warning( $warning['message'], ...$warning['params'] );
-			}
-			return $statusObj;
-		}
-	}
-
-	/**
-	 * @param Status $status
-	 * @return array contains StatusValue ok and errors fields (does not serialize value)
-	 */
-	public static function jsonSerializeStatus( Status $status ): array {
-		if ( $status->isOK() ) {
-			return [
-				'ok' => true
-			];
-		} else {
-			list( $errorsOnlyStatus, $warningsOnlyStatus ) = $status->splitByErrorType();
-			// note that non-scalar values are not supported in errors or warnings
-			return [
-				'ok' => false,
-				'errors' => $errorsOnlyStatus->getErrors(),
-				'warnings' => $warningsOnlyStatus->getErrors()
-			];
-		}
-	}
 }

includes/TemplateDataStatus.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace MediaWiki\Extension\TemplateData;
+
+use Status;
+
+class TemplateDataStatus {
+
+	/**
+	 * @param Status $status
+	 * @return array contains StatusValue ok and errors fields (does not serialize value)
+	 */
+	public static function jsonSerialize( Status $status ): array {
+		if ( $status->isOK() ) {
+			return [ 'ok' => true ];
+		}
+
+		[ $errorsOnlyStatus, $warningsOnlyStatus ] = $status->splitByErrorType();
+		// note that non-scalar values are not supported in errors or warnings
+		return [
+			'ok' => false,
+			'errors' => $errorsOnlyStatus->getErrors(),
+			'warnings' => $warningsOnlyStatus->getErrors()
+		];
+	}
+
+	/**
+	 * @param Status|array|null $json contains StatusValue ok and errors fields (does not serialize value)
+	 * @return Status|null
+	 */
+	public static function newFromJson( $json ): ?Status {
+		if ( !is_array( $json ) ) {
+			return $json;
+		}
+
+		if ( $json['ok'] ) {
+			return Status::newGood();
+		}
+
+		$status = new Status();
+		foreach ( $json['errors'] as $error ) {
+			$status->fatal( $error['message'], ...$error['params'] );
+		}
+		foreach ( $json['warnings'] as $warning ) {
+			$status->warning( $warning['message'], ...$warning['params'] );
+		}
+		return $status;
+	}
+
+}

tests/phpunit/SerializationTest.php

@@ -1,12 +1,13 @@
 <?php
 
-use MediaWiki\Extension\TemplateData\Hooks as TemplateDataHooks;
+use MediaWiki\Extension\TemplateData\TemplateDataStatus;
 
 /**
  * @group TemplateData
- * @covers \MediaWiki\Extension\TemplateData\Hooks
+ * @covers \MediaWiki\Extension\TemplateData\TemplateDataStatus
  */
 class SerializationTest extends MediaWikiIntegrationTestCase {
+
 	public function testParserOutputPersistenceForwardCompatibility() {
 		$output = new ParserOutput();
 
@@ -17,10 +18,10 @@ class SerializationTest extends MediaWikiIntegrationTestCase {
 		// Set JSONified state. Should work before we set JSON-serializable data,
 		// to be robust against old code reading new data after a rollback.
 		$output->setExtensionData( 'TemplateDataStatus',
-			TemplateDataHooks::jsonSerializeStatus( $status )
+			TemplateDataStatus::jsonSerialize( $status )
 		);
 
-		$result = TemplateDataHooks::getStatusFromParserOutput( $output );
+		$result = TemplateDataStatus::newFromJson( $output->getExtensionData( 'TemplateDataStatus' ) );
 		$this->assertEquals( $status->getStatusValue(), $result->getStatusValue() );
 		$this->assertSame( (string)$status, (string)$result );
 	}
@@ -35,7 +36,7 @@ class SerializationTest extends MediaWikiIntegrationTestCase {
 		// Set the object directly. Should still work once we normally set JSON-serializable data.
 		$output->setExtensionData( 'TemplateDataStatus', $status );
 
-		$result = TemplateDataHooks::getStatusFromParserOutput( $output );
+		$result = TemplateDataStatus::newFromJson( $output->getExtensionData( 'TemplateDataStatus' ) );
 		$this->assertEquals( $status->getStatusValue(), $result->getStatusValue() );
 		$this->assertSame( (string)$status, (string)$result );
 	}
@@ -53,8 +54,8 @@ class SerializationTest extends MediaWikiIntegrationTestCase {
 	 */
 	public function testParserOutputPersistenceRoundTrip( Status $status ) {
 		$parserOutput = new ParserOutput();
-		TemplateDataHooks::setStatusToParserOutput( $parserOutput, $status );
+		$parserOutput->setExtensionData( 'TemplateDataStatus', TemplateDataStatus::jsonSerialize( $status ) );
-		$result = TemplateDataHooks::getStatusFromParserOutput( $parserOutput );
+		$result = TemplateDataStatus::newFromJson( $parserOutput->getExtensionData( 'TemplateDataStatus' ) );
 		$this->assertEquals( $status->getStatusValue(), $result->getStatusValue() );
 		$this->assertSame( (string)$status, (string)$result );
 	}
@@ -63,9 +64,10 @@ class SerializationTest extends MediaWikiIntegrationTestCase {
 	 * @dataProvider provideStatus
 	 */
 	public function testJsonRoundTrip( Status $status ) {
-		$json = TemplateDataHooks::jsonSerializeStatus( $status );
+		$json = TemplateDataStatus::jsonSerialize( $status );
-		$result = TemplateDataHooks::newStatusFromJson( $json );
+		$result = TemplateDataStatus::newFromJson( $json );
 		$this->assertEquals( $status->getStatusValue(), $result->getStatusValue() );
 		$this->assertSame( (string)$status, (string)$result );
 	}
+
 }