Merge pull request #10555 from nextcloud/bugfix/10518/only-check-added-items

Only enforce schema limits for supported apps

lib/private/DB/MigrationService.php

@@ -25,11 +25,12 @@ namespace OC\DB;
 
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
-use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaException;
 use Doctrine\DBAL\Schema\Sequence;
+use Doctrine\DBAL\Schema\Table;
+use OC\App\InfoParser;
 use OC\IntegrityCheck\Helpers\AppLocator;
 use OC\Migration\SimpleOutput;
 use OCP\AppFramework\App;
@@ -51,6 +52,8 @@ class MigrationService {
 	private $connection;
 	/** @var string */
 	private $appName;
+	/** @var bool */
+	private $checkOracle;
 
 	/**
 	 * MigrationService constructor.
@@ -72,6 +75,7 @@ class MigrationService {
 		if ($appName === 'core') {
 			$this->migrationsPath = \OC::$SERVERROOT . '/core/Migrations';
 			$this->migrationsNamespace = 'OC\\Core\\Migrations';
+			$this->checkOracle = true;
 		} else {
 			if (null === $appLocator) {
 				$appLocator = new AppLocator();
@@ -80,6 +84,21 @@ class MigrationService {
 			$namespace = App::buildAppNamespace($appName);
 			$this->migrationsPath = "$appPath/lib/Migration";
 			$this->migrationsNamespace = $namespace . '\\Migration';
+
+			$infoParser = new InfoParser();
+			$info = $infoParser->parse($appPath . '/appinfo/info.xml');
+			if (!isset($info['dependencies']['database'])) {
+				$this->checkOracle = true;
+			} else {
+				$this->checkOracle = false;
+				foreach ($info['dependencies']['database'] as $database) {
+					if (\is_string($database) && $database === 'oci') {
+						$this->checkOracle = true;
+					} else if (\is_array($database) && isset($database['@value']) && $database['@value'] === 'oci') {
+						$this->checkOracle = true;
+					}
+				}
+			}
 		}
 	}
 
@@ -457,8 +476,10 @@ class MigrationService {
 
 		if ($toSchema instanceof SchemaWrapper) {
 			$targetSchema = $toSchema->getWrappedSchema();
-			// TODO re-enable once stable14 is branched of: https://github.com/nextcloud/server/issues/10518
-			// $this->ensureOracleIdentifierLengthLimit($targetSchema, strlen($this->connection->getPrefix()));
+			if ($this->checkOracle) {
+				$sourceSchema = $this->connection->createSchema();
+				$this->ensureOracleIdentifierLengthLimit($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
+			}
 			$this->connection->migrateToSchema($targetSchema);
 			$toSchema->performDropTableCalls();
 		}
@@ -472,34 +493,39 @@ class MigrationService {
 		$this->markAsExecuted($version);
 	}
 
-	public function ensureOracleIdentifierLengthLimit(Schema $schema, int $prefixLength) {
-		$sequences = $schema->getSequences();
+	public function ensureOracleIdentifierLengthLimit(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {
+		$sequences = $targetSchema->getSequences();
 
-		foreach ($schema->getTables() as $table) {
-			if (\strlen($table->getName()) - $prefixLength > 27) {
-				throw new \InvalidArgumentException('Table name "'  . $table->getName() . '" is too long.');
+		foreach ($targetSchema->getTables() as $table) {
+			try {
+				$sourceTable = $sourceSchema->getTable($table->getName());
+			} catch (SchemaException $e) {
+				if (\strlen($table->getName()) - $prefixLength > 27) {
+					throw new \InvalidArgumentException('Table name "'  . $table->getName() . '" is too long.');
+				}
+				$sourceTable = null;
 			}
 
 			foreach ($table->getColumns() as $thing) {
-				if (\strlen($thing->getName()) - $prefixLength > 27) {
+				if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
 					throw new \InvalidArgumentException('Column name "'  . $table->getName() . '"."' . $thing->getName() . '" is too long.');
 				}
 			}
 
 			foreach ($table->getIndexes() as $thing) {
-				if (\strlen($thing->getName()) - $prefixLength > 27) {
+				if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
 					throw new \InvalidArgumentException('Index name "'  . $table->getName() . '"."' . $thing->getName() . '" is too long.');
 				}
 			}
 
 			foreach ($table->getForeignKeys() as $thing) {
-				if (\strlen($thing->getName()) - $prefixLength > 27) {
+				if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
 					throw new \InvalidArgumentException('Foreign key name "'  . $table->getName() . '"."' . $thing->getName() . '" is too long.');
 				}
 			}
 
 			$primaryKey = $table->getPrimaryKey();
-			if ($primaryKey instanceof Index) {
+			if ($primaryKey instanceof Index && (!$sourceTable instanceof Table || !$sourceTable->hasPrimaryKey())) {
 				$indexName = strtolower($primaryKey->getName());
 				$isUsingDefaultName = $indexName === 'primary';
 
@@ -528,7 +554,7 @@ class MigrationService {
 		}
 
 		foreach ($sequences as $sequence) {
-			if (\strlen($sequence->getName()) - $prefixLength > 27) {
+			if (!$sourceSchema->hasSequence($sequence->getName()) && \strlen($sequence->getName()) - $prefixLength > 27) {
 				throw new \InvalidArgumentException('Sequence name "'  . $sequence->getName() . '" is too long.');
 			}
 		}

tests/lib/DB/MigrationsTest.php

@@ -16,6 +16,7 @@ use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Schema\SchemaException;
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
 use OC\DB\Connection;
@@ -102,13 +103,12 @@ class MigrationsTest extends \Test\TestCase {
 			->method('migrateToSchema');
 
 		$wrappedSchema = $this->createMock(Schema::class);
-		// TODO re-enable once stable14 is branched of: https://github.com/nextcloud/server/issues/10518
-		/*$wrappedSchema->expects($this->once())
+		$wrappedSchema->expects($this->once())
 			->method('getTables')
 			->willReturn([]);
 		$wrappedSchema->expects($this->once())
 			->method('getSequences')
-			->willReturn([]);*/
+			->willReturn([]);
 
 		$schemaResult = $this->createMock(SchemaWrapper::class);
 		$schemaResult->expects($this->once())
@@ -239,12 +239,12 @@ class MigrationsTest extends \Test\TestCase {
 			->willReturn(\str_repeat('a', 30));
 
 		$table = $this->createMock(Table::class);
-		$table->expects($this->once())
+		$table->expects($this->atLeastOnce())
 			->method('getName')
 			->willReturn(\str_repeat('a', 30));
 
 		$sequence = $this->createMock(Sequence::class);
-		$sequence->expects($this->once())
+		$sequence->expects($this->atLeastOnce())
 			->method('getName')
 			->willReturn(\str_repeat('a', 30));
 
@@ -269,7 +269,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getSequences')
 			->willReturn([$sequence]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKey() {
@@ -304,7 +312,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getSequences')
 			->willReturn([]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKeyDefault() {
@@ -349,7 +365,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getSequences')
 			->willReturn([]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -366,7 +390,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -411,7 +443,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -446,7 +486,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -472,7 +520,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -501,7 +557,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -533,7 +597,15 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getTables')
 			->willReturn([$table]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 
 	/**
@@ -553,6 +625,14 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getSequences')
 			->willReturn([$sequence]);
 
-		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
 }