(minor) Make ORMTable handle failed queries nicely.

Database::select will return false if a query fails and the DB object
is set to "ignore errors" mode. ORMTable now detect this and throw
a DBQueryError, overriding the ignore errors mode.

This change also adds a dummy implementation for ORMTable to ORMTableTest,
so tests can be run on the base implementation.

Change-Id: I5d87295626c0800c38f807d19becb312ba7cc956

diff --git a/includes/db/IORMTable.php b/includes/db/IORMTable.php
index 67dbdfe..e46dbc0 100644 (file)
--- a/includes/db/IORMTable.php
+++ b/includes/db/IORMTable.php
@@ -97,6 +97,8 @@ interface IORMTable {
         * Selects the the specified fields of the records matching the provided
         * conditions and returns them as DBDataObject. Field names get prefixed.
         *
+        * @see DatabaseBase::select()
+        *
         * @since 1.20
         *
         * @param array|string|null $fields
@@ -104,7 +106,8 @@ interface IORMTable {
         * @param array $options
         * @param string|null $functionName
         *
-        * @return ORMResult
+        * @return ORMResult The result set
+        * @throw DBQueryError if the query failed (even if the database was in ignoreErrors mode)
         */
        public function select( $fields = null, array $conditions = array(),
                                                        array $options = array(), $functionName = null );
@@ -136,6 +139,7 @@ interface IORMTable {
         * @param null|string $functionName
         *
         * @return ResultWrapper
+        * @throw DBQueryError if the query failed (even if the database was in ignoreErrors mode)
         */
        public function rawSelect( $fields = null, array $conditions = array(),
                                                           array $options = array(), $functionName = null );

diff --git a/includes/db/ORMTable.php b/includes/db/ORMTable.php
index 1292963..7ae10ea 100644 (file)
--- a/includes/db/ORMTable.php
+++ b/includes/db/ORMTable.php
@@ -95,7 +95,8 @@ abstract class ORMTable extends DBAccessBase implements IORMTable {
         */
        public function select( $fields = null, array $conditions = array(),
                                                        array $options = array(), $functionName  = null ) {
-               return new ORMResult( $this, $this->rawSelect( $fields, $conditions, $options, $functionName ) );
+               $res = $this->rawSelect( $fields, $conditions, $options, $functionName );
+               return new ORMResult( $this, $res );
        }
 
        /**
@@ -109,7 +110,8 @@ abstract class ORMTable extends DBAccessBase implements IORMTable {
         * @param array $options
         * @param string|null $functionName
         *
-        * @return array of self
+        * @return array of row objects
+        * @throws DBQueryError if the query failed (even if the database was in ignoreErrors mode).
         */
        public function selectObjects( $fields = null, array $conditions = array(),
                                                                   array $options = array(), $functionName  = null ) {
@@ -130,11 +132,12 @@ abstract class ORMTable extends DBAccessBase implements IORMTable {
         * @since 1.20
         *
         * @param null|string|array $fields
-        * @param array $conditions
-        * @param array $options
-        * @param null|string $functionName
+        * @param array             $conditions
+        * @param array             $options
+        * @param null|string       $functionName
         *
         * @return ResultWrapper
+        * @throws DBQueryError if the quey failed (even if the database was in ignoreErrors mode).
         */
        public function rawSelect( $fields = null, array $conditions = array(),
                                                           array $options = array(), $functionName  = null ) {
@@ -154,7 +157,29 @@ abstract class ORMTable extends DBAccessBase implements IORMTable {
                        $options
                );
 
+               /* @var Exception $error */
+               $error = null;
+
+               if ( $result === false ) {
+                       // Database connection was in "ignoreErrors" mode. We don't like that.
+                       // So, we emulate the DBQueryError that should have been thrown.
+                       $error = new \DBQueryError(
+                               $dbr,
+                               $dbr->lastError(),
+                               $dbr->lastErrno(),
+                               $dbr->lastQuery(),
+                               is_null( $functionName ) ? __METHOD__ : $functionName
+                       );
+               }
+
                $this->releaseConnection( $dbr );
+
+               if ( $error ) {
+                       // Note: construct the error before releasing the connection,
+                       // but throw it after.
+                       throw $error;
+               }
+
                return $result;
        }
 
@@ -227,7 +252,7 @@ abstract class ORMTable extends DBAccessBase implements IORMTable {
 
                $objects = $this->select( $fields, $conditions, $options, $functionName );
 
-               return $objects->isEmpty() ? false : $objects->current();
+               return ( !$objects || $objects->isEmpty() ) ? false : $objects->current();
        }
 
        /**

diff --git a/tests/phpunit/includes/db/ORMTableTest.php b/tests/phpunit/includes/db/ORMTableTest.php
index 2ed3dd3..4cadf31 100644 (file)
--- a/tests/phpunit/includes/db/ORMTableTest.php
+++ b/tests/phpunit/includes/db/ORMTableTest.php
@@ -23,17 +23,21 @@
  * @ingroup Test
  *
  * @group ORM
+ * @group Database
  *
  * @licence GNU GPL v2+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ * @author Daniel Kinzler
  */
-abstract class ORMTableTest extends MediaWikiTestCase {
+class ORMTableTest extends MediaWikiTestCase {
 
        /**
         * @since 1.21
         * @return string
         */
-       protected abstract function getTableClass();
+       protected function getTableClass() {
+               return 'PageORMTableForTesting';
+       }
 
        /**
         * @since 1.21
@@ -62,4 +66,81 @@ abstract class ORMTableTest extends MediaWikiTestCase {
                $this->assertTrue( $class::singleton() === $class::singleton() );
        }
 
+       /**
+        * @since 1.21
+        */
+       public function testIgnoreErrorsOverride() {
+               $table = $this->getTable();
+
+               $db = $table->getReadDbConnection();
+               $db->ignoreErrors( true );
+
+               try {
+                       $table->rawSelect( "this is invalid" );
+                       $this->fail( "An invalid query should trigger a DBQueryError even if ignoreErrors is enabled." );
+               } catch ( DBQueryError $ex ) {
+                       $this->assertTrue( true, "just making phpunit happy" );
+               }
+
+               $db->ignoreErrors( false );
+       }
+
+}
+
+/**
+ * Dummy ORM table for testing, reading Title objects from the page table.
+ *
+ * @since 1.21
+ */
+
+class PageORMTableForTesting extends ORMTable {
+
+       /**
+        * @see ORMTable::getName
+        *
+        * @return string
+        */
+       public function getName() {
+               return 'page';
+       }
+
+       /**
+        * @see ORMTable::getRowClass
+        *
+        * @return string
+        */
+       public function getRowClass() {
+               return 'Title';
+       }
+
+       /**
+        * @see ORMTable::newRow
+        *
+        * @return IORMRow
+        */
+       public function newRow( array $data, $loadDefaults = false ) {
+               return Title::makeTitle( $data['namespace'], $data['title'] );
+       }
+
+       /**
+        * @see ORMTable::getFields
+        *
+        * @return array
+        */
+       public function getFields() {
+               return array(
+                       'id' => 'int',
+                       'namespace' => 'int',
+                       'title' => 'str',
+               );
+       }
+
+       /**
+        * @see ORMTable::getFieldPrefix
+        *
+        * @return string
+        */
+       protected function getFieldPrefix() {
+               return 'page_';
+       }
 }