From: daniel <daniel.kinzler@wikimedia.de>
Date: Wed, 5 Dec 2012 16:10:31 +0000 (+0100)
Subject: (minor) Make ORMTable handle failed queries nicely.
X-Git-Tag: 1.31.0-rc.0~21117^2
X-Git-Url: https://git.cyclocoop.org/%7B%7B%20url_for%28%27admin_user_edit%27%2C%20iduser=user.userid%29%20%7D%7D?a=commitdiff_plain;h=4f15b61efebd3bde3d26dae321002aa6196a3580;p=lhc%2Fweb%2Fwiklou.git

(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 67dbdfe6ac..e46dbc0078 100644
--- 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 1292963087..7ae10ea13e 100644
--- 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 2ed3dd3a13..4cadf31cf2 100644
--- 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_';
+	}
 }