From a629d7f71d111153f7895b10285a6f1278b9e222 Mon Sep 17 00:00:00 2001 From: Siddharth VP Date: Sat, 9 Dec 2023 03:03:19 +0530 Subject: [PATCH] Introduce MultiGadgetRepo to facilitate repo migration MultiGadgetRepo is a wrapper around two or more GadgetRepos so that they can be used at the same, facilitating migration from one repo to another. If a gadget appears in both repos, the definition in the first repo takes precedence, and a warning is shown on Special:Gadgets. This can be enabled to wrap GadgetDefinitionNamespaceRepo and MediaWikiGadgetsDefinitionRepo by setting $wgGadgetsRepo to 'json+definition'. In this configuration, once a new JSON definition exists for the same name, it is used instead of the old one, and the old one can then safely be removed at a later time in the safe knowledge that it is no longer used. Adapted from If3cc5e22e9812d0fd1a9e8e269ea74a7f667dadd Bug: T140323 Co-authored-by: Kunal Mehta Change-Id: Ibad53629e63ec8713d7a3b84a19838b94600588e --- i18n/en.json | 1 + i18n/qqq.json | 1 + includes/GadgetRepo.php | 3 +- includes/MultiGadgetRepo.php | 131 ++++++++++++++++++ includes/ServiceWiring.php | 6 + includes/Special/SpecialGadgets.php | 2 +- .../integration/MultiGadgetRepoTest.php | 31 +++++ 7 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 includes/MultiGadgetRepo.php create mode 100644 tests/phpunit/integration/MultiGadgetRepoTest.php diff --git a/i18n/en.json b/i18n/en.json index 51036467..bdd03fee 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -58,6 +58,7 @@ "gadgets-validate-invalidrights": "The following {{PLURAL:$2|right does|rights do}} not exist: $1", "gadgets-validate-invalidtitle": "Page title \"$1\" is invalid", "gadgets-validate-unknownpages": "Contains one or more pages without .js, .css or .json suffix. They would not be used.", + "gadgets-validate-duplicate": "A second definition of gadget $1 was detected and shall be ignored", "gadgets-validate-nopage": "Page \"$1\" does not exist.", "gadgets-supports-urlload": "This gadget supports loading via URL with ?withgadget query parameter." } diff --git a/i18n/qqq.json b/i18n/qqq.json index 4ed142a8..fa379580 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -73,6 +73,7 @@ "gadgets-validate-invalidrights": "Warning message to indicate that the rights are not recognised. Parameters:\n* $1 - comma separated list of invalid rights, $2 - number of items in list $1", "gadgets-validate-invalidtitle": "Warning message to indicate that the page title is invalid. Parameters:\n* $1 - page name", "gadgets-validate-unknownpages": "Warning message to indicate that a gadget contains pages without .js, .css or .json suffix, which are not recognised.", + "gadgets-validate-duplicate": "Warning message to indicate that the second gadget definition with the same name would be ignored. Parameters:\n* $1 -gadget name", "gadgets-validate-nopage": "Warning message to indicate the script/style/json page does not exist. Parameters:\n* $1 - page name", "gadgets-supports-urlload": "Used in [[Special:Gadgets]], if the gadget supports ?withgadget query parameter." } diff --git a/includes/GadgetRepo.php b/includes/GadgetRepo.php index 9aba82bf..367d56de 100644 --- a/includes/GadgetRepo.php +++ b/includes/GadgetRepo.php @@ -86,9 +86,10 @@ abstract class GadgetRepo { * to `MediaWiki:Gadget-example.json`. * * @param string $titleText + * @param string $gadgetId * @return string */ - public function titleWithoutPrefix( string $titleText ): string { + public function titleWithoutPrefix( string $titleText, string $gadgetId ): string { $numReplaces = 1; // there will only one occurrence of the prefix return str_replace( self::RESOURCE_TITLE_PREFIX, '', $titleText, $numReplaces ); } diff --git a/includes/MultiGadgetRepo.php b/includes/MultiGadgetRepo.php new file mode 100644 index 00000000..3a9088ed --- /dev/null +++ b/includes/MultiGadgetRepo.php @@ -0,0 +1,131 @@ + + * @copyright 2023 Siddharth VP + */ +class MultiGadgetRepo extends GadgetRepo { + + /** + * @var GadgetRepo[] + */ + private array $repos; + + /** + * @param GadgetRepo[] $repos + */ + public function __construct( array $repos ) { + $this->repos = $repos; + } + + /** + * @inheritDoc + */ + public function getGadget( string $id ): Gadget { + foreach ( $this->repos as $repo ) { + try { + return $repo->getGadget( $id ); + } catch ( InvalidArgumentException $e ) { + // Try next repo + } + } + + throw new InvalidArgumentException( "No gadget registered for '$id'" ); + } + + /** + * @inheritDoc + */ + public function getGadgetIds(): array { + $ids = []; + foreach ( $this->repos as $repo ) { + $ids = array_merge( $ids, $repo->getGadgetIds() ); + } + return array_values( array_unique( $ids ) ); + } + + /** + * @inheritDoc + */ + public function handlePageUpdate( LinkTarget $target ): void { + foreach ( $this->repos as $repo ) { + $repo->handlePageUpdate( $target ); + } + } + + private function getRepoForGadget( string $id ): GadgetRepo { + foreach ( $this->repos as $repo ) { + try { + $repo->getGadget( $id ); + // return repo if it didn't throw + return $repo; + } catch ( InvalidArgumentException $e ) { + } + } + throw new InvalidArgumentException( "No repo found for gadget $id" ); + } + + public function getGadgetDefinitionTitle( string $id ): ?Title { + return $this->getRepoForGadget( $id )->getGadgetDefinitionTitle( $id ); + } + + public function titleWithoutPrefix( string $titleText, string $gadgetId ): string { + return $this->getRepoForGadget( $gadgetId )->titleWithoutPrefix( $titleText, $gadgetId ); + } + + public function validationWarnings( Gadget $gadget ): array { + $duplicateWarnings = $this->isDefinedTwice( $gadget->getName() ) ? [ + wfMessage( "gadgets-validate-duplicate", $gadget->getName() ) + ] : []; + return array_merge( $duplicateWarnings, parent::validationWarnings( $gadget ) ); + } + + /** + * Checks if a gadget is defined with the same name in two different repos. + * @param string $id Gadget name + * @return bool + */ + private function isDefinedTwice( string $id ) { + $found = false; + foreach ( $this->repos as $repo ) { + try { + $repo->getGadget( $id ); + if ( $found ) { + // found it a second time + return true; + } else { + $found = true; + } + } catch ( InvalidArgumentException $e ) { + } + } + return false; + } + +} diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index b76cbb54..db4b16b7 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -3,6 +3,7 @@ use MediaWiki\Extension\Gadgets\GadgetRepo; use MediaWiki\Extension\Gadgets\MediaWikiGadgetsDefinitionRepo; use MediaWiki\Extension\Gadgets\MediaWikiGadgetsJsonRepo; +use MediaWiki\Extension\Gadgets\MultiGadgetRepo; use MediaWiki\MediaWikiServices; return [ @@ -14,6 +15,11 @@ return [ return new MediaWikiGadgetsDefinitionRepo( $wanCache, $revisionLookup ); case 'json': return new MediaWikiGadgetsJsonRepo( $wanCache, $revisionLookup ); + case 'json+definition': + return new MultiGadgetRepo( [ + new MediaWikiGadgetsJsonRepo( $wanCache, $revisionLookup ), + new MediaWikiGadgetsDefinitionRepo( $wanCache, $revisionLookup ) + ] ); default: throw new InvalidArgumentException( 'Unexpected value for $wgGadgetsRepo' ); } diff --git a/includes/Special/SpecialGadgets.php b/includes/Special/SpecialGadgets.php index dbeb5770..27d3093b 100644 --- a/includes/Special/SpecialGadgets.php +++ b/includes/Special/SpecialGadgets.php @@ -223,7 +223,7 @@ class SpecialGadgets extends SpecialPage { $output->addHTML( '
' ); } $output->addHTML( $this->msg( 'gadgets-packaged', - $this->gadgetRepo->titleWithoutPrefix( $gadget->getScripts()[0] ) ) ); + $this->gadgetRepo->titleWithoutPrefix( $gadget->getScripts()[0], $gadget->getName() ) ) ); $needLineBreakAfter = true; } diff --git a/tests/phpunit/integration/MultiGadgetRepoTest.php b/tests/phpunit/integration/MultiGadgetRepoTest.php new file mode 100644 index 00000000..edcdb757 --- /dev/null +++ b/tests/phpunit/integration/MultiGadgetRepoTest.php @@ -0,0 +1,31 @@ + new Gadget( [ 'name' => 'g1', 'onByDefault' => true ] ), + 'g2' => new Gadget( [ 'name' => 'g2', 'onByDefault' => true ] ), + ] ), + new StaticGadgetRepo( [ + 'g1' => new Gadget( [ 'name' => 'g1', 'onByDefault' => false ] ), + 'g3' => new Gadget( [ 'name' => 'g3', 'onByDefault' => false ] ), + ] ) + ] ); + + $this->assertTrue( $repo->getGadget( 'g1' )->isOnByDefault() ); + $this->assertTrue( $repo->getGadget( 'g2' )->isOnByDefault() ); + $this->assertFalse( $repo->getGadget( 'g3' )->isOnByDefault() ); + + $this->assertCount( 1, $repo->validationWarnings( $repo->getGadget( 'g1' ) ) ); + } +}