Merge pull request #7021 from nextcloud/fix-oracle-indexes

Fix oracle indexes

5 files changed, 120 insertions, 71 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 709ac7c50ed..5c55c83b054 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -529,7 +529,6 @@ return array(
     'OC\\DB\\Migrator' => $baseDir . '/lib/private/DB/Migrator.php',
     'OC\\DB\\MySQLMigrator' => $baseDir . '/lib/private/DB/MySQLMigrator.php',
     'OC\\DB\\MySqlTools' => $baseDir . '/lib/private/DB/MySqlTools.php',
-    'OC\\DB\\NoCheckMigrator' => $baseDir . '/lib/private/DB/NoCheckMigrator.php',
     'OC\\DB\\OCPostgreSqlPlatform' => $baseDir . '/lib/private/DB/OCPostgreSqlPlatform.php',
     'OC\\DB\\OCSqlitePlatform' => $baseDir . '/lib/private/DB/OCSqlitePlatform.php',
     'OC\\DB\\OracleConnection' => $baseDir . '/lib/private/DB/OracleConnection.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 7ba734aac80..23fd2d4280b 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -559,7 +559,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\DB\\Migrator' => __DIR__ . '/../../..' . '/lib/private/DB/Migrator.php',
         'OC\\DB\\MySQLMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/MySQLMigrator.php',
         'OC\\DB\\MySqlTools' => __DIR__ . '/../../..' . '/lib/private/DB/MySqlTools.php',
-        'OC\\DB\\NoCheckMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/NoCheckMigrator.php',
         'OC\\DB\\OCPostgreSqlPlatform' => __DIR__ . '/../../..' . '/lib/private/DB/OCPostgreSqlPlatform.php',
         'OC\\DB\\OCSqlitePlatform' => __DIR__ . '/../../..' . '/lib/private/DB/OCSqlitePlatform.php',
         'OC\\DB\\OracleConnection' => __DIR__ . '/../../..' . '/lib/private/DB/OracleConnection.php',
diff --git a/lib/private/DB/MDB2SchemaManager.php b/lib/private/DB/MDB2SchemaManager.php
index 89b0d153212..ad3f93a2643 100644
--- a/lib/private/DB/MDB2SchemaManager.php
+++ b/lib/private/DB/MDB2SchemaManager.php
@@ -89,7 +89,7 @@ class MDB2SchemaManager {
 		} else if ($platform instanceof PostgreSqlPlatform) {
 			return new PostgreSqlMigrator($this->conn, $random, $config, $dispatcher);
 		} else {
-			return new NoCheckMigrator($this->conn, $random, $config, $dispatcher);
+			return new Migrator($this->conn, $random, $config, $dispatcher);
 		}
 	}
 
diff --git a/lib/private/DB/NoCheckMigrator.php b/lib/private/DB/NoCheckMigrator.php
deleted file mode 100644
index 723653511b9..00000000000
--- a/lib/private/DB/NoCheckMigrator.php
+++ /dev/null
@@ -1,39 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Robin Appelman <robin@icewind.nl>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-namespace OC\DB;
-
-use Doctrine\DBAL\Schema\Schema;
-
-/**
- * migrator for database platforms that don't support the upgrade check
- *
- * @package OC\DB
- */
-class NoCheckMigrator extends Migrator {
-	/**
-	 * @param \Doctrine\DBAL\Schema\Schema $targetSchema
-	 * @throws \OC\DB\MigrationException
-	 */
-	public function checkMigrate(Schema $targetSchema) {}
-}
diff --git a/lib/private/DB/OracleMigrator.php b/lib/private/DB/OracleMigrator.php
index 2735529b5e2..f5e06b50d99 100644
--- a/lib/private/DB/OracleMigrator.php
+++ b/lib/private/DB/OracleMigrator.php
@@ -30,8 +30,79 @@ use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Schema\ForeignKeyConstraint;
+
+class OracleMigrator extends Migrator {
+
+	/**
+	 * Quote a column's name but changing the name requires recreating
+	 * the column instance and copying over all properties.
+	 *
+	 * @param Column $column old column
+	 * @return Column new column instance with new name
+	 */
+	protected function quoteColumn(Column $column) {
+		$newColumn = new Column(
+			$this->connection->quoteIdentifier($column->getName()),
+			$column->getType()
+		);
+		$newColumn->setAutoincrement($column->getAutoincrement());
+		$newColumn->setColumnDefinition($column->getColumnDefinition());
+		$newColumn->setComment($column->getComment());
+		$newColumn->setDefault($column->getDefault());
+		$newColumn->setFixed($column->getFixed());
+		$newColumn->setLength($column->getLength());
+		$newColumn->setNotnull($column->getNotnull());
+		$newColumn->setPrecision($column->getPrecision());
+		$newColumn->setScale($column->getScale());
+		$newColumn->setUnsigned($column->getUnsigned());
+		$newColumn->setPlatformOptions($column->getPlatformOptions());
+		$newColumn->setCustomSchemaOptions($column->getPlatformOptions());
+		return $newColumn;
+	}
+
+	/**
+	 * Quote an index's name but changing the name requires recreating
+	 * the index instance and copying over all properties.
+	 *
+	 * @param Index $index old index
+	 * @return Index new index instance with new name
+	 */
+	protected function quoteIndex($index) {
+		return new Index(
+		//TODO migrate existing uppercase indexes, then $this->connection->quoteIdentifier($index->getName()),
+			$index->getName(),
+			array_map(function($columnName) {
+				return $this->connection->quoteIdentifier($columnName);
+			}, $index->getColumns()),
+			$index->isUnique(),
+			$index->isPrimary(),
+			$index->getFlags(),
+			$index->getOptions()
+		);
+	}
+
+	/**
+	 * Quote an ForeignKeyConstraint's name but changing the name requires recreating
+	 * the ForeignKeyConstraint instance and copying over all properties.
+	 *
+	 * @param ForeignKeyConstraint $fkc old fkc
+	 * @return ForeignKeyConstraint new fkc instance with new name
+	 */
+	protected function quoteForeignKeyConstraint($fkc) {
+		return new ForeignKeyConstraint(
+			array_map(function($columnName) {
+				return $this->connection->quoteIdentifier($columnName);
+			}, $fkc->getLocalColumns()),
+			$this->connection->quoteIdentifier($fkc->getForeignTableName()),
+			array_map(function($columnName) {
+				return $this->connection->quoteIdentifier($columnName);
+			}, $fkc->getForeignColumns()),
+			$fkc->getName(),
+			$fkc->getOptions()
+		);
+	}
 
-class OracleMigrator extends NoCheckMigrator {
 	/**
 	 * @param Schema $targetSchema
 	 * @param \Doctrine\DBAL\Connection $connection
@@ -46,37 +117,14 @@ class OracleMigrator extends NoCheckMigrator {
 			return new Table(
 				$this->connection->quoteIdentifier($table->getName()),
 				array_map(function(Column $column) {
-					$newColumn = new Column(
-						$this->connection->quoteIdentifier($column->getName()),
-						$column->getType()
-					);
-					$newColumn->setAutoincrement($column->getAutoincrement());
-					$newColumn->setColumnDefinition($column->getColumnDefinition());
-					$newColumn->setComment($column->getComment());
-					$newColumn->setDefault($column->getDefault());
-					$newColumn->setFixed($column->getFixed());
-					$newColumn->setLength($column->getLength());
-					$newColumn->setNotnull($column->getNotnull());
-					$newColumn->setPrecision($column->getPrecision());
-					$newColumn->setScale($column->getScale());
-					$newColumn->setUnsigned($column->getUnsigned());
-					$newColumn->setPlatformOptions($column->getPlatformOptions());
-					$newColumn->setCustomSchemaOptions($column->getPlatformOptions());
-					return $newColumn;
+					return $this->quoteColumn($column);
 				}, $table->getColumns()),
 				array_map(function(Index $index) {
-					return new Index(
-						$this->connection->quoteIdentifier($index->getName()),
-						array_map(function($columnName) {
-							return $this->connection->quoteIdentifier($columnName);
-						}, $index->getColumns()),
-						$index->isUnique(),
-						$index->isPrimary(),
-						$index->getFlags(),
-						$index->getOptions()
-					);
+					return $this->quoteIndex($index);
 				}, $table->getIndexes()),
-				$table->getForeignKeys(),
+				array_map(function(ForeignKeyConstraint $fck) {
+					return $this->quoteForeignKeyConstraint($fck);
+				}, $table->getForeignKeys()),
 				0,
 				$table->getOptions()
 			);
@@ -95,14 +143,56 @@ class OracleMigrator extends NoCheckMigrator {
 
 		foreach ($schemaDiff->changedTables as $tableDiff) {
 			$tableDiff->name = $this->connection->quoteIdentifier($tableDiff->name);
+
+			$tableDiff->addedColumns = array_map(function(Column $column) {
+				return $this->quoteColumn($column);
+			}, $tableDiff->addedColumns);
+
 			foreach ($tableDiff->changedColumns as $column) {
 				$column->oldColumnName = $this->connection->quoteIdentifier($column->oldColumnName);
 				// auto increment is not relevant for oracle and can anyhow not be applied on change
 				$column->changedProperties = array_diff($column->changedProperties, ['autoincrement', 'unsigned']);
 			}
+			// remove columns that no longer have changed (because autoincrement and unsigned are not supported)
 			$tableDiff->changedColumns = array_filter($tableDiff->changedColumns, function (ColumnDiff $column) {
 				return count($column->changedProperties) > 0;
 			});
+
+			$tableDiff->removedColumns = array_map(function(Column $column) {
+				return $this->quoteColumn($column);
+			}, $tableDiff->removedColumns);
+
+			$tableDiff->renamedColumns = array_map(function(Column $column) {
+				return $this->quoteColumn($column);
+			}, $tableDiff->renamedColumns);
+
+			$tableDiff->addedIndexes = array_map(function(Index $index) {
+				return $this->quoteIndex($index);
+			}, $tableDiff->addedIndexes);
+
+			$tableDiff->changedIndexes = array_map(function(Index $index) {
+				return $this->quoteIndex($index);
+			}, $tableDiff->changedIndexes);
+
+			$tableDiff->removedIndexes = array_map(function(Index $index) {
+				return $this->quoteIndex($index);
+			}, $tableDiff->removedIndexes);
+
+			$tableDiff->renamedIndexes = array_map(function(Index $index) {
+				return $this->quoteIndex($index);
+			}, $tableDiff->renamedIndexes);
+
+			$tableDiff->addedForeignKeys = array_map(function(ForeignKeyConstraint $fkc) {
+				return $this->quoteForeignKeyConstraint($fkc);
+			}, $tableDiff->addedForeignKeys);
+
+			$tableDiff->changedForeignKeys = array_map(function(ForeignKeyConstraint $fkc) {
+				return $this->quoteForeignKeyConstraint($fkc);
+			}, $tableDiff->changedForeignKeys);
+
+			$tableDiff->removedForeignKeys = array_map(function(ForeignKeyConstraint $fkc) {
+				return $this->quoteForeignKeyConstraint($fkc);
+			}, $tableDiff->removedForeignKeys);
 		}
 
 		return $schemaDiff;