From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 5 Mar 2015 01:02:05 +0000 (-0800)
Subject: Made the "bypassCache" flag for file locator methods use the master DB
X-Git-Tag: 1.31.0-rc.0~12178^2
X-Git-Url: http://git.cyclocoop.org/%22.%24image2.%22?a=commitdiff_plain;h=7508b86ad37f0533aa38770baff2367a82af651c;p=lhc%2Fweb%2Fwiklou.git

Made the "bypassCache" flag for file locator methods use the master DB

* Added a File::load() stub method
* Cleaned up the File loading flags bitfield a bit

bug: T89184
Change-Id: I1aa4b096c0cad5f5ca34321cc897019005c53a76
---

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 1d44038f5e..883153f86d 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -3650,19 +3650,7 @@ function wfGetLBFactory() {
  * Shortcut for RepoGroup::singleton()->findFile()
  *
  * @param string $title String or Title object
- * @param array $options Associative array of options:
- *     time:           requested time for an archived image, or false for the
- *                     current version. An image object will be returned which was
- *                     created at the specified time.
- *
- *     ignoreRedirect: If true, do not follow file redirects
- *
- *     private:        If true, return restricted (deleted) files if the current
- *                     user is allowed to view them. Otherwise, such files will not
- *                     be found.
- *
- *     bypassCache:    If true, do not use the process-local cache of File objects
- *
+ * @param array $options Associative array of options (see RepoGroup::findFile)
  * @return File|bool File, or false if the file does not exist
  */
 function wfFindFile( $title, $options = array() ) {
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index d1a16b5460..01495a4032 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -406,6 +406,7 @@ class FileRepo {
 	 *   private:        If true, return restricted (deleted) files if the current
 	 *                   user is allowed to view them. Otherwise, such files will not
 	 *                   be found. If a User object, use that user instead of the current.
+	 *   bypassCache:    If true, do not use the process/persistent cache of File objects
 	 * @return File|bool False on failure
 	 */
 	public function findFile( $title, $options = array() ) {
@@ -414,17 +415,20 @@ class FileRepo {
 			return false;
 		}
 		$time = isset( $options['time'] ) ? $options['time'] : false;
+		$flags = !empty( $options['bypassCache'] ) ? File::READ_LATEST : 0;
 		# First try the current version of the file to see if it precedes the timestamp
 		$img = $this->newFile( $title );
 		if ( !$img ) {
 			return false;
 		}
+		$img->load( $flags );
 		if ( $img->exists() && ( !$time || $img->getTimestamp() == $time ) ) {
 			return $img;
 		}
 		# Now try an old version of the file
 		if ( $time !== false ) {
 			$img = $this->newFile( $title, $time );
+			$img->load( $flags );
 			if ( $img && $img->exists() ) {
 				if ( !$img->isDeleted( File::DELETED_FILE ) ) {
 					return $img; // always OK
@@ -445,6 +449,7 @@ class FileRepo {
 		$redir = $this->checkRedirect( $title );
 		if ( $redir && $title->getNamespace() == NS_FILE ) {
 			$img = $this->newFile( $redir );
+			$img->load( $flags );
 			if ( !$img ) {
 				return false;
 			}
diff --git a/includes/filerepo/RepoGroup.php b/includes/filerepo/RepoGroup.php
index fab4216281..6ac00dec24 100644
--- a/includes/filerepo/RepoGroup.php
+++ b/includes/filerepo/RepoGroup.php
@@ -114,7 +114,7 @@ class RepoGroup {
 	 *   private:        If true, return restricted (deleted) files if the current
 	 *                   user is allowed to view them. Otherwise, such files will not
 	 *                   be found.
-	 *   bypassCache:    If true, do not use the process-local cache of File objects
+	 *   bypassCache:    If true, do not use the process/persistent cache of File objects
 	 * @return File|bool False if title is not found
 	 */
 	function findFile( $title, $options = array() ) {
diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php
index 2721693461..4fd332bebd 100644
--- a/includes/filerepo/file/File.php
+++ b/includes/filerepo/file/File.php
@@ -47,7 +47,7 @@
  *
  * @ingroup FileAbstraction
  */
-abstract class File {
+abstract class File implements IDBAccessObject {
 	// Bitfield values akin to the Revision deletion constants
 	const DELETED_FILE = 1;
 	const DELETED_COMMENT = 2;
@@ -836,6 +836,18 @@ abstract class File {
 		return false;
 	}
 
+	/**
+	 * Load any lazy-loaded file object fields from source
+	 *
+	 * This is only useful when setting $flags
+	 *
+	 * Overridden by LocalFile to actually query the DB
+	 *
+	 * @param integer $flags Bitfield of File::READ_* constants
+	 */
+	public function load( $flags = 0 ) {
+	}
+
 	/**
 	 * Returns true if file exists in the repository.
 	 *
diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index 699c9154a2..3056ad8407 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -127,8 +127,8 @@ class LocalFile extends File {
 	/** @var int UNIX timestamp of last markVolatile() call */
 	private $lastMarkedVolatile = 0;
 
-	const LOAD_ALL = 1; // integer; load all the lazy fields too (like metadata)
-	const LOAD_VIA_SLAVE = 2; // integer; use a slave to load the data
+	// @note: higher than IDBAccessObject constants
+	const LOAD_ALL = 16; // integer; load all the lazy fields too (like metadata)
 
 	const VOLATILE_TTL = 300; // integer; seconds
 
@@ -387,9 +387,9 @@ class LocalFile extends File {
 		$this->dataLoaded = true;
 		$this->extraDataLoaded = true;
 
-		$dbr = ( $flags & self::LOAD_VIA_SLAVE )
-			? $this->repo->getSlaveDB()
-			: $this->repo->getMasterDB();
+		$dbr = ( $flags & self::READ_LATEST )
+			? $this->repo->getMasterDB()
+			: $this->repo->getSlaveDB();
 
 		$row = $dbr->selectRow( 'image', $this->getCacheFields( 'img_' ),
 			array( 'img_name' => $this->getName() ), $fname );
@@ -530,13 +530,18 @@ class LocalFile extends File {
 	 */
 	function load( $flags = 0 ) {
 		if ( !$this->dataLoaded ) {
-			if ( !$this->loadFromCache() ) {
-				$this->loadFromDB( $this->isVolatile() ? 0 : self::LOAD_VIA_SLAVE );
+			if ( ( $flags & self::READ_LATEST ) || !$this->loadFromCache() ) {
+				// b/c for now for data consistency
+				if ( $this->isVolatile() ) {
+					$flags |= self::READ_LATEST;
+				}
+				$this->loadFromDB( $flags );
 				$this->saveToCache();
 			}
 			$this->dataLoaded = true;
 		}
 		if ( ( $flags & self::LOAD_ALL ) && !$this->extraDataLoaded ) {
+			// @note: loads on name/timestamp to reduce race condition problems
 			$this->loadExtraFromDB();
 		}
 	}
diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php
index 73c614a3ec..fd92e11a06 100644
--- a/includes/filerepo/file/OldLocalFile.php
+++ b/includes/filerepo/file/OldLocalFile.php
@@ -175,10 +175,12 @@ class OldLocalFile extends LocalFile {
 	}
 
 	function loadFromDB( $flags = 0 ) {
-
 		$this->dataLoaded = true;
 
-		$dbr = $this->repo->getSlaveDB();
+		$dbr = ( $flags & self::READ_LATEST )
+			? $this->repo->getMasterDB()
+			: $this->repo->getSlaveDB();
+
 		$conds = array( 'oi_name' => $this->getName() );
 		if ( is_null( $this->requestedTime ) ) {
 			$conds['oi_archive_name'] = $this->archive_name;