Merge pull request #4155 from owncloud/mdb2schemareader-booleans

Fix boolean support for xml schema definitions

lib/db/mdb2schemareader.php

@@ -6,35 +6,57 @@
  * See the COPYING-README file.
  */
 
-class OC_DB_MDB2SchemaReader {
-	static protected $DBNAME;
-	static protected $DBTABLEPREFIX;
-	static protected $platform;
+namespace OC\DB;
 
+class MDB2SchemaReader {
 	/**
-	 * @param $file
-	 * @param $platform
+	 * @var string $DBNAME
+	 */
+	protected $DBNAME;
+
+	/**
+	 * @var string $DBTABLEPREFIX
+	 */
+	protected $DBTABLEPREFIX;
+
+	/**
+	 * @var \Doctrine\DBAL\Platforms\AbstractPlatform $platform
+	 */
+	protected $platform;
+
+	/**
+	 * @param \OC\Config $config
+	 * @param \Doctrine\DBAL\Platforms\AbstractPlatform $platform
+	 */
+	public function __construct($config, $platform) {
+		$this->platform = $platform;
+		$this->DBNAME = $config->getValue('dbname', 'owncloud');
+		$this->DBTABLEPREFIX = $config->getValue('dbtableprefix', 'oc_');
+	}
+
+	/**
+	 * @param string $file
 	 * @return \Doctrine\DBAL\Schema\Schema
-	 * @throws DomainException
+	 * @throws \DomainException
 	 */
-	public static function loadSchemaFromFile($file, $platform) {
-		self::$DBNAME  = OC_Config::getValue( "dbname", "owncloud" );
-		self::$DBTABLEPREFIX = OC_Config::getValue( "dbtableprefix", "oc_" );
-		self::$platform = $platform;
+	public function loadSchemaFromFile($file) {
 		$schema = new \Doctrine\DBAL\Schema\Schema();
 		$xml = simplexml_load_file($file);
-		foreach($xml->children() as $child) {
-			switch($child->getName()) {
+		foreach ($xml->children() as $child) {
+			/**
+			 * @var \SimpleXMLElement $child
+			 */
+			switch ($child->getName()) {
 				case 'name':
 				case 'create':
 				case 'overwrite':
 				case 'charset':
 					break;
 				case 'table':
-					self::loadTable($schema, $child);
+					$this->loadTable($schema, $child);
 					break;
 				default:
-					throw new DomainException('Unknown element: '.$child->getName());
+					throw new \DomainException('Unknown element: ' . $child->getName());
 
 			}
 		}
@@ -43,16 +65,20 @@ class OC_DB_MDB2SchemaReader {
 
 	/**
 	 * @param\Doctrine\DBAL\Schema\Schema $schema
-	 * @param $xml
-	 * @throws DomainException
+	 * @param \SimpleXMLElement $xml
+	 * @throws \DomainException
 	 */
-	private static function loadTable($schema, $xml) {
-		foreach($xml->children() as $child) {
-			switch($child->getName()) {
+	private function loadTable($schema, $xml) {
+		$table = null;
+		foreach ($xml->children() as $child) {
+			/**
+			 * @var \SimpleXMLElement $child
+			 */
+			switch ($child->getName()) {
 				case 'name':
 					$name = (string)$child;
-					$name = str_replace( '*dbprefix*', self::$DBTABLEPREFIX, $name );
-					$name = self::$platform->quoteIdentifier($name);
+					$name = str_replace('*dbprefix*', $this->DBTABLEPREFIX, $name);
+					$name = $this->platform->quoteIdentifier($name);
 					$table = $schema->createTable($name);
 					break;
 				case 'create':
@@ -60,10 +86,13 @@ class OC_DB_MDB2SchemaReader {
 				case 'charset':
 					break;
 				case 'declaration':
-					self::loadDeclaration($table, $child);
+					if (is_null($table)) {
+						throw new \DomainException('Table declaration before table name');
+					}
+					$this->loadDeclaration($table, $child);
 					break;
 				default:
-					throw new DomainException('Unknown element: '.$child->getName());
+					throw new \DomainException('Unknown element: ' . $child->getName());
 
 			}
 		}
@@ -71,36 +100,47 @@ class OC_DB_MDB2SchemaReader {
 
 	/**
 	 * @param \Doctrine\DBAL\Schema\Table $table
-	 * @param $xml
-	 * @throws DomainException
+	 * @param \SimpleXMLElement $xml
+	 * @throws \DomainException
 	 */
-	private static function loadDeclaration($table, $xml) {
-		foreach($xml->children() as $child) {
-			switch($child->getName()) {
+	private function loadDeclaration($table, $xml) {
+		foreach ($xml->children() as $child) {
+			/**
+			 * @var \SimpleXMLElement $child
+			 */
+			switch ($child->getName()) {
 				case 'field':
-					self::loadField($table, $child);
+					$this->loadField($table, $child);
 					break;
 				case 'index':
-					self::loadIndex($table, $child);
+					$this->loadIndex($table, $child);
 					break;
 				default:
-					throw new DomainException('Unknown element: '.$child->getName());
+					throw new \DomainException('Unknown element: ' . $child->getName());
 
 			}
 		}
 	}
 
-	private static function loadField($table, $xml) {
+	/**
+	 * @param \Doctrine\DBAL\Schema\Table $table
+	 * @param \SimpleXMLElement $xml
+	 * @throws \DomainException
+	 */
+	private function loadField($table, $xml) {
 		$options = array();
-		foreach($xml->children() as $child) {
-			switch($child->getName()) {
+		foreach ($xml->children() as $child) {
+			/**
+			 * @var \SimpleXMLElement $child
+			 */
+			switch ($child->getName()) {
 				case 'name':
 					$name = (string)$child;
-					$name = self::$platform->quoteIdentifier($name);
+					$name = $this->platform->quoteIdentifier($name);
 					break;
 				case 'type':
 					$type = (string)$child;
-					switch($type) {
+					switch ($type) {
 						case 'text':
 							$type = 'string';
 							break;
@@ -110,8 +150,6 @@ class OC_DB_MDB2SchemaReader {
 						case 'timestamp':
 							$type = 'datetime';
 							break;
-						// TODO
-						return;
 					}
 					break;
 				case 'length':
@@ -119,15 +157,15 @@ class OC_DB_MDB2SchemaReader {
 					$options['length'] = $length;
 					break;
 				case 'unsigned':
-					$unsigned = self::asBool($child);
+					$unsigned = $this->asBool($child);
 					$options['unsigned'] = $unsigned;
 					break;
 				case 'notnull':
-					$notnull = self::asBool($child);
+					$notnull = $this->asBool($child);
 					$options['notnull'] = $notnull;
 					break;
 				case 'autoincrement':
-					$autoincrement = self::asBool($child);
+					$autoincrement = $this->asBool($child);
 					$options['autoincrement'] = $autoincrement;
 					break;
 				case 'default':
@@ -138,8 +176,12 @@ class OC_DB_MDB2SchemaReader {
 					$comment = (string)$child;
 					$options['comment'] = $comment;
 					break;
+				case 'primary':
+					$primary = $this->asBool($child);
+					$options['primary'] = $primary;
+					break;
 				default:
-					throw new DomainException('Unknown element: '.$child->getName());
+					throw new \DomainException('Unknown element: ' . $child->getName());
 
 			}
 		}
@@ -153,22 +195,30 @@ class OC_DB_MDB2SchemaReader {
 				}
 				if ($type == 'integer') {
 					$options['default'] = 0;
+				} elseif ($type == 'boolean') {
+					$options['default'] = false;
 				}
 				if (!empty($options['autoincrement']) && $options['autoincrement']) {
 					unset($options['default']);
 				}
 			}
-			if ($type == 'integer' && isset($options['length'])) {
+			if ($type === 'integer' && isset($options['default'])) {
+				$options['default'] = (int)$options['default'];
+			}
+			if ($type === 'integer' && isset($options['length'])) {
 				$length = $options['length'];
 				if ($length < 4) {
 					$type = 'smallint';
-				}
-				else if ($length > 4) {
+				} else if ($length > 4) {
 					$type = 'bigint';
 				}
 			}
+			if ($type === 'boolean' && isset($options['default'])) {
+				$options['default'] = $this->asBool($options['default']);
+			}
 			if (!empty($options['autoincrement'])
-			    && !empty($options['notnull'])) {
+				&& !empty($options['notnull'])
+			) {
 				$options['primary'] = true;
 			}
 			$table->addColumn($name, $type, $options);
@@ -178,38 +228,49 @@ class OC_DB_MDB2SchemaReader {
 		}
 	}
 
-	private static function loadIndex($table, $xml) {
+	/**
+	 * @param \Doctrine\DBAL\Schema\Table $table
+	 * @param \SimpleXMLElement $xml
+	 * @throws \DomainException
+	 */
+	private function loadIndex($table, $xml) {
 		$name = null;
 		$fields = array();
-		foreach($xml->children() as $child) {
-			switch($child->getName()) {
+		foreach ($xml->children() as $child) {
+			/**
+			 * @var \SimpleXMLElement $child
+			 */
+			switch ($child->getName()) {
 				case 'name':
 					$name = (string)$child;
 					break;
 				case 'primary':
-					$primary = self::asBool($child);
+					$primary = $this->asBool($child);
 					break;
 				case 'unique':
-					$unique = self::asBool($child);
+					$unique = $this->asBool($child);
 					break;
 				case 'field':
-					foreach($child->children() as $field) {
-						switch($field->getName()) {
+					foreach ($child->children() as $field) {
+						/**
+						 * @var \SimpleXMLElement $field
+						 */
+						switch ($field->getName()) {
 							case 'name':
 								$field_name = (string)$field;
-								$field_name = self::$platform->quoteIdentifier($field_name);
+								$field_name = $this->platform->quoteIdentifier($field_name);
 								$fields[] = $field_name;
 								break;
 							case 'sorting':
 								break;
 							default:
-								throw new DomainException('Unknown element: '.$field->getName());
+								throw new \DomainException('Unknown element: ' . $field->getName());
 
 						}
 					}
 					break;
 				default:
-					throw new DomainException('Unknown element: '.$child->getName());
+					throw new \DomainException('Unknown element: ' . $child->getName());
 
 			}
 		}
@@ -217,22 +278,25 @@ class OC_DB_MDB2SchemaReader {
 			if (isset($primary) && $primary) {
 				$table->setPrimaryKey($fields, $name);
 			} else
-			if (isset($unique) && $unique) {
-				$table->addUniqueIndex($fields, $name);
-			} else {
-				$table->addIndex($fields, $name);
-			}
+				if (isset($unique) && $unique) {
+					$table->addUniqueIndex($fields, $name);
+				} else {
+					$table->addIndex($fields, $name);
+				}
 		} else {
-			throw new DomainException('Empty index definition: '.$name.' options:'. print_r($fields, true));
+			throw new \DomainException('Empty index definition: ' . $name . ' options:' . print_r($fields, true));
 		}
 	}
 
-	private static function asBool($xml) {
+	/**
+	 * @param \SimpleXMLElement | string $xml
+	 * @return bool
+	 */
+	private function asBool($xml) {
 		$result = (string)$xml;
 		if ($result == 'true') {
 			$result = true;
-		} else
-		if ($result == 'false') {
+		} elseif ($result == 'false') {
 			$result = false;
 		}
 		return (bool)$result;

lib/db/schema.php

@@ -24,18 +24,21 @@ class OC_DB_Schema {
 
 	/**
 	 * @brief Creates tables from XML file
+	 * @param \Doctrine\DBAL\Connection $conn
 	 * @param string $file file to read structure from
 	 * @return bool
 	 *
 	 * TODO: write more documentation
 	 */
 	public static function createDbFromStructure( $conn, $file ) {
-		$toSchema = OC_DB_MDB2SchemaReader::loadSchemaFromFile($file, $conn->getDatabasePlatform());
+		$schemaReader = new \OC\DB\MDB2SchemaReader(\OC_Config::getObject(), $conn->getDatabasePlatform());
+		$toSchema = $schemaReader->loadSchemaFromFile($file);
 		return self::executeSchemaChange($conn, $toSchema);
 	}
 
 	/**
 	 * @brief update the database scheme
+	 * @param \Doctrine\DBAL\Connection $conn
 	 * @param string $file file to read structure from
 	 * @return bool
 	 */
@@ -43,7 +46,8 @@ class OC_DB_Schema {
 		$sm = $conn->getSchemaManager();
 		$fromSchema = $sm->createSchema();
 
-		$toSchema = OC_DB_MDB2SchemaReader::loadSchemaFromFile($file, $conn->getDatabasePlatform());
+		$schemaReader = new \OC\DB\MDB2SchemaReader(\OC_Config::getObject(), $conn->getDatabasePlatform());
+		$toSchema = $schemaReader->loadSchemaFromFile($file);
 
 		// remove tables we don't know about
 		foreach($fromSchema->getTables() as $table) {
@@ -79,6 +83,7 @@ class OC_DB_Schema {
 
 	/**
 	 * @brief drop a table
+	 * @param \Doctrine\DBAL\Connection $conn
 	 * @param string $tableName the table to drop
 	 */
 	public static function dropTable($conn, $tableName) {
@@ -92,10 +97,12 @@ class OC_DB_Schema {
 
 	/**
 	 * remove all tables defined in a database structure xml file
+	 * @param \Doctrine\DBAL\Connection $conn
 	 * @param string $file the xml file describing the tables
 	 */
 	public static function removeDBStructure($conn, $file) {
-		$fromSchema = OC_DB_MDB2SchemaReader::loadSchemaFromFile($file, $conn->getDatabasePlatform());
+		$schemaReader = new \OC\DB\MDB2SchemaReader(\OC_Config::getObject(), $conn->getDatabasePlatform());
+		$fromSchema = $schemaReader->loadSchemaFromFile($file);
 		$toSchema = clone $fromSchema;
 		foreach($toSchema->getTables() as $table) {
 			$toSchema->dropTable($table->getName());
@@ -107,6 +114,7 @@ class OC_DB_Schema {
 
 	/**
 	 * @brief replaces the ownCloud tables with a new set
+	 * @param \Doctrine\DBAL\Connection $conn
 	 * @param $file string path to the MDB2 xml db export file
 	 */
 	public static function replaceDB( $conn, $file ) {
@@ -126,11 +134,17 @@ class OC_DB_Schema {
 		self::commit();
 	}
 
+	/**
+	 * @param \Doctrine\DBAL\Connection $conn
+	 * @param \Doctrine\DBAL\Schema\Schema $schema
+	 * @return bool
+	 */
 	private static function executeSchemaChange($conn, $schema) {
 		$conn->beginTransaction();
 		foreach($schema->toSql($conn->getDatabasePlatform()) as $sql) {
 			$conn->query($sql);
 		}
 		$conn->commit();
+		return true;
 	}
 }

lib/legacy/config.php

@@ -46,6 +46,10 @@ class OC_Config {
 	 */
 	public static $object;
 
+	public static function getObject() {
+		return self::$object;
+	}
+
 	/**
 	 * @brief Lists all available config keys
 	 * @return array with key names

tests/lib/db/mdb2schemareader.php

+<?php
+
+/**
+ * Copyright (c) 2013 Robin Appelman <icewind@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace Test\DB;
+
+use Doctrine\DBAL\Platforms\MySqlPlatform;
+
+class MDB2SchemaReader extends \PHPUnit_Framework_TestCase {
+	/**
+	 * @var \OC\DB\MDB2SchemaReader $reader
+	 */
+	protected $reader;
+
+	/**
+	 * @return \OC\Config
+	 */
+	protected function getConfig() {
+		$config = $this->getMockBuilder('\OC\Config')
+			->disableOriginalConstructor()
+			->getMock();
+		$config->expects($this->any())
+			->method('getValue')
+			->will($this->returnValueMap(array(
+				array('dbname', 'owncloud', 'testDB'),
+				array('dbtableprefix', 'oc_', 'test_')
+			)));
+		return $config;
+	}
+
+	public function testRead() {
+		$reader = new \OC\DB\MDB2SchemaReader($this->getConfig(), new MySqlPlatform());
+		$schema = $reader->loadSchemaFromFile(__DIR__ . '/testschema.xml');
+		$this->assertCount(1, $schema->getTables());
+
+		$table = $schema->getTable('test_table');
+		$this->assertCount(7, $table->getColumns());
+
+		$this->assertEquals(4, $table->getColumn('integerfield')->getLength());
+		$this->assertTrue($table->getColumn('integerfield')->getAutoincrement());
+		$this->assertNull($table->getColumn('integerfield')->getDefault());
+		$this->assertTrue($table->getColumn('integerfield')->getNotnull());
+		$this->assertInstanceOf('Doctrine\DBAL\Types\IntegerType', $table->getColumn('integerfield')->getType());
+
+		$this->assertSame(10, $table->getColumn('integerfield_default')->getDefault());
+
+		$this->assertEquals(32, $table->getColumn('textfield')->getLength());
+		$this->assertFalse($table->getColumn('textfield')->getAutoincrement());
+		$this->assertSame('foo', $table->getColumn('textfield')->getDefault());
+		$this->assertTrue($table->getColumn('textfield')->getNotnull());
+		$this->assertInstanceOf('Doctrine\DBAL\Types\StringType', $table->getColumn('textfield')->getType());
+
+		$this->assertNull($table->getColumn('clobfield')->getLength());
+		$this->assertFalse($table->getColumn('clobfield')->getAutoincrement());
+		$this->assertSame('', $table->getColumn('clobfield')->getDefault());
+		$this->assertTrue($table->getColumn('clobfield')->getNotnull());
+		$this->assertInstanceOf('Doctrine\DBAL\Types\TextType', $table->getColumn('clobfield')->getType());
+
+		$this->assertNull($table->getColumn('booleanfield')->getLength());
+		$this->assertFalse($table->getColumn('booleanfield')->getAutoincrement());
+		$this->assertFalse($table->getColumn('booleanfield')->getDefault());
+		$this->assertInstanceOf('Doctrine\DBAL\Types\BooleanType', $table->getColumn('booleanfield')->getType());
+
+		$this->assertTrue($table->getColumn('booleanfield_true')->getDefault());
+		$this->assertFalse($table->getColumn('booleanfield_false')->getDefault());
+
+		$this->assertCount(2, $table->getIndexes());
+		$this->assertEquals(array('integerfield'), $table->getIndex('primary')->getUnquotedColumns());
+		$this->assertTrue($table->getIndex('primary')->isPrimary());
+		$this->assertTrue($table->getIndex('primary')->isUnique());
+		$this->assertEquals(array('booleanfield'), $table->getIndex('index_boolean')->getUnquotedColumns());
+		$this->assertFalse($table->getIndex('index_boolean')->isPrimary());
+		$this->assertFalse($table->getIndex('index_boolean')->isUnique());
+	}
+}

tests/lib/db/testschema.xml

+<?xml version="1.0" encoding="utf-8" ?>
+<database>
+
+	<name>*dbname*</name>
+	<create>true</create>
+	<overwrite>false</overwrite>
+
+	<charset>utf8</charset>
+
+	<table>
+
+		<name>*dbprefix*table</name>
+
+		<declaration>
+			<field>
+				<name>integerfield</name>
+				<type>integer</type>
+				<default>0</default>
+				<notnull>true</notnull>
+				<autoincrement>1</autoincrement>
+				<length>4</length>
+			</field>
+			<field>
+				<name>integerfield_default</name>
+				<type>integer</type>
+				<default>10</default>
+				<notnull>true</notnull>
+				<length>4</length>
+			</field>
+			<field>
+				<name>textfield</name>
+				<type>text</type>
+				<default>foo</default>
+				<notnull>true</notnull>
+				<length>32</length>
+			</field>
+			<field>
+				<name>clobfield</name>
+				<type>clob</type>
+				<notnull>true</notnull>
+			</field>
+			<field>
+				<name>booleanfield</name>
+				<type>boolean</type>
+			</field>
+			<field>
+				<name>booleanfield_true</name>
+				<type>boolean</type>
+				<default>true</default>
+			</field>
+			<field>
+				<name>booleanfield_false</name>
+				<type>boolean</type>
+				<default>false</default>
+			</field>
+
+			<index>
+				<name>index_primary</name>
+				<primary>true</primary>
+				<unique>true</unique>
+				<field>
+					<name>integerfield</name>
+					<sorting>ascending</sorting>
+				</field>
+			</index>
+
+			<index>
+				<name>index_boolean</name>
+				<unique>false</unique>
+				<field>
+					<name>booleanfield</name>
+					<sorting>ascending</sorting>
+				</field>
+			</index>
+		</declaration>
+	</table>
+</database>