From 584fdcddf695df80792766583d2bec3fa5348cce Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Thu, 9 Mar 2023 11:43:14 +0100 Subject: [PATCH] Extract serialization methods into TemplateDataStatus class This makes the large Hook class quite a bit smaller. Change-Id: I55229116eb16ccd9be21d1f34de5e52826ece2bf --- includes/Hooks.php | 80 ++++------------------------- includes/TemplateDataStatus.php | 50 ++++++++++++++++++ tests/phpunit/SerializationTest.php | 20 ++++---- 3 files changed, 70 insertions(+), 80 deletions(-) create mode 100644 includes/TemplateDataStatus.php diff --git a/includes/Hooks.php b/includes/Hooks.php index 655b4671..17511b6e 100644 --- a/includes/Hooks.php +++ b/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 ); - if ( $templateDataStatus instanceof Status && !$templateDataStatus->isOK() ) { + $status = TemplateDataStatus::newFromJson( $parserOutput->getExtensionData( 'TemplateDataStatus' ) ); + 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' ] ); - $parser->getOutput()->setEnableOOUI( true ); + $parserOutput->addModules( [ 'jquery.tablesorter' ] ); + $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() - ]; - } - } } diff --git a/includes/TemplateDataStatus.php b/includes/TemplateDataStatus.php new file mode 100644 index 00000000..bc9c9587 --- /dev/null +++ b/includes/TemplateDataStatus.php @@ -0,0 +1,50 @@ +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; + } + +} diff --git a/tests/phpunit/SerializationTest.php b/tests/phpunit/SerializationTest.php index 2dcbc55b..f0e349ae 100644 --- a/tests/phpunit/SerializationTest.php +++ b/tests/phpunit/SerializationTest.php @@ -1,12 +1,13 @@ 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 ); - $result = TemplateDataHooks::getStatusFromParserOutput( $parserOutput ); + $parserOutput->setExtensionData( 'TemplateDataStatus', TemplateDataStatus::jsonSerialize( $status ) ); + $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 ); - $result = TemplateDataHooks::newStatusFromJson( $json ); + $json = TemplateDataStatus::jsonSerialize( $status ); + $result = TemplateDataStatus::newFromJson( $json ); $this->assertEquals( $status->getStatusValue(), $result->getStatusValue() ); $this->assertSame( (string)$status, (string)$result ); } + }