Lazy instantiation of Cite

Only create a Cite object if we need one.  Never clearState, just
destroy and recreate later.

This makes it less likely that we leak state between parsers, and
saves memory and processing on pages without references.

It's also preparation to decouple Cite logic from state.

Change-Id: I3db517591f4131c23151c76c223af7419cc00ae9
This commit is contained in:
Adam Wight 2019-12-11 11:01:05 +01:00
parent ef98c9826d
commit a227395e3a
7 changed files with 31 additions and 85 deletions

View file

@ -28,8 +28,8 @@
"APIQuerySiteInfoGeneralInfo": "Cite\\Hooks\\CiteHooks::onAPIQuerySiteInfoGeneralInfo",
"ContentHandlerDefaultModelFor": "Cite\\Hooks\\CiteHooks::onContentHandlerDefaultModelFor",
"ParserAfterParse": "Cite\\Hooks\\CiteParserHooks::onParserAfterParse",
"ParserClearState": "Cite\\Hooks\\CiteParserHooks::onParserClearState",
"ParserCloned": "Cite\\Hooks\\CiteParserHooks::onParserCloned",
"ParserClearState": "Cite\\Hooks\\CiteParserHooks::onParserClearStateOrCloned",
"ParserCloned": "Cite\\Hooks\\CiteParserHooks::onParserClearStateOrCloned",
"ParserFirstCallInit": "Cite\\Hooks\\CiteParserHooks::onParserFirstCallInit",
"ResourceLoaderGetConfigVars": "Cite\\Hooks\\CiteHooks::onResourceLoaderGetConfigVars",
"ResourceLoaderRegisterModules": "Cite\\Hooks\\CiteHooks::onResourceLoaderRegisterModules",

View file

@ -508,25 +508,6 @@ class Cite {
return $ret;
}
/**
* Gets run when Parser::clearState() gets run, since we don't
* want the counts to transcend pages and other instances
*
* @param string $force Set to "force" to interrupt parsing
*/
public function clearState( string $force = '' ) {
if ( $force === 'force' ) {
$this->mInCite = false;
$this->inReferencesGroup = null;
} elseif ( $this->mInCite || $this->inReferencesGroup !== null ) {
// Don't clear when we're in the middle of parsing a <ref> or <references> tag
return;
}
if ( $this->referenceStack ) {
$this->referenceStack->clear();
}
}
/**
* Called at the end of page processing to append a default references
* section, if refs were used without a main references tag. If there are references

View file

@ -17,34 +17,19 @@ class CiteParserHooks {
* @param Parser $parser
*/
public static function onParserFirstCallInit( Parser $parser ) {
$parser->extCite = new Cite();
CiteParserTagHooks::register( $parser );
}
/**
* @see https://www.mediawiki.org/wiki/Manual:Hooks/ParserClearState
*
* @param Parser $parser
*/
public static function onParserClearState( Parser $parser ) {
/** @var Cite $cite */
$cite = $parser->extCite;
$cite->clearState();
}
/**
* @see https://www.mediawiki.org/wiki/Manual:Hooks/ParserCloned
*
* @param Parser $parser
*/
public static function onParserCloned( Parser $parser ) {
$parser->extCite = clone $parser->extCite;
/** @var Cite $cite */
$cite = $parser->extCite;
$cite->clearState( 'force' );
CiteParserTagHooks::register( $parser );
public static function onParserClearStateOrCloned( Parser $parser ) {
if ( isset( $parser->extCite ) ) {
unset( $parser->extCite );
}
}
/**
@ -55,9 +40,11 @@ class CiteParserHooks {
* @param StripState $stripState
*/
public static function onParserAfterParse( Parser $parser, &$text, $stripState ) {
/** @var Cite $cite */
$cite = $parser->extCite;
$text .= $cite->checkRefsNoReferences( $parser->getOptions()->getIsSectionPreview() );
if ( isset( $parser->extCite ) ) {
/** @var Cite $cite */
$cite = $parser->extCite;
$text .= $cite->checkRefsNoReferences( $parser->getOptions()->getIsSectionPreview() );
}
}
}

View file

@ -32,8 +32,7 @@ class CiteParserTagHooks {
* @return string HTML
*/
public static function ref( $content, array $attributes, Parser $parser, PPFrame $frame ) {
/** @var Cite $cite */
$cite = $parser->extCite;
$cite = self::citeForParser( $parser );
$result = $cite->ref( $content, $attributes, $parser );
if ( $result === false ) {
@ -60,8 +59,7 @@ class CiteParserTagHooks {
* @return string HTML
*/
public static function references( $content, array $attributes, Parser $parser, PPFrame $frame ) {
/** @var Cite $cite */
$cite = $parser->extCite;
$cite = self::citeForParser( $parser );
$result = $cite->references( $content, $attributes, $parser );
if ( $result === false ) {
@ -76,4 +74,17 @@ class CiteParserTagHooks {
return $result;
}
/**
* Get or create Cite state for this parser.
*
* @param Parser $parser
* @return Cite
*/
private static function citeForParser( Parser $parser ): Cite {
if ( !isset( $parser->extCite ) ) {
$parser->extCite = new Cite();
}
return $parser->extCite;
}
}

View file

@ -347,16 +347,6 @@ class ReferenceStack {
}
}
/**
* Reset all state.
*/
public function clear() {
$this->groupRefSequence = [];
$this->refSequence = 0;
$this->refs = [];
$this->refCallStack = [];
}
/**
* Clear state for a single group.
*

View file

@ -29,38 +29,19 @@ class CiteParserHooksTest extends \MediaWikiUnitTestCase {
);
CiteParserHooks::onParserFirstCallInit( $parser );
$this->assertInstanceOf( Cite::class, $parser->extCite );
}
/**
* @covers ::onParserClearState
*/
public function testOnParserClearState() {
$cite = $this->createMock( Cite::class );
$cite->expects( $this->once() )
->method( 'clearState' );
$parser = $this->createMock( Parser::class );
$parser->extCite = $cite;
CiteParserHooks::onParserClearState( $parser );
}
/**
* @covers ::onParserCloned
* @covers ::onParserClearStateOrCloned
*/
public function testOnParserCloned() {
$cite = $this->createMock( Cite::class );
$cite->expects( $this->once() )
->method( 'clearState' );
$parser = $this->createMock( Parser::class );
$parser->extCite = $cite;
$parser->extCite = $this->createMock( Cite::class );
CiteParserHooks::onParserCloned( $parser );
/** @var Parser $parser */
CiteParserHooks::onParserClearStateOrCloned( $parser );
$this->assertNotSame( $cite, $parser->extCite );
$this->assertFalse( isset( $parser->extCite ) );
}
/**

View file

@ -1009,7 +1009,6 @@ class ReferenceStackTest extends MediaWikiUnitTestCase {
}
/**
* @covers ::clear
* @covers ::deleteGroup
*/
public function testRemovals() {
@ -1018,9 +1017,6 @@ class ReferenceStackTest extends MediaWikiUnitTestCase {
$stack->deleteGroup( 'group1' );
$this->assertSame( [ 'group2' => [] ], $stack->refs );
$stack->clear();
$this->assertSame( [], $stack->refs );
}
/**