From 298c8b4786344faa7c8bcaf8b61ce5883f0af44a Mon Sep 17 00:00:00 2001 From: jeroendedauw Date: Fri, 25 May 2012 23:09:29 +0200 Subject: [PATCH] Added base class for testing ORMRow deriving classes and added a mock implementation in order to test the abstract parent class itself Patchset 2: attempt to fix sql error when using sqlite Patchset 3: for great justice Patchset 4: sqlite, not postgres... /facepalm Patchset 5: joy, more sql divergence... Patchset 7: handle inconsistencies in MWs database abstraction layer Change-Id: I1948c4ad815008321801c93584eb249c1f597560 --- includes/db/ORMRow.php | 15 +- includes/db/ORMTable.php | 4 +- tests/phpunit/includes/db/ORMRowTest.php | 134 +++++++++++++++ tests/phpunit/includes/db/TestORMRowTest.php | 172 +++++++++++++++++++ 4 files changed, 319 insertions(+), 6 deletions(-) create mode 100644 tests/phpunit/includes/db/ORMRowTest.php create mode 100644 tests/phpunit/includes/db/TestORMRowTest.php diff --git a/includes/db/ORMRow.php b/includes/db/ORMRow.php index bebddd60b8..6f19fe1302 100644 --- a/includes/db/ORMRow.php +++ b/includes/db/ORMRow.php @@ -353,7 +353,8 @@ abstract class ORMRow implements IORMRow { is_null( $functionName ) ? __METHOD__ : $functionName ); - return $success; + // DatabaseBase::update does not always return true for success as documented... + return $success !== false; } /** @@ -381,18 +382,21 @@ abstract class ORMRow implements IORMRow { protected function insert( $functionName = null, array $options = null ) { $dbw = wfGetDB( DB_MASTER ); - $result = $dbw->insert( + $success = $dbw->insert( $this->table->getName(), $this->getWriteValues(), is_null( $functionName ) ? __METHOD__ : $functionName, is_null( $options ) ? array( 'IGNORE' ) : $options ); - if ( $result ) { + // DatabaseBase::insert does not always return true for success as documented... + $success = $success !== false; + + if ( $success ) { $this->setField( 'id', $dbw->insertId() ); } - return $result; + return $success; } /** @@ -407,6 +411,9 @@ abstract class ORMRow implements IORMRow { $success = $this->table->delete( array( 'id' => $this->getId() ) ); + // DatabaseBase::delete does not always return true for success as documented... + $success = $success !== false; + if ( $success ) { $this->onRemoved(); } diff --git a/includes/db/ORMTable.php b/includes/db/ORMTable.php index cc84d65d3b..b6e2d52656 100644 --- a/includes/db/ORMTable.php +++ b/includes/db/ORMTable.php @@ -330,7 +330,7 @@ abstract class ORMTable implements IORMTable { $this->getName(), $this->getPrefixedValues( $conditions ), $functionName - ); + ) !== false; // DatabaseBase::delete does not always return true for success as documented... } /** @@ -437,7 +437,7 @@ abstract class ORMTable implements IORMTable { $this->getPrefixedValues( $values ), $this->getPrefixedValues( $conditions ), __METHOD__ - ); + ) !== false; // DatabaseBase::update does not always return true for success as documented... } /** diff --git a/tests/phpunit/includes/db/ORMRowTest.php b/tests/phpunit/includes/db/ORMRowTest.php new file mode 100644 index 0000000000..9a275be381 --- /dev/null +++ b/tests/phpunit/includes/db/ORMRowTest.php @@ -0,0 +1,134 @@ + + */ +abstract class ORMRowTest extends \MediaWikiTestCase { + + /** + * @since 1.20 + * @return string + */ + protected abstract function getRowClass(); + + /** + * @since 1.20 + * @return IORMTable + */ + protected abstract function getTableInstance(); + + /** + * @since 1.20 + * @return array + */ + public abstract function constructorTestProvider(); + + /** + * @since 1.20 + * @param IORMRow $row + * @param array $data + */ + protected function verifyFields( IORMRow $row, array $data ) { + foreach ( array_keys( $data ) as $fieldName ) { + $this->assertEquals( $data[$fieldName], $row->getField( $fieldName ) ); + } + } + + /** + * @since 1.20 + * @param array $data + * @param boolean $loadDefaults + * @return IORMRow + */ + protected function getRowInstance( array $data, $loadDefaults ) { + $class = $this->getRowClass(); + return new $class( $this->getTableInstance(), $data, $loadDefaults ); + } + + /** + * @dataProvider constructorTestProvider + */ + public function testConstructor( array $data, $loadDefaults ) { + $this->verifyFields( $this->getRowInstance( $data, $loadDefaults ), $data ); + } + + /** + * @dataProvider constructorTestProvider + */ + public function testSave( array $data, $loadDefaults ) { + $item = $this->getRowInstance( $data, $loadDefaults ); + + $this->assertTrue( $item->save() ); + + $this->assertTrue( $item->hasIdField() ); + $this->assertTrue( is_integer( $item->getId() ) ); + + $id = $item->getId(); + + $this->assertTrue( $item->save() ); + + $this->assertEquals( $id, $item->getId() ); + + $this->verifyFields( $item, $data ); + } + + /** + * @dataProvider constructorTestProvider + */ + public function testRemove( array $data, $loadDefaults ) { + $item = $this->getRowInstance( $data, $loadDefaults ); + + $this->assertTrue( $item->save() ); + + $this->assertTrue( $item->remove() ); + + $this->assertFalse( $item->hasIdField() ); + + $this->assertTrue( $item->save() ); + + $this->verifyFields( $item, $data ); + + $this->assertTrue( $item->remove() ); + + $this->assertFalse( $item->hasIdField() ); + + $this->verifyFields( $item, $data ); + } + + // TODO: test all of the methods! + +} \ No newline at end of file diff --git a/tests/phpunit/includes/db/TestORMRowTest.php b/tests/phpunit/includes/db/TestORMRowTest.php new file mode 100644 index 0000000000..49c2161a5a --- /dev/null +++ b/tests/phpunit/includes/db/TestORMRowTest.php @@ -0,0 +1,172 @@ + + */ +class TestORMRowTest extends ORMRowTest { + + /** + * @since 1.20 + * @return string + */ + protected function getRowClass() { + return 'TestORMRow'; + } + + /** + * @since 1.20 + * @return IORMTable + */ + protected function getTableInstance() { + return TestORMtable::singleton(); + } + + public function setUp() { + parent::setUp(); + + $dbw = wfGetDB( DB_MASTER ); + + $isSqlite = $GLOBALS['wgDBtype'] === 'sqlite'; + + $idField = $isSqlite ? 'INTEGER' : 'INT unsigned'; + $primaryKey = $isSqlite ? 'PRIMARY KEY AUTOINCREMENT' : 'auto_increment PRIMARY KEY'; + + $dbw->safeQuery( + 'CREATE TABLE IF NOT EXISTS ' . $dbw->tableName( 'orm_test' ) . '( + test_id ' . $idField . ' NOT NULL ' . $primaryKey . ', + test_name VARCHAR(255) NOT NULL, + test_age TINYINT unsigned NOT NULL, + test_height FLOAT NOT NULL, + test_awesome TINYINT unsigned NOT NULL, + test_stuff BLOB NOT NULL, + test_moarstuff BLOB NOT NULL, + test_time varbinary(14) NOT NULL + );' + ); + } + + public function constructorTestProvider() { + return array( + array( + array( + 'name' => 'Foobar', + 'age' => 42, + 'height' => 9000.1, + 'awesome' => true, + 'stuff' => array( 13, 11, 7, 5, 3, 2 ), + 'moarstuff' => (object)array( 'foo' => 'bar', 'bar' => array( 4, 2 ), 'baz' => true ) + ), + true + ), + ); + } + +} + +class TestORMRow extends ORMRow {} + +class TestORMTable extends ORMTable { + + /** + * Returns the name of the database table objects of this type are stored in. + * + * @since 1.20 + * + * @return string + */ + public function getName() { + return 'orm_test'; + } + + /** + * Returns the name of a IORMRow implementing class that + * represents single rows in this table. + * + * @since 1.20 + * + * @return string + */ + public function getRowClass() { + return 'TestORMRow'; + } + + /** + * Returns an array with the fields and their types this object contains. + * This corresponds directly to the fields in the database, without prefix. + * + * field name => type + * + * Allowed types: + * * id + * * str + * * int + * * float + * * bool + * * array + * * blob + * + * @since 1.20 + * + * @return array + */ + public function getFields() { + return array( + 'id' => 'id', + 'name' => 'str', + 'age' => 'int', + 'height' => 'float', + 'awesome' => 'bool', + 'stuff' => 'array', + 'moarstuff' => 'blob', + 'time' => 'int', // TS_MW + ); + } + + /** + * Gets the db field prefix. + * + * @since 1.20 + * + * @return string + */ + protected function getFieldPrefix() { + return 'test_'; + } + + +} -- 2.20.1