From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Mon, 12 Jun 2017 15:02:58 +0000 (-0400)
Subject: ApiParse: Clean up parsing code
X-Git-Tag: 1.31.0-rc.0~2997^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/recherche.php?a=commitdiff_plain;h=1e2b3fb37f3452ee9037f6efff24d86275308e7b;p=lhc%2Fweb%2Fwiklou.git

ApiParse: Clean up parsing code

Now that ParserOptions->isSafeToCache() exists, use it where necessary.
This also moves the use inside the makeParserOptions() method so other
callers can pick it up as well.

Then pass the flag as $forceParse into WikiPage::getParserOutput()
instead of duplicating the logic in several cases, and generally clean
up the logic in the module to let WikiPage decide when to use the cache
in more cases.

Change-Id: I0079e10a40997e4a3b59ac21ef6c92246a147736
---

diff --git a/includes/api/ApiParse.php b/includes/api/ApiParse.php
index 91e49abc9c..402494c617 100644
--- a/includes/api/ApiParse.php
+++ b/includes/api/ApiParse.php
@@ -38,6 +38,9 @@ class ApiParse extends ApiBase {
 	/** @var Content $pstContent */
 	private $pstContent = null;
 
+	/** @var bool */
+	private $contentIsDeleted = false, $contentIsSuppressed = false;
+
 	public function execute() {
 		// The data is hot but user-dependent, like page views, so we set vary cookies
 		$this->getMain()->setCacheMode( 'anon-public-user-private' );
@@ -110,27 +113,9 @@ class ApiParse extends ApiBase {
 				$wgTitle = $titleObj;
 				$pageObj = WikiPage::factory( $titleObj );
 				list( $popts, $reset, $suppressCache ) = $this->makeParserOptions( $pageObj, $params );
-
-				// If for some reason the "oldid" is actually the current revision, it may be cached
-				// Deliberately comparing $pageObj->getLatest() with $rev->getId(), rather than
-				// checking $rev->isCurrent(), because $pageObj is what actually ends up being used,
-				// and if its ->getLatest() is outdated, $rev->isCurrent() won't tell us that.
-				if ( !$suppressCache && $rev->getId() == $pageObj->getLatest() ) {
-					// May get from/save to parser cache
-					$p_result = $this->getParsedContent( $pageObj, $popts,
-						$pageid, isset( $prop['wikitext'] ) );
-				} else { // This is an old revision, so get the text differently
-					$this->content = $rev->getContent( Revision::FOR_THIS_USER, $this->getUser() );
-
-					if ( $this->section !== false ) {
-						$this->content = $this->getSectionContent(
-							$this->content, $this->msg( 'revid', $rev->getId() )
-						);
-					}
-
-					// Should we save old revision parses to the parser cache?
-					$p_result = $this->content->getParserOutput( $titleObj, $rev->getId(), $popts );
-				}
+				$p_result = $this->getParsedContent(
+					$pageObj, $popts, $suppressCache, $pageid, $rev, isset( $prop['wikitext'] )
+				);
 			} else { // Not $oldid, but $pageid or $page
 				if ( $params['redirects'] ) {
 					$reqParams = [
@@ -172,25 +157,9 @@ class ApiParse extends ApiBase {
 				}
 
 				list( $popts, $reset, $suppressCache ) = $this->makeParserOptions( $pageObj, $params );
-
-				// Don't pollute the parser cache when setting options that aren't
-				// in ParserOptions::optionsHash()
-				/// @todo: This should be handled closer to the actual cache instead of here, see T110269
-				$suppressCache = $suppressCache ||
-					$params['disablepp'] ||
-					$params['disablelimitreport'] ||
-					$params['preview'] ||
-					$params['sectionpreview'] ||
-					$params['disabletidy'];
-
-				if ( $suppressCache ) {
-					$this->content = $this->getContent( $pageObj, $pageid );
-					$p_result = $this->content->getParserOutput( $titleObj, null, $popts );
-				} else {
-					// Potentially cached
-					$p_result = $this->getParsedContent( $pageObj, $popts, $pageid,
-						isset( $prop['wikitext'] ) );
-				}
+				$p_result = $this->getParsedContent(
+					$pageObj, $popts, $suppressCache, $pageid, null, isset( $prop['wikitext'] )
+				);
 			}
 		} else { // Not $oldid, $pageid, $page. Hence based on $text
 			$titleObj = Title::newFromText( $title );
@@ -249,6 +218,12 @@ class ApiParse extends ApiBase {
 			if ( $params['onlypst'] ) {
 				// Build a result and bail out
 				$result_array = [];
+				if ( $this->contentIsDeleted ) {
+					$result_array['textdeleted'] = true;
+				}
+				if ( $this->contentIsSuppressed ) {
+					$result_array['textsuppressed'] = true;
+				}
 				$result_array['text'] = $this->pstContent->serialize( $format );
 				$result_array[ApiResult::META_BC_SUBELEMENTS][] = 'text';
 				if ( isset( $prop['wikitext'] ) ) {
@@ -279,6 +254,12 @@ class ApiParse extends ApiBase {
 
 		$result_array['title'] = $titleObj->getPrefixedText();
 		$result_array['pageid'] = $pageid ?: $pageObj->getId();
+		if ( $this->contentIsDeleted ) {
+			$result_array['textdeleted'] = true;
+		}
+		if ( $this->contentIsSuppressed ) {
+			$result_array['textsuppressed'] = true;
+		}
 
 		if ( $params['disabletoc'] ) {
 			$p_result->setTOCEnabled( false );
@@ -541,64 +522,72 @@ class ApiParse extends ApiBase {
 		Hooks::run( 'ApiMakeParserOptions',
 			[ $popts, $pageObj->getTitle(), $params, $this, &$reset, &$suppressCache ] );
 
+		// Force cache suppression when $popts aren't cacheable.
+		$suppressCache = $suppressCache || !$popts->isSafeToCache();
+
 		return [ $popts, $reset, $suppressCache ];
 	}
 
 	/**
 	 * @param WikiPage $page
 	 * @param ParserOptions $popts
+	 * @param bool $suppressCache
 	 * @param int $pageId
-	 * @param bool $getWikitext
+	 * @param Revision|null $rev
+	 * @param bool $getContent
 	 * @return ParserOutput
 	 */
-	private function getParsedContent( WikiPage $page, $popts, $pageId = null, $getWikitext = false ) {
-		$this->content = $this->getContent( $page, $pageId );
+	private function getParsedContent(
+		WikiPage $page, $popts, $suppressCache, $pageId, $rev, $getContent
+	) {
+		$revId = $rev ? $rev->getId() : null;
+		$isDeleted = $rev && $rev->isDeleted( Revision::DELETED_TEXT );
+
+		if ( $getContent || $this->section !== false || $isDeleted ) {
+			if ( $rev ) {
+				$this->content = $rev->getContent( Revision::FOR_THIS_USER, $this->getUser() );
+				if ( !$this->content ) {
+					$this->dieWithError( [ 'apierror-missingcontent-revid', $revId ] );
+				}
+			} else {
+				$this->content = $page->getContent( Revision::FOR_THIS_USER, $this->getUser() );
+				if ( !$this->content ) {
+					$this->dieWithError( [ 'apierror-missingcontent-pageid', $pageId ] );
+				}
+			}
+			$this->contentIsDeleted = $isDeleted;
+			$this->contentIsSuppressed = $rev &&
+				$rev->isDeleted( Revision::DELETED_TEXT | Revision::DELETED_RESTRICTED );
+		}
 
-		if ( $this->section !== false && $this->content !== null ) {
-			// Not cached (save or load)
-			return $this->content->getParserOutput( $page->getTitle(), null, $popts );
+		if ( $this->section !== false ) {
+			$this->content = $this->getSectionContent(
+				$this->content,
+				$pageId === null ? $page->getTitle()->getPrefixedText() : $this->msg( 'pageid', $pageId )
+			);
+			return $this->content->getParserOutput( $page->getTitle(), $revId, $popts );
 		}
 
-		// Try the parser cache first
-		// getParserOutput will save to Parser cache if able
-		$pout = $page->getParserOutput( $popts );
-		if ( !$pout ) {
-			$this->dieWithError( [ 'apierror-nosuchrevid', $page->getLatest() ] );
+		if ( $isDeleted ) {
+			// getParserOutput can't do revdeled revisions
+			$pout = $this->content->getParserOutput( $page->getTitle(), $revId, $popts );
+		} else {
+			// getParserOutput will save to Parser cache if able
+			$pout = $page->getParserOutput( $popts, $revId, $suppressCache );
 		}
-		if ( $getWikitext ) {
-			$this->content = $page->getContent( Revision::RAW );
+		if ( !$pout ) {
+			$this->dieWithError( [ 'apierror-nosuchrevid', $revId ?: $page->getLatest() ] );
 		}
 
 		return $pout;
 	}
 
-	/**
-	 * Get the content for the given page and the requested section.
-	 *
-	 * @param WikiPage $page
-	 * @param int $pageId
-	 * @return Content
-	 */
-	private function getContent( WikiPage $page, $pageId = null ) {
-		$content = $page->getContent( Revision::RAW ); // XXX: really raw?
-
-		if ( $this->section !== false && $content !== null ) {
-			$content = $this->getSectionContent(
-				$content,
-				!is_null( $pageId )
-					? $this->msg( 'pageid', $pageId )
-					: $page->getTitle()->getPrefixedText()
-			);
-		}
-		return $content;
-	}
-
 	/**
 	 * Extract the requested section from the given Content
 	 *
 	 * @param Content $content
 	 * @param string|Message $what Identifies the content in error messages, e.g. page title.
-	 * @return Content|bool
+	 * @return Content
 	 */
 	private function getSectionContent( Content $content, $what ) {
 		// Not cached (save or load)
diff --git a/tests/phpunit/includes/api/ApiParseTest.php b/tests/phpunit/includes/api/ApiParseTest.php
index f01a670b71..028d3b4135 100644
--- a/tests/phpunit/includes/api/ApiParseTest.php
+++ b/tests/phpunit/includes/api/ApiParseTest.php
@@ -9,18 +9,117 @@
  */
 class ApiParseTest extends ApiTestCase {
 
-	protected function setUp() {
-		parent::setUp();
-		$this->doLogin();
+	protected static $pageId;
+	protected static $revIds = [];
+
+	public function addDBDataOnce() {
+		$user = static::getTestSysop()->getUser();
+		$title = Title::newFromText( __CLASS__ );
+		$page = WikiPage::factory( $title );
+
+		$status = $page->doEditContent(
+			ContentHandler::makeContent( 'Test for revdel', $title, CONTENT_MODEL_WIKITEXT ),
+			__METHOD__ . ' Test for revdel', 0, false, $user
+		);
+		if ( !$status->isOk() ) {
+			$this->fail( "Failed to create $title: " . $status->getWikiText( false, false, 'en' ) );
+		}
+		self::$pageId = $status->value['revision']->getPage();
+		self::$revIds['revdel'] = $status->value['revision']->getId();
+
+		$status = $page->doEditContent(
+			ContentHandler::makeContent( 'Test for oldid', $title, CONTENT_MODEL_WIKITEXT ),
+			__METHOD__ . ' Test for oldid', 0, false, $user
+		);
+		if ( !$status->isOk() ) {
+			$this->fail( "Failed to edit $title: " . $status->getWikiText( false, false, 'en' ) );
+		}
+		self::$revIds['oldid'] = $status->value['revision']->getId();
+
+		$status = $page->doEditContent(
+			ContentHandler::makeContent( 'Test for latest', $title, CONTENT_MODEL_WIKITEXT ),
+			__METHOD__ . ' Test for latest', 0, false, $user
+		);
+		if ( !$status->isOk() ) {
+			$this->fail( "Failed to edit $title: " . $status->getWikiText( false, false, 'en' ) );
+		}
+		self::$revIds['latest'] = $status->value['revision']->getId();
+
+		RevisionDeleter::createList(
+			'revision', RequestContext::getMain(), $title, [ self::$revIds['revdel'] ]
+		)->setVisibility( [
+			'value' => [
+				Revision::DELETED_TEXT => 1,
+			],
+			'comment' => 'Test for revdel',
+		] );
+
+		Title::clearCaches(); // Otherwise it has the wrong latest revision for some reason
 	}
 
-	public function testParseNonexistentPage() {
-		$somePage = mt_rand();
+	public function testParseByName() {
+		$res = $this->doApiRequest( [
+			'action' => 'parse',
+			'page' => __CLASS__,
+		] );
+		$this->assertContains( 'Test for latest', $res[0]['parse']['text'] );
+
+		$res = $this->doApiRequest( [
+			'action' => 'parse',
+			'page' => __CLASS__,
+			'disablelimitreport' => 1,
+		] );
+		$this->assertContains( 'Test for latest', $res[0]['parse']['text'] );
+	}
+
+	public function testParseById() {
+		$res = $this->doApiRequest( [
+			'action' => 'parse',
+			'pageid' => self::$pageId,
+		] );
+		$this->assertContains( 'Test for latest', $res[0]['parse']['text'] );
+	}
+
+	public function testParseByOldId() {
+		$res = $this->doApiRequest( [
+			'action' => 'parse',
+			'oldid' => self::$revIds['oldid'],
+		] );
+		$this->assertContains( 'Test for oldid', $res[0]['parse']['text'] );
+		$this->assertArrayNotHasKey( 'textdeleted', $res[0]['parse'] );
+		$this->assertArrayNotHasKey( 'textsuppressed', $res[0]['parse'] );
+	}
+
+	public function testParseRevDel() {
+		$user = static::getTestUser()->getUser();
+		$sysop = static::getTestSysop()->getUser();
 
 		try {
 			$this->doApiRequest( [
 				'action' => 'parse',
-				'page' => $somePage ] );
+				'oldid' => self::$revIds['revdel'],
+			], null, null, $user );
+			$this->fail( "API did not return an error as expected" );
+		} catch ( ApiUsageException $ex ) {
+			$this->assertTrue( ApiTestCase::apiExceptionHasCode( $ex, 'permissiondenied' ),
+				"API failed with error 'permissiondenied'" );
+		}
+
+		$res = $this->doApiRequest( [
+			'action' => 'parse',
+			'oldid' => self::$revIds['revdel'],
+		], null, null, $sysop );
+		$this->assertContains( 'Test for revdel', $res[0]['parse']['text'] );
+		$this->assertArrayHasKey( 'textdeleted', $res[0]['parse'] );
+		$this->assertArrayNotHasKey( 'textsuppressed', $res[0]['parse'] );
+	}
+
+	public function testParseNonexistentPage() {
+		try {
+			$this->doApiRequest( [
+				'action' => 'parse',
+				'page' => 'DoesNotExist',
+			] );
 
 			$this->fail( "API did not return an error when parsing a nonexistent page" );
 		} catch ( ApiUsageException $ex ) {