Try and check migration before applying it

2 files changed, 65 insertions, 60 deletions

diff --git a/lib/private/db/mdb2schemamanager.php b/lib/private/db/mdb2schemamanager.php
index 4208dbd18f4..0eec4d4d317 100644
--- a/lib/private/db/mdb2schemamanager.php
+++ b/lib/private/db/mdb2schemamanager.php
@@ -8,6 +8,10 @@
 
 namespace OC\DB;
 
+use Doctrine\DBAL\Platforms\MySqlPlatform;
+use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+
 class MDB2SchemaManager {
 	/**
 	 * @var \OC\DB\Connection $conn
@@ -31,7 +35,7 @@ class MDB2SchemaManager {
 	 *
 	 * TODO: write more documentation
 	 */
-	public function getDbStructure( $file, $mode = MDB2_SCHEMA_DUMP_STRUCTURE) {
+	public function getDbStructure($file, $mode = MDB2_SCHEMA_DUMP_STRUCTURE) {
 		$sm = $this->conn->getSchemaManager();
 
 		return \OC_DB_MDB2SchemaWriter::saveSchemaToFile($file, $sm);
@@ -44,7 +48,7 @@ class MDB2SchemaManager {
 	 *
 	 * TODO: write more documentation
 	 */
-	public function createDbFromStructure( $file ) {
+	public function createDbFromStructure($file) {
 		$schemaReader = new MDB2SchemaReader(\OC_Config::getObject(), $this->conn->getDatabasePlatform());
 		$toSchema = $schemaReader->loadSchemaFromFile($file);
 		return $this->executeSchemaChange($toSchema);
@@ -53,49 +57,40 @@ class MDB2SchemaManager {
 	/**
 	 * update the database scheme
 	 * @param string $file file to read structure from
+	 * @param bool $generateSql only return the sql needed for the upgrade
 	 * @return string|boolean
 	 */
 	public function updateDbFromStructure($file, $generateSql = false) {
-		$sm = $this->conn->getSchemaManager();
-		$fromSchema = $sm->createSchema();
-
-		$schemaReader = new MDB2SchemaReader(\OC_Config::getObject(), $this->conn->getDatabasePlatform());
-		$toSchema = $schemaReader->loadSchemaFromFile($file);
-
-		// remove tables we don't know about
-		/** @var $table \Doctrine\DBAL\Schema\Table */
-		foreach($fromSchema->getTables() as $table) {
-			if (!$toSchema->hasTable($table->getName())) {
-				$fromSchema->dropTable($table->getName());
-			}
-		}
-		// remove sequences we don't know about
-		foreach($fromSchema->getSequences() as $table) {
-			if (!$toSchema->hasSequence($table->getName())) {
-				$fromSchema->dropSequence($table->getName());
-			}
-		}
-
-		$comparator = new \Doctrine\DBAL\Schema\Comparator();
-		$schemaDiff = $comparator->compare($fromSchema, $toSchema);
 
 		$platform = $this->conn->getDatabasePlatform();
-		foreach($schemaDiff->changedTables as $tableDiff) {
-			$tableDiff->name = $platform->quoteIdentifier($tableDiff->name);
-			foreach($tableDiff->changedColumns as $column) {
-				$column->oldColumnName = $platform->quoteIdentifier($column->oldColumnName);
-			}
+		$schemaReader = new MDB2SchemaReader(\OC_Config::getObject(), $platform);
+		$toSchema = $schemaReader->loadSchemaFromFile($file);
+		if ($platform instanceof SqlitePlatform) {
+			$check = true;
+			$migrator = new SQLiteMigrator($this->conn);
+		} else if ($platform instanceof MySqlPlatform or $platform instanceof PostgreSqlPlatform) {
+			$check = true;
+			$migrator = new Migrator($this->conn);
+		} else {
+			// dont do the upgrade check for oracle
+			$check = false;
+			$migrator = new Migrator($this->conn);
 		}
 
 		if ($generateSql) {
-			return $this->generateChangeScript($schemaDiff);
+			return $migrator->generateChangeScript($toSchema);
+		} else {
+			if ($check) {
+				$migrator->checkMigrate($toSchema);
+			}
+			$migrator->migrate($toSchema);
+			return true;
 		}
-
-		return $this->executeSchemaChange($schemaDiff);
 	}
 
 	/**
 	 * remove all tables defined in a database structure xml file
+	 *
 	 * @param string $file the xml file describing the tables
 	 */
 	public function removeDBStructure($file) {
@@ -103,7 +98,7 @@ class MDB2SchemaManager {
 		$fromSchema = $schemaReader->loadSchemaFromFile($file);
 		$toSchema = clone $fromSchema;
 		/** @var $table \Doctrine\DBAL\Schema\Table */
-		foreach($toSchema->getTables() as $table) {
+		foreach ($toSchema->getTables() as $table) {
 			$toSchema->dropTable($table->getName());
 		}
 		$comparator = new \Doctrine\DBAL\Schema\Comparator();
@@ -117,26 +112,10 @@ class MDB2SchemaManager {
 	 */
 	private function executeSchemaChange($schema) {
 		$this->conn->beginTransaction();
-		foreach($schema->toSql($this->conn->getDatabasePlatform()) as $sql) {
+		foreach ($schema->toSql($this->conn->getDatabasePlatform()) as $sql) {
 			$this->conn->query($sql);
 		}
 		$this->conn->commit();
 		return true;
 	}
-
-	/**
-	 * @param \Doctrine\DBAL\Schema\Schema $schema
-	 * @return string
-	 */
-	public function generateChangeScript($schema) {
-
-		$script = '';
-		$sqls = $schema->toSql($this->conn->getDatabasePlatform());
-		foreach($sqls as $sql) {
-			$script .= $sql . ';';
-			$script .= PHP_EOL;
-		}
-
-		return $script;
-	}
 }
diff --git a/lib/private/db/migrator.php b/lib/private/db/migrator.php
index 5ba16e34311..917f92f64b6 100644
--- a/lib/private/db/migrator.php
+++ b/lib/private/db/migrator.php
@@ -36,6 +36,23 @@ class Migrator {
 	}
 
 	/**
+	 * @param \Doctrine\DBAL\Schema\Schema $targetSchema
+	 * @return string
+	 */
+	public function generateChangeScript(Schema $targetSchema) {
+		$schemaDiff = $this->getDiff($targetSchema, $this->connection);
+
+		$script = '';
+		$sqls = $schemaDiff->toSql($this->connection->getDatabasePlatform());
+		foreach ($sqls as $sql) {
+			$script .= $sql . ';';
+			$script .= PHP_EOL;
+		}
+
+		return $script;
+	}
+
+	/**
 	 * @param Schema $targetSchema
 	 * @throws \OC\DB\MigrationException
 	 */
@@ -109,16 +126,8 @@ class Migrator {
 		return new Table($newName, $table->getColumns(), $newIndexes, array(), 0, $table->getOptions());
 	}
 
-	/**
-	 * @param \Doctrine\DBAL\Schema\Schema $targetSchema
-	 * @param \Doctrine\DBAL\Connection $connection
-	 */
-	protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $connection = null) {
-		if (is_null($connection)) {
-			$connection = $this->connection;
-		}
-
-		$sourceSchema = $this->connection->getSchemaManager()->createSchema();
+	protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) {
+		$sourceSchema = $connection->getSchemaManager()->createSchema();
 
 		// remove tables we don't know about
 		/** @var $table \Doctrine\DBAL\Schema\Table */
@@ -139,8 +148,25 @@ class Migrator {
 
 		foreach ($schemaDiff->changedTables as $tableDiff) {
 			$tableDiff->name = $this->connection->quoteIdentifier($tableDiff->name);
+			foreach ($tableDiff->changedColumns as $column) {
+				$column->oldColumnName = $this->connection->quoteIdentifier($column->oldColumnName);
+			}
+		}
+
+		return $schemaDiff;
+	}
+
+	/**
+	 * @param \Doctrine\DBAL\Schema\Schema $targetSchema
+	 * @param \Doctrine\DBAL\Connection $connection
+	 */
+	protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $connection = null) {
+		if (is_null($connection)) {
+			$connection = $this->connection;
 		}
 
+		$schemaDiff = $this->getDiff($targetSchema, $connection);
+
 		$connection->beginTransaction();
 		foreach ($schemaDiff->toSql($connection->getDatabasePlatform()) as $sql) {
 			$connection->query($sql);