Avoid caching serialized PHP object

Cache array representation of the Gadget object instead of the
php-serialized representation. Gadget::toArray() is an inverse of the
constructor which already constructs an object from an array of config
values.

Also, the static Gadget::newFromDefinitionContent method which accesses
the service container is replaced with the dependency-less
serializeDefinition() method.

Bug: T303194
Change-Id: Ieae6706537143d766777b2299c31726e2a1dfd29

includes/Gadget.php

@@ -14,7 +14,6 @@
 namespace MediaWiki\Extension\Gadgets;
 
 use InvalidArgumentException;
-use MediaWiki\Extension\Gadgets\Content\GadgetDefinitionContent;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\Authority;
 use MediaWiki\User\UserIdentity;
@@ -105,18 +104,18 @@ class Gadget {
 	}
 
 	/**
-	 * Create a object based on the metadata in a GadgetDefinitionContent object
+	 * Create a serialized array based on the metadata in a GadgetDefinitionContent object,
+	 * from which a Gadget object can be constructed.
 	 *
 	 * @param string $id
-	 * @param GadgetDefinitionContent $content
-	 * @return Gadget
+	 * @param array $data
+	 * @return array
 	 */
-	public static function newFromDefinitionContent( $id, GadgetDefinitionContent $content ) {
-		$data = $content->getAssocArray();
+	public static function serializeDefinition( string $id, array $data ): array {
 		$prefixGadgetNs = static function ( $page ) {
 			return 'Gadget:' . $page;
 		};
-		$info = [
+		return [
 			'name' => $id,
 			'resourceLoaded' => true,
 			'requiresES6' => $data['settings']['requiresES6'],
@@ -137,8 +136,35 @@ class Gadget {
 			'messages' => $data['module']['messages'],
 			'type' => $data['module']['type'],
 		];
+	}
 
-		return new self( $info );
+	/**
+	 * Serialize to an array
+	 * @return array
+	 */
+	public function toArray(): array {
+		return [
+			'name' => $this->name,
+			'definition' => $this->definition,
+			'resourceLoaded' => $this->resourceLoaded,
+			'requiresES6' => $this->requiresES6,
+			'requiredRights' => $this->requiredRights,
+			'onByDefault' => $this->onByDefault,
+			'package' => $this->package,
+			'hidden' => $this->hidden,
+			'requiredActions' => $this->requiredActions,
+			'requiredSkins' => $this->requiredSkins,
+			'category' => $this->category,
+			'supportsUrlLoad' => $this->supportsUrlLoad,
+			'scripts' => $this->scripts,
+			'styles' => $this->styles,
+			'datas' => $this->datas,
+			'dependencies' => $this->dependencies,
+			'peers' => $this->peers,
+			'messages' => $this->messages,
+			'type' => $this->type,
+			'targets' => $this->targets,
+		];
 	}
 
 	/**

includes/GadgetDefinitionNamespaceRepo.php

@@ -131,12 +131,13 @@ class GadgetDefinitionNamespaceRepo extends GadgetRepo {
 					return null;
 				}
 
-				return Gadget::newFromDefinitionContent( $id, $content );
+				return Gadget::serializeDefinition( $id, $content->getAssocArray() );
 			},
 			[
 				'checkKeys' => [ $key ],
 				'pcTTL' => WANObjectCache::TTL_PROC_SHORT,
-				'lockTSE' => 30
+				'lockTSE' => 30,
+				'version' => 2,
 			]
 		);
 
@@ -144,7 +145,7 @@ class GadgetDefinitionNamespaceRepo extends GadgetRepo {
 			throw new InvalidArgumentException( "No gadget registered for '$id'" );
 		}
 
-		return $gadget;
+		return new Gadget( $gadget );
 	}
 
 	/**

includes/MediaWikiGadgetsDefinitionRepo.php

@@ -35,7 +35,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 			throw new InvalidArgumentException( "No gadget registered for '$id'" );
 		}
 
-		return $gadgets[$id];
+		return new Gadget( $gadgets[$id] );
 	}
 
 	public function getGadgetIds(): array {
@@ -81,7 +81,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 	/**
 	 * Get list of gadgets.
 	 *
-	 * @return Gadget[] List of Gadget objects
+	 * @return array[] List of Gadget objects
 	 */
 	protected function loadGadgets(): array {
 		// From back to front:
@@ -114,6 +114,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 							return $this->fetchStructuredList();
 						},
 						[
+							'version' => 2,
 							// Avoid database stampede
 							'lockTSE' => 300,
 						 ]
@@ -128,7 +129,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 	 * Fetch list of gadgets and returns it as associative array of sections with gadgets
 	 * e.g. [ $name => $gadget1, etc. ]
 	 * @param string|null $forceNewText Injected text of MediaWiki:gadgets-definition [optional]
-	 * @return array
+	 * @return array[]
 	 */
 	public function fetchStructuredList( $forceNewText = null ) {
 		if ( $forceNewText === null ) {
@@ -162,9 +163,9 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 	 * Generates a structured list of Gadget objects from a definition
 	 *
 	 * @param string $definition
-	 * @return Gadget[] List of Gadget objects indexed by the gadget's name.
+	 * @return array[] List of Gadget objects indexed by the gadget's name.
 	 */
-	private function listFromDefinition( $definition ) {
+	private function listFromDefinition( $definition ): array {
 		$definition = preg_replace( '/<!--.*?-->/s', '', $definition );
 		$lines = preg_split( '/(\r\n|\r|\n)+/', $definition );
 
@@ -178,7 +179,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo {
 			} else {
 				$gadget = $this->newFromDefinition( $line, $section );
 				if ( $gadget ) {
-					$gadgets[$gadget->getName()] = $gadget;
+					$gadgets[$gadget->getName()] = $gadget->toArray();
 				}
 			}
 		}

tests/phpunit/unit/GadgetTest.php

@@ -7,6 +7,60 @@ use MediaWiki\Extension\Gadgets\MediaWikiGadgetsDefinitionRepo;
  * @group Gadgets
  */
 class GadgetTest extends MediaWikiUnitTestCase {
+
+	/**
+	 * @covers \MediaWiki\Extension\Gadgets\MediaWikiGadgetsDefinitionRepo::newFromDefinition
+	 * @covers \MediaWiki\Extension\Gadgets\Gadget::toArray
+	 */
+	public function testToArray() {
+		$g = GadgetTestUtils::makeGadget( '*bar[ResourceLoader|rights=test]|bar.js|foo.css | foo.json' );
+		$gNewFromSerialized = new Gadget( $g->toArray() );
+		$this->assertArrayEquals( $g->toArray(), $gNewFromSerialized->toArray() );
+	}
+
+	/**
+	 * @covers \MediaWiki\Extension\Gadgets\Gadget::serializeDefinition
+	 */
+	public function testGadgetFromDefinitionContent() {
+		$gArray = Gadget::serializeDefinition( 'bar', [
+			'settings' => [
+				'rights' => [],
+				'default' => true,
+				'package' => true,
+				'hidden' => false,
+				'actions' => [],
+				'targets' => [ 'desktop' ],
+				'skins' => [],
+				'category' => 'misc',
+				'supportsUrlLoad' => false,
+				'requiresES6' => false,
+			],
+			'module' => [
+				'scripts' => [ 'foo.js' ],
+				'styles' => [ 'bar.css' ],
+				'datas' => [],
+				'dependencies' => [ 'moment' ],
+				'peers' => [],
+				'messages' => [ 'blanknamespace' ],
+				'type' => 'general',
+			]
+		] );
+
+		$g = new Gadget( $gArray );
+
+		$this->assertTrue( $g->isOnByDefault() );
+		$this->assertTrue( $g->isPackaged() );
+		$this->assertFalse( $g->isHidden() );
+		$this->assertFalse( $g->supportsUrlLoad() );
+		$this->assertTrue( $g->supportsResourceLoader() );
+		$this->assertCount( 1, $g->getTargets() );
+		$this->assertCount( 1, $g->getScripts() );
+		$this->assertCount( 1, $g->getStyles() );
+		$this->assertCount( 0, $g->getJSONs() );
+		$this->assertCount( 1, $g->getDependencies() );
+		$this->assertCount( 1, $g->getMessages() );
+	}
+
 	/**
 	 * @covers \MediaWiki\Extension\Gadgets\MediaWikiGadgetsDefinitionRepo::newFromDefinition
 	 */