From 3a56848dfc6722240f1dd6905313c79f9c8b4ecc Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 30 Sep 2015 00:50:54 -0700
Subject: [PATCH] Converted checkRedirect() to using getWithSetCallback()

* Also merged in the code for LocalRepo::getArticleID()

Change-Id: I80d46d2358a48a39e251be83cdb336f663dbfaff
---
 includes/filerepo/LocalRepo.php | 88 ++++++++++++---------------------
 1 file changed, 32 insertions(+), 56 deletions(-)

diff --git a/includes/filerepo/LocalRepo.php b/includes/filerepo/LocalRepo.php
index 63614638fd..c7ca4c2eb8 100644
--- a/includes/filerepo/LocalRepo.php
+++ b/includes/filerepo/LocalRepo.php
@@ -189,8 +189,6 @@ class LocalRepo extends FileRepo {
 	 * @return bool|Title
 	 */
 	function checkRedirect( Title $title ) {
-		$cache = ObjectCache::getMainWANInstance();
-
 		$title = File::normalizeTitle( $title, 'exception' );
 
 		$memcKey = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) );
@@ -201,65 +199,43 @@ class LocalRepo extends FileRepo {
 			$expiry = 86400; // has invalidation, 1 day
 		}
 
-		$cachedValue = $cache->get( $memcKey );
-		if ( $cachedValue === ' ' || $cachedValue === '' ) {
-			// Does not exist
-			return false;
-		} elseif ( strval( $cachedValue ) !== '' && $cachedValue !== ' PURGED' ) {
-			return Title::newFromText( $cachedValue, NS_FILE );
-		} // else $cachedValue is false or null: cache miss
-
-		$opts = array( 'since' => $this->getSlaveDB()->trxTimestamp() );
-
-		$id = $this->getArticleID( $title );
-		if ( !$id ) {
-			$cache->set( $memcKey, " ", $expiry, $opts );
+		$that = $this;
+		$redirDbKey = ObjectCache::getMainWANInstance()->getWithSetCallback(
+			$memcKey,
+			function ( $oldValue, &$ttl, array &$setOpts ) use ( $that, $title ) {
+				$dbr = $that->getSlaveDB(); // possibly remote DB
+
+				$setOpts = array( 'since' => $dbr->trxTimestamp() );
+
+				if ( $title instanceof Title ) {
+					$row = $dbr->selectRow(
+						array( 'page', 'redirect' ),
+						array( 'rd_namespace', 'rd_title' ),
+						array(
+							'page_namespace' => $title->getNamespace(),
+							'page_title' => $title->getDBkey(),
+							'rd_from = page_id'
+						),
+						__METHOD__
+					);
+				} else {
+					$row = false;
+				}
 
-			return false;
-		}
-		$dbr = $this->getSlaveDB();
-		$row = $dbr->selectRow(
-			'redirect',
-			array( 'rd_title', 'rd_namespace' ),
-			array( 'rd_from' => $id ),
-			__METHOD__
+				return ( $row && $row->rd_namespace == NS_FILE )
+					? Title::makeTitle( $row->rd_namespace, $row->rd_title )->getDBkey()
+					: ''; // negative cache
+			},
+			$expiry
 		);
 
-		if ( $row && $row->rd_namespace == NS_FILE ) {
-			$targetTitle = Title::makeTitle( $row->rd_namespace, $row->rd_title );
-			$cache->set( $memcKey, $targetTitle->getDBkey(), $expiry, $opts );
-
-			return $targetTitle;
-		} else {
-			$cache->set( $memcKey, '', $expiry, $opts );
-
-			return false;
+		// @note: also checks " " for b/c
+		if ( $redirDbKey !== ' ' && strval( $redirDbKey ) !== '' ) {
+			// Page is a redirect to another file
+			return Title::newFromText( $redirDbKey, NS_FILE );
 		}
-	}
-
-	/**
-	 * Function link Title::getArticleID().
-	 * We can't say Title object, what database it should use, so we duplicate that function here.
-	 *
-	 * @param Title $title
-	 * @return bool|int|mixed
-	 */
-	protected function getArticleID( $title ) {
-		if ( !$title instanceof Title ) {
-			return 0;
-		}
-		$dbr = $this->getSlaveDB();
-		$id = $dbr->selectField(
-			'page', // Table
-			'page_id', // Field
-			array( // Conditions
-				'page_namespace' => $title->getNamespace(),
-				'page_title' => $title->getDBkey(),
-			),
-			__METHOD__ // Function name
-		);
 
-		return $id;
+		return false; // no redirect
 	}
 
 	public function findFiles( array $items, $flags = 0 ) {
-- 
2.20.1