MDL-14905 Completed all core functional tests for database_manager (ddl). Found some API design issues and commented on them using @TODO comments.

lib/ddl/database_manager.php

@@ -364,7 +364,7 @@ class database_manager {
     }
 
     /**
-     * This function will all tables found in XMLDB file from db
+     * This function will delete all tables found in XMLDB file from db
      *
      * @param $file full path to the XML file to be used
      * @param $feedback
@@ -411,6 +411,10 @@ class database_manager {
      * @param boolean continue to specify if must continue on error (true) or stop (false)
      * @param boolean feedback to specify to show status info (true) or not (false)
      * @return boolean true on success, false on error
+     * @TODO I don't think returning TRUE when trying to drop a non-existing table is a good idea.
+     *       the point is, the method name is drop_table, and if it doesn't drop a table,
+     *       it should be obvious to the code calling this method, and not rely on the visual
+     *       feedback of debugging(). Exception handling may solve this.
      */
     public function drop_table($xmldb_table, $continue=true, $feedback=true) {
         if (!($xmldb_table instanceof xmldb_table)) {
@@ -483,6 +487,10 @@ class database_manager {
      * @param boolean continue to specify if must continue on error (true) or stop (false)
      * @param boolean feedback to specify to show status info (true) or not (false)
      * @return boolean true on success, false on error
+     * @TODO I don't think returning TRUE when trying to create an existing table is a good idea.
+     *       the point is, the method name is create_table, and if it doesn't create a table,
+     *       it should be obvious to the code calling this method, and not rely on the visual
+     *       feedback of debugging(). Exception handling may solve this.
      */
     public function create_table($xmldb_table, $continue=true, $feedback=true) {
         if (!($xmldb_table instanceof xmldb_table)) {
@@ -515,6 +523,10 @@ class database_manager {
      * NOTE: The return value is the tablename - some DBs (MSSQL at least) use special
      * names for temp tables.
      *
+     * @TODO There is no way to know, from the return value alone, whether a table was actually created
+     *       or not: if an existing table is given as param, its name will be returned, but no DB action
+     *       will have occurred. This should be remedied using an Exception
+     *
      * @param xmldb_table table object (full specs are required)
      * @param boolean continue to specify if must continue on error (true) or stop (false)
      * @param boolean feedback to specify to show status info (true) or not (false)

lib/ddl/simpletest/fixtures/invalid.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<XMLDB PATH="lib/ddl/simpletest/fixtures" VERSION="20080521" COMMENT="This is an invalid xml file used for testing.">
+  <TABLES>
+    <TABLE NAME="anothertest" COMMENT="Just a test table">
+      <FIELDS>
+        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="true" ENUM="false" NEXT="incorrect_next_field"/> <!-- invalid because of NEXT value -->
+        <FIELD NAME="course" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="id" NEXT="name"/>
+        <FIELD NAME="name" TYPE="char" LENGTH="30" NOTNULL="false" SEQUENCE="false" DEFAULT="Moodle" ENUM="false" PREVIOUS="course" NEXT="secondname"/>
+        <FIELD NAME="secondname" TYPE="char" LENGTH="30" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="name" NEXT="intro"/>
+        <FIELD NAME="intro" TYPE="text" LENGTH="medium" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="secondname" NEXT="avatar"/>
+        <FIELD NAME="avatar" TYPE="binary" LENGTH="medium" NOTNULL="false" UNSIGNED="false" SEQUENCE="false" ENUM="false" PREVIOUS="intro" NEXT="grade"/>
+        <FIELD NAME="grade" TYPE="number" LENGTH="20" DECIMALS="10" NOTNULL="false" UNSIGNED="false" SEQUENCE="false" ENUM="false" PREVIOUS="avatar"/>
+      </FIELDS>
+      <KEYS>
+        <KEY NAME="primary" TYPE="primary" FIELDS="id" />
+      </KEYS>
+    </TABLE>
+  </TABLES>
+</XMLDB>

lib/ddl/simpletest/fixtures/xmldb_table.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<XMLDB PATH="lib/ddl/simpletest/fixtures" VERSION="20080521" COMMENT="XMLDB file DLL unit tests">
+  <TABLES>
+    <TABLE NAME="anothertest" COMMENT="Just a test table">
+      <FIELDS>
+        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="true" ENUM="false" NEXT="course"/>
+        <FIELD NAME="course" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="id" NEXT="name"/>
+        <FIELD NAME="name" TYPE="char" LENGTH="30" NOTNULL="false" SEQUENCE="false" DEFAULT="Moodle" ENUM="false" PREVIOUS="course" NEXT="secondname"/>
+        <FIELD NAME="secondname" TYPE="char" LENGTH="30" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="name" NEXT="intro"/>
+        <FIELD NAME="intro" TYPE="text" LENGTH="medium" NOTNULL="true" SEQUENCE="false" ENUM="false" PREVIOUS="secondname" NEXT="avatar"/>
+        <FIELD NAME="avatar" TYPE="binary" LENGTH="medium" NOTNULL="false" UNSIGNED="false" SEQUENCE="false" ENUM="false" PREVIOUS="intro" NEXT="grade"/>
+        <FIELD NAME="grade" TYPE="number" LENGTH="20" DECIMALS="10" NOTNULL="false" UNSIGNED="false" SEQUENCE="false" ENUM="false" PREVIOUS="avatar"/>
+      </FIELDS>
+      <KEYS>
+        <KEY NAME="primary" TYPE="primary" FIELDS="id" />
+      </KEYS>
+    </TABLE>
+  </TABLES>
+</XMLDB>

lib/ddl/simpletest/testddllib.php

@@ -94,12 +94,29 @@ class ddllib_test extends UnitTestCase {
         $this->assertTrue($this->dbmanager->create_table($table));
         $this->assertTrue($this->dbmanager->table_exists("other_test_table"));
         $this->dbmanager->drop_table($table);
+
+        // Give existing table as argument
+        $table = $this->tables[1];
+        $this->assertFalse($this->dbmanager->create_table($table));
+
+        // Give a wrong table param
+        $table = 'string';
+        $this->assertFalse($this->dbmanager->create_table($table));
+
     }
 
     public function testDropTable() {
         $table = $this->tables[0];
         $this->assertTrue($this->dbmanager->drop_table($table, true, false));
         $this->assertFalse($this->dbmanager->table_exists("testtable"));
+
+        // Try dropping non-existent table
+        $table = new xmldb_table('nonexistenttable');
+        $this->assertFalse($this->dbmanager->drop_table($table, true, false));
+
+        // Give a wrong table param
+        $table = 'string';
+        $this->assertFalse($this->dbmanager->drop_table($table, true, false));
     }
 
     public function testAddEnumField() {
@@ -429,16 +446,58 @@ class ddllib_test extends UnitTestCase {
     }
 
     public function testDeleteTablesFromXmldbFile() {
+        global $CFG;
+        $this->assertTrue($this->dbmanager->table_exists('anothertest'));
 
+        // feed nonexistent file
+        $this->assertFalse($this->dbmanager->delete_tables_from_xmldb_file('fpsoiudfposui', false));
+
+        // Real file but invalid xml file
+        $this->assertFalse($this->dbmanager->delete_tables_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/invalid.xml', false));
+
+        // Check that the table has not been deleted from DB
+        $this->assertTrue($this->dbmanager->table_exists('anothertest'));
+
+        // Real and valid xml file
+        $this->assertTrue($this->dbmanager->delete_tables_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/xmldb_table.xml', false));
+
+        // Check that the table has been deleted from DB
+        $this->assertFalse($this->dbmanager->table_exists('anothertest'));
     }
 
     public function testInstallFromXmldbFile() {
+        global $CFG;
+        // First delete existing test table to make room for new one
+        $table = $this->tables[1];
+        $this->dbmanager->drop_table($table);
+        $this->assertFalse($this->dbmanager->table_exists('anothertest'));
 
+        // feed nonexistent file
+        $this->assertFalse($this->dbmanager->install_from_xmldb_file('fpsoiudfposui', false));
+
+        // Real but invalid xml file
+        $this->assertFalse($this->dbmanager->install_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/invalid.xml', false));
+
+        // Check that the table has not yet been created in DB
+        $this->assertFalse($this->dbmanager->table_exists('anothertest'));
+
+        // Real and valid xml file
+        $this->assertTrue($this->dbmanager->install_from_xmldb_file($CFG->libdir . '/ddl/simpletest/fixtures/xmldb_table.xml', false));
+        $this->assertTrue($this->dbmanager->table_exists('anothertest'));
     }
 
     public function testCreateTempTable() {
+        // Feed incorrect table param
+        $this->assertFalse($this->dbmanager->create_temp_table('anothertest'));
+
+        // Correct table but with existing name
+        $table = $this->tables[0];
+        $this->assertEqual('testtable', $this->dbmanager->create_temp_table($table));
+
+        // New table
+        $this->dbmanager->drop_table($this->tables[0]);
+        $this->assertEqual('testtable', $this->dbmanager->create_temp_table($table));
 
     }
 }
-
 ?>