diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 42f3b7de6a..ee64b45be6 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -459,7 +459,7 @@ class MigrationService { $targetSchema = $toSchema->getWrappedSchema(); if ($this->checkOracle) { $beforeSchema = $this->connection->createSchema(); - $this->ensureOracleIdentifierLengthLimit($beforeSchema, $targetSchema, strlen($this->connection->getPrefix())); + $this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix())); } $this->connection->migrateToSchema($targetSchema); $toSchema->performDropTableCalls(); @@ -536,7 +536,7 @@ class MigrationService { $targetSchema = $toSchema->getWrappedSchema(); if ($this->checkOracle) { $sourceSchema = $this->connection->createSchema(); - $this->ensureOracleIdentifierLengthLimit($sourceSchema, $targetSchema, strlen($this->connection->getPrefix())); + $this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix())); } $this->connection->migrateToSchema($targetSchema); $toSchema->performDropTableCalls(); @@ -551,7 +551,25 @@ class MigrationService { $this->markAsExecuted($version); } - public function ensureOracleIdentifierLengthLimit(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) { + /** + * Naming constraints: + * - Tables names must be 30 chars or shorter (27 + oc_ prefix) + * - Column names must be 30 chars or shorter + * - Index names must be 30 chars or shorter + * - Sequence names must be 30 chars or shorter + * - Primary key names must be set or the table name 23 chars or shorter + * + * Data constraints: + * - Columns with "NotNull" can not have empty string as default value + * - Columns with "NotNull" can not have number 0 as default value + * - Columns with type "bool" (which is in fact integer of length 1) can not be "NotNull" as it can not store 0/false + * + * @param Schema $sourceSchema + * @param Schema $targetSchema + * @param int $prefixLength + * @throws \Doctrine\DBAL\Exception + */ + public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) { $sequences = $targetSchema->getSequences(); foreach ($targetSchema->getTables() as $table) { @@ -571,7 +589,11 @@ class MigrationService { if ($thing->getNotnull() && $thing->getDefault() === '' && $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) { - throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.'); + throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.'); + } + + if ($thing->getNotnull() && $thing->getType()->getName() === Types::BOOLEAN) { + throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type Bool and also NotNull, so it can not store "false".'); } } @@ -583,7 +605,7 @@ class MigrationService { foreach ($table->getForeignKeys() as $thing) { if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) && \strlen($thing->getName()) > 30) { - throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.'); + throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.'); } } diff --git a/tests/lib/DB/MigrationsTest.php b/tests/lib/DB/MigrationsTest.php index 9b6dec006d..507f3d0e22 100644 --- a/tests/lib/DB/MigrationsTest.php +++ b/tests/lib/DB/MigrationsTest.php @@ -18,6 +18,7 @@ use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaException; use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; +use Doctrine\DBAL\Types\Type; use OC\DB\Connection; use OC\DB\MigrationService; use OC\DB\SchemaWrapper; @@ -219,7 +220,7 @@ class MigrationsTest extends \Test\TestCase { $this->migrationService->migrate(); } - public function testEnsureOracleIdentifierLengthLimitValid() { + public function testEnsureOracleConstraintsValid() { $column = $this->createMock(Column::class); $column->expects($this->once()) ->method('getName') @@ -274,10 +275,10 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKey() { + public function testEnsureOracleConstraintsValidWithPrimaryKey() { $index = $this->createMock(Index::class); $index->expects($this->any()) ->method('getName') @@ -317,10 +318,10 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKeyDefault() { + public function testEnsureOracleConstraintsValidWithPrimaryKeyDefault() { $defaultName = 'PRIMARY'; if ($this->db->getDatabasePlatform() instanceof PostgreSqlPlatform) { $defaultName = \str_repeat('a', 26) . '_' . \str_repeat('b', 30) . '_seq'; @@ -370,11 +371,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongTableName() { + public function testEnsureOracleConstraintsTooLongTableName() { $this->expectException(\InvalidArgumentException::class); $table = $this->createMock(Table::class); @@ -395,11 +396,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithDefault() { + public function testEnsureOracleConstraintsTooLongPrimaryWithDefault() { $this->expectException(\InvalidArgumentException::class); $defaultName = 'PRIMARY'; @@ -448,11 +449,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithName() { + public function testEnsureOracleConstraintsTooLongPrimaryWithName() { $this->expectException(\InvalidArgumentException::class); $index = $this->createMock(Index::class); @@ -491,11 +492,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongColumnName() { + public function testEnsureOracleConstraintsTooLongColumnName() { $this->expectException(\InvalidArgumentException::class); $column = $this->createMock(Column::class); @@ -525,11 +526,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongIndexName() { + public function testEnsureOracleConstraintsTooLongIndexName() { $this->expectException(\InvalidArgumentException::class); $index = $this->createMock(Index::class); @@ -562,11 +563,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongForeignKeyName() { + public function testEnsureOracleConstraintsTooLongForeignKeyName() { $this->expectException(\InvalidArgumentException::class); $foreignKey = $this->createMock(ForeignKeyConstraint::class); @@ -602,11 +603,11 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } - public function testEnsureOracleIdentifierLengthLimitTooLongSequenceName() { + public function testEnsureOracleConstraintsTooLongSequenceName() { $this->expectException(\InvalidArgumentException::class); $sequence = $this->createMock(Sequence::class); @@ -630,6 +631,46 @@ class MigrationsTest extends \Test\TestCase { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]); + self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + } + + + public function testEnsureOracleConstraintsBooleanNotNull() { + $this->expectException(\InvalidArgumentException::class); + + $column = $this->createMock(Column::class); + $column->expects($this->any()) + ->method('getName') + ->willReturn('aaaa'); + $column->expects($this->any()) + ->method('getType') + ->willReturn(Type::getType('boolean')); + $column->expects($this->any()) + ->method('getNotnull') + ->willReturn(true); + + $table = $this->createMock(Table::class); + $table->expects($this->any()) + ->method('getName') + ->willReturn(\str_repeat('a', 30)); + + $table->expects($this->once()) + ->method('getColumns') + ->willReturn([$column]); + + $schema = $this->createMock(Schema::class); + $schema->expects($this->once()) + ->method('getTables') + ->willReturn([$table]); + + $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, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); } }