Disallow setting DBO_IGNORE in Database for sanity

In the off chance something called this, it would break all
sorts of code that expects that either query result functions
either succeed or throw an error.

Callers are not expected to have to check if the result of
a query is meaningful or false due to an error.

Change-Id: I0b4fe1403f55a399ffd40817ed12f857087d6f83

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index e04566e..5edf3fd 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -461,9 +461,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected function ignoreErrors( $ignoreErrors = null ) {
                $res = $this->getFlag( self::DBO_IGNORE );
                if ( $ignoreErrors !== null ) {
-                       $ignoreErrors
-                               ? $this->setFlag( self::DBO_IGNORE )
-                               : $this->clearFlag( self::DBO_IGNORE );
+                       // setFlag()/clearFlag() do not allow DBO_IGNORE changes for sanity
+                       if ( $ignoreErrors ) {
+                               $this->mFlags |= self::DBO_IGNORE;
+                       } else {
+                               $this->mFlags &= ~self::DBO_IGNORE;
+                       }
                }
 
                return $res;
@@ -621,6 +624,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) {
+               if ( ( $flag & self::DBO_IGNORE ) ) {
+                       throw new \UnexpectedValueException( "Modifying DBO_IGNORE is not allowed." );
+               }
+
                if ( $remember === self::REMEMBER_PRIOR ) {
                        array_push( $this->priorFlags, $this->mFlags );
                }
@@ -628,6 +635,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ) {
+               if ( ( $flag & self::DBO_IGNORE ) ) {
+                       throw new \UnexpectedValueException( "Modifying DBO_IGNORE is not allowed." );
+               }
+
                if ( $remember === self::REMEMBER_PRIOR ) {
                        array_push( $this->priorFlags, $this->mFlags );
                }

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
index 456447f..461ef09 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
@@ -27,6 +27,7 @@ use Wikimedia\Rdbms\TransactionProfiler;
 use Wikimedia\Rdbms\DatabaseDomain;
 use Wikimedia\Rdbms\MySQLMasterPos;
 use Wikimedia\Rdbms\DatabaseMysqlBase;
+use Wikimedia\Rdbms\Database;
 
 /**
  * Fake class around abstract class so we can call concrete methods.
@@ -368,4 +369,24 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase {
                        [ 1000.77 ],
                ];
        }
+
+       /**
+        * @expectedException UnexpectedValueException
+        * @covers Wikimedia\Rdbms\Database::setFlag
+        */
+       public function testDBOIgnoreSet() {
+               $db = new FakeDatabaseMysqlBase();
+
+               $db->setFlag( Database::DBO_IGNORE );
+       }
+
+       /**
+        * @expectedException UnexpectedValueException
+        * @covers Wikimedia\Rdbms\Database::clearFlag
+        */
+       public function testDBOIgnoreClear() {
+               $db = new FakeDatabaseMysqlBase();
+
+               $db->clearFlag( Database::DBO_IGNORE );
+       }
 }