From 7b6fcddbc5314e7401e4f5579853aa6353d462f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 18 Jun 2013 18:24:48 +0200 Subject: [PATCH 1/5] use executeAudited, add table name to assert message, skip schema changing test on oracle --- tests/lib/dbschema.php | 87 +++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 59f203993e..5d52db6a5a 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -7,9 +7,8 @@ */ class Test_DBSchema extends PHPUnit_Framework_TestCase { - protected static $schema_file = 'static://test_db_scheme'; - protected static $schema_file2 = 'static://test_db_scheme2'; - protected $test_prefix; + protected $schema_file = 'static://test_db_scheme'; + protected $schema_file2 = 'static://test_db_scheme2'; protected $table1; protected $table2; @@ -20,19 +19,20 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { $r = '_'.OC_Util::generate_random_bytes('4').'_'; $content = file_get_contents( $dbfile ); $content = str_replace( '*dbprefix*', '*dbprefix*'.$r, $content ); - file_put_contents( self::$schema_file, $content ); + file_put_contents( $this->schema_file, $content ); $content = file_get_contents( $dbfile2 ); $content = str_replace( '*dbprefix*', '*dbprefix*'.$r, $content ); - file_put_contents( self::$schema_file2, $content ); + file_put_contents( $this->schema_file2, $content ); - $this->test_prefix = $r; - $this->table1 = $this->test_prefix.'cntcts_addrsbks'; - $this->table2 = $this->test_prefix.'cntcts_cards'; + $prefix = OC_Config::getValue( "dbtableprefix", "oc_" ); + + $this->table1 = $prefix.$r.'cntcts_addrsbks'; + $this->table2 = $prefix.$r.'cntcts_cards'; } public function tearDown() { - unlink(self::$schema_file); - unlink(self::$schema_file2); + unlink($this->schema_file); + unlink($this->schema_file2); } // everything in one test, they depend on each other @@ -47,13 +47,19 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaCreating() { - OC_DB::createDbFromStructure(self::$schema_file); + OC_DB::createDbFromStructure($this->schema_file); $this->assertTableExist($this->table1); $this->assertTableExist($this->table2); } public function doTestSchemaChanging() { - OC_DB::updateDbFromStructure(self::$schema_file2); + if (OC_Config::getValue( 'dbtype', 'sqlite' ) === 'oci') { + $this->markTestSkipped( + // see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html + 'Oracle does not simply ALTER a VARCHAR into a CLOB.' + ); + } + OC_DB::updateDbFromStructure($this->schema_file2); $this->assertTableExist($this->table2); } @@ -66,67 +72,62 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaRemoving() { - OC_DB::removeDBStructure(self::$schema_file); + OC_DB::removeDBStructure($this->schema_file); $this->assertTableNotExist($this->table1); $this->assertTableNotExist($this->table2); } public function tableExist($table) { - $table = '*PREFIX*' . $table; switch (OC_Config::getValue( 'dbtype', 'sqlite' )) { case 'sqlite': case 'sqlite3': $sql = "SELECT name FROM sqlite_master " - . "WHERE type = 'table' AND name != 'sqlite_sequence' " - . "AND name != 'geometry_columns' AND name != 'spatial_ref_sys' " - . "UNION ALL SELECT name FROM sqlite_temp_master " - . "WHERE type = 'table' AND name = '".$table."'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + . "WHERE type = 'table' AND name = ? " + . "UNION ALL SELECT name FROM sqlite_temp_master " + . "WHERE type = 'table' AND name = ?"; + $result = \OC_DB::executeAudited($sql, array($table, $table)); break; case 'mysql': - $sql = 'SHOW TABLES LIKE "'.$table.'"'; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SHOW TABLES LIKE ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; case 'pgsql': - $sql = "SELECT tablename AS table_name, schemaname AS schema_name " - . "FROM pg_tables WHERE schemaname NOT LIKE 'pg_%' " - . "AND schemaname != 'information_schema' " - . "AND tablename = '".$table."'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SELECT tablename AS table_name, schemaname AS schema_name ' + . 'FROM pg_tables WHERE schemaname NOT LIKE \'pg_%\' ' + . 'AND schemaname != \'information_schema\' ' + . 'AND tablename = ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; case 'oci': - $sql = 'SELECT table_name FROM user_tables WHERE table_name = ?'; + $sql = 'SELECT TABLE_NAME FROM USER_TABLES WHERE TABLE_NAME = ?'; $result = \OC_DB::executeAudited($sql, array($table)); - $exists = (bool)$result->fetchOne(); //oracle uses MDB2 and returns null break; case 'mssql': - $sql = "SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = '{$table}'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; } - return $exists; + + $name = $result->fetchOne(); //FIXME checking with '$result->numRows() === 1' does not seem to work? + OC_DB::raiseExceptionOnError($name); + if ($name === $table) { + return true; + } else { + return false; + } } public function assertTableExist($table) { - $this->assertTrue($this->tableExist($table)); + $this->assertTrue($this->tableExist($table), 'Table ' . $table . ' does not exist'); } public function assertTableNotExist($table) { $type=OC_Config::getValue( "dbtype", "sqlite" ); if( $type == 'sqlite' || $type == 'sqlite3' ) { // sqlite removes the tables after closing the DB - } - else { - $this->assertFalse($this->tableExist($table)); + } else { + $this->assertFalse($this->tableExist($table), 'Table ' . $table . ' exists.'); } } } From e62eb2e8d1937f1708ce7efdd6ab8a31e12c6f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 12:31:23 +0300 Subject: [PATCH 2/5] correctly handle error results of PDO and MDB2 backends --- lib/db.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/db.php b/lib/db.php index 4d6788f2bd..5e624bf30b 100644 --- a/lib/db.php +++ b/lib/db.php @@ -962,11 +962,14 @@ class OC_DB { * @return bool */ public static function isError($result) { - if(self::$backend==self::BACKEND_PDO and $result === false) { + //PDO returns false on error (and throws an exception) + if (self::$backend===self::BACKEND_PDO and $result === false) { return true; - }elseif(self::$backend==self::BACKEND_MDB2 and PEAR::isError($result)) { + } else + //MDB2 returns an MDB2_Error object + if (self::$backend===self::BACKEND_MDB2 and PEAR::isError($result)) { return true; - }else{ + } else { return false; } } From 4bbdd67a22fd5ac0efc6b8c9dc3e024a0af45c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 16:58:49 +0200 Subject: [PATCH 3/5] remove wrong check here --- tests/lib/dbschema.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 5d52db6a5a..103909e2e2 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -110,7 +110,6 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } $name = $result->fetchOne(); //FIXME checking with '$result->numRows() === 1' does not seem to work? - OC_DB::raiseExceptionOnError($name); if ($name === $table) { return true; } else { From 66e1eaac9379e16bb0c9e60b2794b03bda3b9eec Mon Sep 17 00:00:00 2001 From: Thomas Mueller Date: Thu, 27 Jun 2013 16:48:09 +0200 Subject: [PATCH 4/5] isError should detect a PEAR_Error even if the backend is PDO. This can happen on errors during schema migration - which is always done with MDB2 --- lib/db.php | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/db.php b/lib/db.php index 5e624bf30b..515563b78e 100644 --- a/lib/db.php +++ b/lib/db.php @@ -962,21 +962,21 @@ class OC_DB { * @return bool */ public static function isError($result) { + //MDB2 returns an MDB2_Error object + if (class_exists('PEAR') === true && PEAR::isError($result)) { + return true; + } //PDO returns false on error (and throws an exception) if (self::$backend===self::BACKEND_PDO and $result === false) { return true; - } else - //MDB2 returns an MDB2_Error object - if (self::$backend===self::BACKEND_MDB2 and PEAR::isError($result)) { - return true; - } else { - return false; } + + return false; } /** * check if a result is an error and throws an exception, works with MDB2 and PDOException * @param mixed $result - * @param string message + * @param string $message * @return void * @throws DatabaseException */ @@ -992,12 +992,15 @@ class OC_DB { } public static function getErrorCode($error) { - if ( self::$backend==self::BACKEND_MDB2 and PEAR::isError($error) ) { - $code = $error->getCode(); - } elseif ( self::$backend==self::BACKEND_PDO and self::$PDO ) { - $code = self::$PDO->errorCode(); + if ( class_exists('PEAR') === true && PEAR::isError($error) ) { + /** @var $error PEAR_Error */ + return $error->getCode(); } - return $code; + if ( self::$backend==self::BACKEND_PDO and self::$PDO ) { + return self::$PDO->errorCode(); + } + + return -1; } /** * returns the error code and message as a string for logging @@ -1006,23 +1009,24 @@ class OC_DB { * @return string */ public static function getErrorMessage($error) { - if ( self::$backend==self::BACKEND_MDB2 and PEAR::isError($error) ) { + if ( class_exists('PEAR') === true && PEAR::isError($error) ) { $msg = $error->getCode() . ': ' . $error->getMessage(); $msg .= ' (' . $error->getDebugInfo() . ')'; - } elseif (self::$backend==self::BACKEND_PDO and self::$PDO) { + + return $msg; + } + if (self::$backend==self::BACKEND_PDO and self::$PDO) { $msg = self::$PDO->errorCode() . ': '; $errorInfo = self::$PDO->errorInfo(); if (is_array($errorInfo)) { $msg .= 'SQLSTATE = '.$errorInfo[0] . ', '; $msg .= 'Driver Code = '.$errorInfo[1] . ', '; $msg .= 'Driver Message = '.$errorInfo[2]; - }else{ - $msg = ''; } - }else{ - $msg = ''; + return $msg; } - return $msg; + + return ''; } /** From c80e76720f4e65a5f7af31f67d70ec66019bcb68 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Jun 2013 16:07:22 +0200 Subject: [PATCH 5/5] Going from text to clob is not something we do. Also Oracle DB has problems with this, see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html --- tests/data/db_structure2.xml | 3 ++- tests/lib/dbschema.php | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/data/db_structure2.xml b/tests/data/db_structure2.xml index fc6fe0bba7..6f12f81f47 100644 --- a/tests/data/db_structure2.xml +++ b/tests/data/db_structure2.xml @@ -49,8 +49,9 @@ description - clob + text false + 1024 diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 103909e2e2..c2e55eabf4 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -53,12 +53,6 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaChanging() { - if (OC_Config::getValue( 'dbtype', 'sqlite' ) === 'oci') { - $this->markTestSkipped( - // see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html - 'Oracle does not simply ALTER a VARCHAR into a CLOB.' - ); - } OC_DB::updateDbFromStructure($this->schema_file2); $this->assertTableExist($this->table2); }