Hard-deprecate all public property access on CacheTime and ParserOutput.

- Added a test where ParserOutput objects with CacheTime
properties set are unserialized from previous versions.
- Generate new serialization tests for 1.38

Now all serialization in production is JSON, so changing
property visibility shouldn't affect ParserCache.

Bug: T263851
Depends-On: I283340ff559420ceee8f286ba3ef202c01206a23
Change-Id: I70d6feb1c995a0a0f763b21261141ae8ee6dc570

RELEASE-NOTES-1.38

@@ -210,6 +210,8 @@ because of Phabricator reports.
 * Return values in the parameter $pageLang of the hook PageContentLanguage
   with other types than a Language object, deprecated since 1.33, now emmits
   deprecation warnings.
+* All external access to ParserOutput and CacheTime classes properties
+  was hard-deprecated. Use getters and setters instead.
 * …
 
 === Other changes in 1.38 ===

includes/parser/CacheTime.php

@@ -46,18 +46,18 @@ class CacheTime implements ParserCacheMetadata, JsonUnserializable {
 	 * @var string|int TS_MW timestamp when this object was generated, or -1 for not cacheable. Used
 	 * in ParserCache.
 	 */
-	public $mCacheTime = '';
+	protected $mCacheTime = '';
 
 	/**
 	 * @var int|null Seconds after which the object should expire, use 0 for not cacheable. Used in
 	 * ParserCache.
 	 */
-	public $mCacheExpiry = null;
+	protected $mCacheExpiry = null;
 
 	/**
 	 * @var int|null Revision ID that was parsed
 	 */
-	public $mCacheRevisionId = null;
+	protected $mCacheRevisionId = null;
 
 	/**
 	 * @return string|int TS_MW timestamp
@@ -292,4 +292,31 @@ class CacheTime implements ParserCacheMetadata, JsonUnserializable {
 			$this->recordOptions( $priorOptions );
 		}
 	}
+
+	public function __get( $name ) {
+		if ( property_exists( get_called_class(), $name ) ) {
+			// Direct access to a public property, deprecated.
+			wfDeprecatedMsg( "CacheTime::{$name} public read access deprecated", '1.38' );
+			return $this->$name;
+		} elseif ( property_exists( $this, $name ) ) {
+			// Dynamic property access, deprecated.
+			wfDeprecatedMsg( "CacheTime::{$name} dynamic property read access deprecated", '1.38' );
+			return $this->$name;
+		} else {
+			trigger_error( "Inaccessible property via __set(): $name" );
+			return null;
+		}
+	}
+
+	public function __set( $name, $value ) {
+		if ( property_exists( get_called_class(), $name ) ) {
+			// Direct access to a public property, deprecated.
+			wfDeprecatedMsg( "CacheTime::$name public write access deprecated", '1.38' );
+			$this->$name = $value;
+		} else {
+			// Dynamic property access, deprecated.
+			wfDeprecatedMsg( "CacheTime::$name dynamic property write access deprecated", '1.38' );
+			$this->$name = $value;
+		}
+	}
 }

includes/parser/ParserOutput.php

@@ -49,145 +49,145 @@ class ParserOutput extends CacheTime {
 	/**
 	 * @var string|null The output text
 	 */
-	public $mText = null;
+	private $mText = null;
 
 	/**
 	 * @var array List of the full text of language links,
 	 *  in the order they appear.
 	 */
-	public $mLanguageLinks;
+	private $mLanguageLinks;
 
 	/**
 	 * @var array Map of category names to sort keys
 	 */
-	public $mCategories;
+	private $mCategories;
 
 	/**
 	 * @var array Page status indicators, usually displayed in top-right corner.
 	 */
-	public $mIndicators = [];
+	private $mIndicators = [];
 
 	/**
 	 * @var string Title text of the chosen language variant, as HTML.
 	 */
-	public $mTitleText;
+	private $mTitleText;
 
 	/**
 	 * @var int[][] 2-D map of NS/DBK to ID for the links in the document.
 	 *  ID=zero for broken.
 	 * @phan-var array<int,array<string,int>>
 	 */
-	public $mLinks = [];
+	private $mLinks = [];
 
 	/**
 	 * @var array Keys are DBKs for the links to special pages in the document.
 	 * @since 1.35
 	 */
-	public $mLinksSpecial = [];
+	private $mLinksSpecial = [];
 
 	/**
 	 * @var array 2-D map of NS/DBK to ID for the template references.
 	 *  ID=zero for broken.
 	 */
-	public $mTemplates = [];
+	private $mTemplates = [];
 
 	/**
 	 * @var array 2-D map of NS/DBK to rev ID for the template references.
 	 *  ID=zero for broken.
 	 */
-	public $mTemplateIds = [];
+	private $mTemplateIds = [];
 
 	/**
 	 * @var array DB keys of the images used, in the array key only
 	 */
-	public $mImages = [];
+	private $mImages = [];
 
 	/**
 	 * @var array DB keys of the images used mapped to sha1 and MW timestamp.
 	 */
-	public $mFileSearchOptions = [];
+	private $mFileSearchOptions = [];
 
 	/**
 	 * @var array External link URLs, in the key only.
 	 */
-	public $mExternalLinks = [];
+	private $mExternalLinks = [];
 
 	/**
 	 * @var array 2-D map of prefix/DBK (in keys only)
 	 *  for the inline interwiki links in the document.
 	 */
-	public $mInterwikiLinks = [];
+	private $mInterwikiLinks = [];
 
 	/**
 	 * @var bool Show a new section link?
 	 */
-	public $mNewSection = false;
+	private $mNewSection = false;
 
 	/**
 	 * @var bool Hide the new section link?
 	 */
-	public $mHideNewSection = false;
+	private $mHideNewSection = false;
 
 	/**
 	 * @var bool No gallery on category page? (__NOGALLERY__).
 	 */
-	public $mNoGallery = false;
+	private $mNoGallery = false;
 
 	/**
 	 * @var array Items to put in the <head> section
 	 */
-	public $mHeadItems = [];
+	private $mHeadItems = [];
 
 	/**
 	 * @var array Modules to be loaded by ResourceLoader
 	 */
-	public $mModules = [];
+	private $mModules = [];
 
 	/**
 	 * @var array Modules of which only the CSSS will be loaded by ResourceLoader.
 	 */
-	public $mModuleStyles = [];
+	private $mModuleStyles = [];
 
 	/**
 	 * @var array JavaScript config variable for mw.config combined with this page.
 	 */
-	public $mJsConfigVars = [];
+	private $mJsConfigVars = [];
 
 	/**
 	 * @var array Hook tags as per $wgParserOutputHooks.
 	 */
-	public $mOutputHooks = [];
+	private $mOutputHooks = [];
 
 	/**
 	 * @var array Warning text to be returned to the user.
 	 *  Wikitext formatted, in the key only.
 	 */
-	public $mWarnings = [];
+	private $mWarnings = [];
 
 	/**
 	 * @var array Table of contents
 	 */
-	public $mSections = [];
+	private $mSections = [];
 
 	/**
 	 * @var array Name/value pairs to be cached in the DB.
 	 */
-	public $mProperties = [];
+	private $mProperties = [];
 
 	/**
 	 * @var string HTML of the TOC.
 	 */
-	public $mTOCHTML = '';
+	private $mTOCHTML = '';
 
 	/**
 	 * @var string Timestamp of the revision.
 	 */
-	public $mTimestamp;
+	private $mTimestamp;
 
 	/**
 	 * @var bool Whether OOUI should be enabled.
 	 */
-	public $mEnableOOUI = false;
+	private $mEnableOOUI = false;
 
 	/**
 	 * @var string 'index' or 'noindex'?  Any other value will result in no change.
@@ -1950,4 +1950,31 @@ class ParserOutput extends CacheTime {
 			$this->mParseUsedOptions = $priorAccessedOptions;
 		}
 	}
+
+	public function __get( $name ) {
+		if ( property_exists( get_called_class(), $name ) ) {
+			// Direct access to a public property, deprecated.
+			wfDeprecatedMsg( "ParserOutput::{$name} public read access deprecated", '1.38' );
+			return $this->$name;
+		} elseif ( property_exists( $this, $name ) ) {
+			// Dynamic property access, deprecated.
+			wfDeprecatedMsg( "ParserOutput::{$name} dynamic property read access deprecated", '1.38' );
+			return $this->$name;
+		} else {
+			trigger_error( "Inaccessible property via __set(): $name" );
+			return null;
+		}
+	}
+
+	public function __set( $name, $value ) {
+		if ( property_exists( get_called_class(), $name ) ) {
+			// Direct access to a public property, deprecated.
+			wfDeprecatedMsg( "ParserOutput::$name public write access deprecated", '1.38' );
+			$this->$name = $value;
+		} else {
+			// Dynamic property access, deprecated.
+			wfDeprecatedMsg( "ParserOutput::$name dynamic property write access deprecated", '1.38' );
+			$this->$name = $value;
+		}
+	}
 }

tests/phpunit/data/ParserCache/1.36-ParserOutput-cacheTime.json

@@ -0,0 +1 @@
+{"Text":"CacheTime","LanguageLinks":[],"Categories":[],"Indicators":[],"TitleText":"","Links":[],"LinksSpecial":[],"Templates":[],"TemplateIds":[],"Images":[],"FileSearchOptions":[],"ExternalLinks":[],"InterwikiLinks":[],"NewSection":false,"HideNewSection":false,"NoGallery":false,"HeadItems":[],"Modules":[],"ModuleStyles":[],"JsConfigVars":[],"OutputHooks":[],"Warnings":[],"Sections":[],"Properties":[],"TOCHTML":"","Timestamp":null,"EnableOOUI":false,"IndexPolicy":"","ExtensionData":[],"LimitReportData":[],"LimitReportJSData":[],"ParseStartTime":[],"PreventClickjacking":false,"ExtraScriptSrcs":[],"ExtraDefaultSrcs":[],"ExtraStyleSrcs":[],"Flags":[],"SpeculativeRevId":null,"SpeculativePageIdUsed":null,"RevisionTimestampUsed":null,"RevisionUsedSha1Base36":null,"WrapperDivClasses":[],"ParseUsedOptions":[],"CacheExpiry":10,"CacheTime":"20010419042521","CacheRevisionId":42,"Version":"1.6.4","_type_":"ParserOutput"}

tests/phpunit/data/ParserCache/1.36_wmf.18-ParserOutput-cacheTime.json

@@ -0,0 +1 @@
+{"Text":"CacheTime","LanguageLinks":[],"Categories":[],"Indicators":[],"TitleText":"","Links":[],"LinksSpecial":[],"Templates":[],"TemplateIds":[],"Images":[],"FileSearchOptions":[],"ExternalLinks":[],"InterwikiLinks":[],"NewSection":false,"HideNewSection":false,"NoGallery":false,"HeadItems":[],"Modules":[],"ModuleStyles":[],"JsConfigVars":[],"OutputHooks":[],"Warnings":[],"Sections":[],"Properties":[],"TOCHTML":"","Timestamp":null,"EnableOOUI":false,"IndexPolicy":"","AccessedOptions":[],"ExtensionData":[],"LimitReportData":[],"LimitReportJSData":[],"ParseStartTime":[],"PreventClickjacking":false,"ExtraScriptSrcs":[],"ExtraDefaultSrcs":[],"ExtraStyleSrcs":[],"Flags":[],"SpeculativeRevId":null,"SpeculativePageIdUsed":null,"RevisionTimestampUsed":null,"RevisionUsedSha1Base36":null,"WrapperDivClasses":[],"UsedOptions":null,"CacheExpiry":10,"CacheTime":"20010419042521","CacheRevisionId":42,"Version":"1.6.4","_type_":"ParserOutput"}

tests/phpunit/data/ParserCache/1.38-ParserOutput-cacheTime.json

@@ -0,0 +1 @@
+{"Text":"CacheTime","LanguageLinks":[],"Categories":[],"Indicators":[],"TitleText":"","Links":[],"LinksSpecial":[],"Templates":[],"TemplateIds":[],"Images":[],"FileSearchOptions":[],"ExternalLinks":[],"InterwikiLinks":[],"NewSection":false,"HideNewSection":false,"NoGallery":false,"HeadItems":[],"Modules":[],"ModuleStyles":[],"JsConfigVars":[],"OutputHooks":[],"Warnings":[],"Sections":[],"Properties":[],"TOCHTML":"","Timestamp":null,"EnableOOUI":false,"IndexPolicy":"","ExtensionData":[],"LimitReportData":[],"LimitReportJSData":[],"ParseStartTime":[],"PreventClickjacking":false,"ExtraScriptSrcs":[],"ExtraDefaultSrcs":[],"ExtraStyleSrcs":[],"Flags":[],"SpeculativeRevId":null,"SpeculativePageIdUsed":null,"RevisionTimestampUsed":null,"RevisionUsedSha1Base36":null,"WrapperDivClasses":[],"ParseUsedOptions":[],"CacheExpiry":10,"CacheTime":"20010419042521","CacheRevisionId":42,"_type_":"ParserOutput"}

tests/phpunit/includes/libs/serialization/SerializationTestUtils.php

@@ -215,12 +215,14 @@ class SerializationTestUtils {
 			sort( $savedFiles );
 			$path = $savedFiles[count( $savedFiles ) - 1];
 		}
+		$curPath = "$this->serializedDataPath/{$this->getCurrentVersion()}-$class-$testCaseName.$this->ext";
 		return (object)[
 			'version' => $version,
 			'class' => $class,
 			'testCaseName' => $testCaseName,
 			'ext' => $this->ext,
 			'path' => $path,
+			'currentVersionPath' => $curPath,
 			'data' => file_exists( $path ) ? file_get_contents( $path ) : null,
 		];
 	}

tests/phpunit/includes/parser/ParserCacheSerializationTestCases.php

@@ -55,6 +55,8 @@ abstract class ParserCacheSerializationTestCases {
 			. '\xca\x49\x01\x00\x85\x11\x4a\x0d\x0b\x00\x00\x00",
 	];
 
+	private const CACHE_TIME = '20010419042521';
+
 	/**
 	 * Get acceptance test cases for CacheTime class.
 	 * @see SerializationTestTrait::getTestInstancesAndAssertions()
@@ -64,9 +66,8 @@ abstract class ParserCacheSerializationTestCases {
 		$cacheTimeWithUsedOptions = new CacheTime();
 		$cacheTimeWithUsedOptions->recordOptions( [ 'optA', 'optX' ] );
 
-		$cacheTimestamp = MWTimestamp::convert( TS_MW, 987654321 );
 		$cacheTimeWithTime = new CacheTime();
-		$cacheTimeWithTime->setCacheTime( $cacheTimestamp );
+		$cacheTimeWithTime->setCacheTime( self::CACHE_TIME );
 
 		$cacheExpiry = 10;
 		$cacheTimeWithExpiry = new CacheTime();
@@ -106,8 +107,8 @@ abstract class ParserCacheSerializationTestCases {
 				'instance' => $cacheTimeWithTime,
 				'assertions' => static function (
 					MediaWikiIntegrationTestCase $testCase, CacheTime $object
-				) use ( $cacheTimestamp ) {
+				) {
-					$testCase->assertSame( $cacheTimestamp, $object->getCacheTime() );
+					$testCase->assertSame( self::CACHE_TIME, $object->getCacheTime() );
 				}
 			],
 			'cacheExpiry' => [
@@ -135,6 +136,11 @@ abstract class ParserCacheSerializationTestCases {
 	 * @return array[]
 	 */
 	public static function getParserOutputTestCases() {
+		$parserOutputWithCacheTimeProps = new ParserOutput( 'CacheTime' );
+		$parserOutputWithCacheTimeProps->setCacheTime( self::CACHE_TIME );
+		$parserOutputWithCacheTimeProps->updateCacheExpiry( 10 );
+		$parserOutputWithCacheTimeProps->setCacheRevisionId( 42 );
+
 		$parserOutputWithUsedOptions = new ParserOutput( 'Dummy' );
 		$parserOutputWithUsedOptions->recordOption( 'optA' );
 		$parserOutputWithUsedOptions->recordOption( 'optX' );
@@ -265,6 +271,14 @@ abstract class ParserCacheSerializationTestCases {
 					$testCase->assertNull( $object->getTimeSinceStart( 'wall' ) );
 				}
 			],
+			'cacheTime' => [
+				'instance' => $parserOutputWithCacheTimeProps,
+				'assertions' => static function ( MediaWikiIntegrationTestCase $testCase, ParserOutput $object ) {
+					$testCase->assertSame( self::CACHE_TIME, $object->getCacheTime() );
+					$testCase->assertSame( 10, $object->getCacheExpiry() );
+					$testCase->assertSame( 42, $object->getCacheRevisionId() );
+				}
+			],
 			'text' => [
 				'instance' => new ParserOutput( 'Lorem Ipsum' ),
 				'assertions' => static function ( MediaWikiIntegrationTestCase $testCase, ParserOutput $object ) {

tests/phpunit/includes/parser/validateParserCacheSerializationTestData.php

@@ -84,8 +84,8 @@ class ValidateParserCacheSerializationTestData extends Maintenance {
 		} else {
 			if ( $data !== $fileInfo->data ) {
 				if ( $this->hasOption( 'update' ) ) {
-					$this->output( 'Data mismatch, updating file: ' . $fileInfo->path . "\n" );
+					$this->output( 'Data mismatch, updating file: ' . $fileInfo->currentVersionPath . "\n" );
-					file_put_contents( $fileInfo->path, $data );
+					file_put_contents( $fileInfo->currentVersionPath, $data );
 				} else {
 					$this->fatalError( "Serialization data mismatch: {$fileInfo->path}. "
 						. "If this was expected, rerun the script with the --update option "