Patch for bug #1138 (ipblock-related)

diff --git a/includes/BlockCache.php b/includes/BlockCache.php
index 8f9a5a6..ee8e2b8 100644 (file)
--- a/includes/BlockCache.php
+++ b/includes/BlockCache.php
@@ -36,15 +36,16 @@ class BlockCache
 
        /**
         * Load the blocks from the database and save them to memcached
+        *  @param bool $bFromSlave Whether to load data from slaves or master
         */
-       function loadFromDB() {
+       function loadFromDB( $bFromSlave = false ) {
                global $wgUseMemCached, $wgMemc;
                $this->mData = array();
                # Selecting FOR UPDATE is a convenient way to serialise the memcached and DB operations,
                # which is necessary even though we don't update the DB
-               if ( $wgUseMemCached ) {
+               if ( !$bFromSlave ) {
                        Block::enumBlocks( 'wfBlockCacheInsert', '', EB_FOR_UPDATE );
-                       $wgMemc->set( $this->mMemcKey, $this->mData, 0 );
+                       #$wgMemc->set( $this->mMemcKey, $this->mData, 0 );
                } else {
                        Block::enumBlocks( 'wfBlockCacheInsert', '' );
                }
@@ -53,18 +54,21 @@ class BlockCache
        /**
         * Load the cache from memcached or, if that's not possible, from the DB
         */
-       function load() {
+       function load( $bFromSlave ) {
                global $wgUseMemCached, $wgMemc;
 
                if ( $this->mData === false) {
+                       $this->loadFromDB( $bFromSlave );
+/*
+               // Memcache disabled for performance issues.
                        # Try memcached
                        if ( $wgUseMemCached ) {
                                $this->mData = $wgMemc->get( $this->mMemcKey );
                        }
 
                        if ( !is_array( $this->mData ) ) {
-                               $this->loadFromDB();
-                       }
+                               $this->loadFromDB( $bFromSlave );
+                       }*/
                }
        }
 
@@ -91,9 +95,10 @@ class BlockCache
         * Find out if a given IP address is blocked
         *
         * @param String $ip   IP address
+        * @param bool $bFromSlave True means to load check against slave, else check against master.
         */
-       function get( $ip ) {
-               $this->load();
+       function get( $ip, $bFromSlave ) {
+               $this->load( $bFromSlave );
                $ipint = ip2long( $ip );
                $blocked = false;
 
@@ -110,6 +115,7 @@ class BlockCache
                                $ip = Block::normaliseRange( $ip );
                        }
                        $block = new Block();
+                       $block->forUpdate( $bFromSlave );
                        $block->load( $ip );
                } else {
                        $block = false;

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 95e59c8..9c69f48 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -150,7 +150,9 @@ class EditPage {
                        $wgOut->readOnlyPage( $this->mArticle->getContent( true ), true );
                        return;
                }
-               if ( $wgUser->isBlocked() ) {
+               if ( !$this->preview && $wgUser->isBlocked( !$this->save ) ) {
+                       # When previewing, don't check blocked state - will get caught at save time.
+                       # Also, check when starting edition is done against slave to improve performance.
                        $this->blockedIPpage();
                        return;
                }
@@ -310,7 +312,8 @@ class EditPage {
                                # Error messages or other handling should be performed by the filter function
                                return;
                        }
-                       if ( $wgUser->isBlocked() ) {
+                       if ( $wgUser->isBlocked( false ) ) {
+                               # Check block state against master, thus 'false'.
                                $this->blockedIPpage();
                                return;
                        }

diff --git a/includes/User.php b/includes/User.php
index a116389..d0ac0d5 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -242,8 +242,16 @@ class User {
        /**
         * Get blocking information
         * @access private
+        * @param bool $bFromSlave Specify whether to check slave or master. To improve performance,
+        *  non-critical checks are done against slaves. Check when actually saving should be done against
+        *  master.
+        *
+        * Note that even if $bFromSlave is false, the check is done first against slave, then master.
+        * The logic is that if blocked on slave, we'll assume it's either blocked on master or
+        * just slightly outta sync and soon corrected - safer to block slightly more that less.
+        * And it's cheaper to check slave first, then master if needed, than master always.
         */
-       function getBlockedStatus() {
+       function getBlockedStatus( $bFromSlave = false ) {
                global $wgIP, $wgBlockCache, $wgProxyList;
 
                if ( -1 != $this->mBlockedby ) { return; }
@@ -253,7 +261,8 @@ class User {
                # User blocking
                if ( $this->mId ) {
                        $block = new Block();
-                       if ( $block->load( $wgIP , $this->mId ) ) {
+                       $block->forUpdate( $bFromSlave );
+                       if ( $block->load( $wgIP , $this->mId ) ) {
                                $this->mBlockedby = $block->mBy;
                                $this->mBlockreason = $block->mReason;
                        }
@@ -261,7 +270,14 @@ class User {
 
                # IP/range blocking
                if ( !$this->mBlockedby ) {
-                       $block = $wgBlockCache->get( $wgIP );
+                       # Check first against slave, and optionally from master.
+                       $block = $wgBlockCache->get( $wgIP, true );
+                       if ( !block && !$bFromSlave )
+                               {
+                               # Not blocked: check against master, to make sure.
+                               $wgBlockCache->clearLocal( );
+                               $block = $wgBlockCache->get( $wgIP, false );
+                               }
                        if ( $block !== false ) {
                                $this->mBlockedby = $block->mBy;
                                $this->mBlockreason = $block->mReason;
@@ -281,8 +297,8 @@ class User {
         * Check if user is blocked
         * @return bool True if blocked, false otherwise
         */
-       function isBlocked() {
-               $this->getBlockedStatus();
+       function isBlocked( $bFromSlave = false ) {
+               $this->getBlockedStatus( $bFromSlave );
                if ( 0 === $this->mBlockedby ) { return false; }
                return true;
        }