diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 985c331570f38..61645dc901af3 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -7,6 +7,7 @@ */ namespace OC\DB; +use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaException; @@ -17,13 +18,14 @@ use OC\Migration\SimpleOutput; use OCP\App\IAppManager; use OCP\AppFramework\App; -use OCP\AppFramework\QueryException; use OCP\DB\ISchemaWrapper; use OCP\DB\Types; +use OCP\IConfig; use OCP\IDBConnection; use OCP\Migration\IMigrationStep; use OCP\Migration\IOutput; use OCP\Server; +use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; class MigrationService { @@ -47,6 +49,7 @@ public function __construct( ?LoggerInterface $logger = null, ) { $this->appName = $appName; + $this->checkOracle = false; $this->connection = $connection; if ($logger === null) { $this->logger = Server::get(LoggerInterface::class); @@ -103,7 +106,7 @@ private function createMigrationTable(): bool { return false; } - if ($this->connection->tableExists('migrations') && \OC::$server->getConfig()->getAppValue('core', 'vendor', '') !== 'owncloud') { + if ($this->connection->tableExists('migrations') && \OCP\Server::get(IConfig::class)->getAppValue('core', 'vendor', '') !== 'owncloud') { $this->migrationTableCreated = true; return false; } @@ -282,7 +285,7 @@ private function shallBeExecuted($m, $knownMigrations) { /** * @param string $version */ - private function markAsExecuted($version) { + private function markAsExecuted($version): void { $this->connection->insertIfNotExist('*PREFIX*migrations', [ 'app' => $this->appName, 'version' => $version @@ -343,7 +346,7 @@ private function getRelativeVersion(string $version, int $delta): ?string { $versions = $this->getAvailableVersions(); array_unshift($versions, '0'); - /** @var int $offset */ + /** @var int|false $offset */ $offset = array_search($version, $versions, true); if ($offset === false || !isset($versions[$offset + $delta])) { // Unknown version or delta out of bounds. @@ -358,8 +361,7 @@ private function getCurrentVersion(): string { if (count($m) === 0) { return '0'; } - $migrations = array_values($m); - return @end($migrations); + return @end($m); } /** @@ -431,10 +433,11 @@ public function migrateSchemaOnly(string $to = 'latest'): void { if ($toSchema instanceof SchemaWrapper) { $this->output->debug('- Checking target database schema'); $targetSchema = $toSchema->getWrappedSchema(); + $beforeSchema = $this->connection->createSchema(); $this->ensureUniqueNamesConstraints($targetSchema, true); + $this->ensureNamingConstraints($beforeSchema, $targetSchema, \strlen($this->connection->getPrefix())); if ($this->checkOracle) { - $beforeSchema = $this->connection->createSchema(); - $this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix())); + $this->ensureOracleConstraints($beforeSchema, $targetSchema); } $this->output->debug('- Migrate database schema'); @@ -472,14 +475,11 @@ public function describeMigrationStep($to = 'latest') { * @throws \InvalidArgumentException */ public function createInstance($version) { + /** @psalm-var class-string $class */ $class = $this->getClass($version); try { $s = \OCP\Server::get($class); - - if (!$s instanceof IMigrationStep) { - throw new \InvalidArgumentException('Not a valid migration'); - } - } catch (QueryException $e) { + } catch (NotFoundExceptionInterface) { if (class_exists($class)) { $s = new $class(); } else { @@ -487,6 +487,9 @@ public function createInstance($version) { } } + if (!$s instanceof IMigrationStep) { + throw new \InvalidArgumentException('Not a valid migration'); + } return $s; } @@ -497,7 +500,7 @@ public function createInstance($version) { * @param bool $schemaOnly * @throws \InvalidArgumentException */ - public function executeStep($version, $schemaOnly = false) { + public function executeStep($version, $schemaOnly = false): void { $instance = $this->createInstance($version); if (!$schemaOnly) { @@ -512,10 +515,11 @@ public function executeStep($version, $schemaOnly = false) { if ($toSchema instanceof SchemaWrapper) { $targetSchema = $toSchema->getWrappedSchema(); + $sourceSchema = $this->connection->createSchema(); $this->ensureUniqueNamesConstraints($targetSchema, $schemaOnly); + $this->ensureNamingConstraints($sourceSchema, $targetSchema, \strlen($this->connection->getPrefix())); if ($this->checkOracle) { - $sourceSchema = $this->connection->createSchema(); - $this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix())); + $this->ensureOracleConstraints($sourceSchema, $targetSchema); } $this->connection->migrateToSchema($targetSchema); $toSchema->performDropTableCalls(); @@ -531,12 +535,108 @@ public function executeStep($version, $schemaOnly = false) { } /** + * Enforces some naming conventions to make sure tables can be used on all supported database engines. + * * 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 + * - Tables names must be 63 chars or shorter (including its prefix (default 'oc_')) + * - Column names must be 63 chars or shorter + * - Index names must be 63 chars or shorter + * - Sequence names must be 63 chars or shorter + * - Primary key names must be set to 63 chars or shorter - or the table name must be <= 58 characters (63 - 5 for '_pKey' suffix) including the table name prefix + * + * This is based on the identifier limits set by our supported database engines: + * - MySQL and MariaDB support 64 characters + * - Oracle supports 128 characters (since 12c) + * - PostgreSQL support 63 + * - SQLite does not have any limits + * + * @see https://github.com/nextcloud/documentation/blob/master/developer_manual/basics/storage/database.rst + * + * @throws \Doctrine\DBAL\Exception + */ + public function ensureNamingConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength): void { + $MAX_NAME_LENGTH = 63; + $sequences = $targetSchema->getSequences(); + + foreach ($targetSchema->getTables() as $table) { + try { + $sourceTable = $sourceSchema->getTable($table->getName()); + } catch (SchemaException $e) { + // we only validate new tables + if (\strlen($table->getName()) + $prefixLength > $MAX_NAME_LENGTH) { + throw new \InvalidArgumentException('Table name "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + $sourceTable = null; + } + + foreach ($table->getColumns() as $thing) { + // If the table doesn't exist OR if the column doesn't exist in the table + if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) + && \strlen($thing->getName()) > $MAX_NAME_LENGTH + ) { + throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + } + + foreach ($table->getIndexes() as $thing) { + if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName())) + && \strlen($thing->getName()) > $MAX_NAME_LENGTH + ) { + throw new \InvalidArgumentException('Index name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + } + + foreach ($table->getForeignKeys() as $thing) { + if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) + && \strlen($thing->getName()) > $MAX_NAME_LENGTH + ) { + throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + } + + $primaryKey = $table->getPrimaryKey(); + // only check if there is a primary key + // and there was non in the old table or there was no old table + if ($primaryKey !== null && ($sourceTable === null || $sourceTable->getPrimaryKey() === null)) { + $indexName = strtolower($primaryKey->getName()); + $isUsingDefaultName = $indexName === 'primary'; + // This is the default name when using postgres - we use this for length comparison + // as this is the longest default names for the DB engines provided by doctrine + $defaultName = strtolower($table->getName() . '_pkey'); + + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_POSTGRES) { + $isUsingDefaultName = $defaultName === $indexName; + + if ($isUsingDefaultName) { + $sequenceName = $table->getName() . '_' . implode('_', $primaryKey->getColumns()) . '_seq'; + $sequences = array_filter($sequences, function (Sequence $sequence) use ($sequenceName) { + return $sequence->getName() !== $sequenceName; + }); + } + } elseif ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + $isUsingDefaultName = strtolower($table->getName() . '_seq') === $indexName; + } + + if (!$isUsingDefaultName && \strlen($indexName) > $MAX_NAME_LENGTH) { + throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + if ($isUsingDefaultName && \strlen($defaultName) + $prefixLength > $MAX_NAME_LENGTH) { + throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + } + } + + foreach ($sequences as $sequence) { + if (!$sourceSchema->hasSequence($sequence->getName()) + && \strlen($sequence->getName()) > $MAX_NAME_LENGTH + ) { + throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH); + } + } + } + + /** + * Enforces some data conventions to make sure tables can be used on Oracle SQL. * * Data constraints: * - Tables need a primary key (Not specific to Oracle, but required for performant clustering support) @@ -546,66 +646,47 @@ public function executeStep($version, $schemaOnly = false) { * - Columns with type "string" can not be longer than 4.000 characters, use "text" instead * * @see https://github.com/nextcloud/documentation/blob/master/developer_manual/basics/storage/database.rst - * - * @param Schema $sourceSchema - * @param Schema $targetSchema - * @param int $prefixLength * @throws \Doctrine\DBAL\Exception */ - public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) { + public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema): void { $sequences = $targetSchema->getSequences(); 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) { + foreach ($table->getColumns() as $column) { // If the table doesn't exist OR if the column doesn't exist in the table - if (!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) { - if (\strlen($thing->getName()) > 30) { - throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.'); - } - - if ($thing->getNotnull() && $thing->getDefault() === '' - && $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) { - throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.'); + if (!$sourceTable instanceof Table || !$sourceTable->hasColumn($column->getName())) { + if ($column->getNotnull() && $column->getDefault() === '' + && $sourceTable instanceof Table && !$sourceTable->hasColumn($column->getName())) { + // null and empty string are the same on Oracle SQL + throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $column->getName() . '" is NotNull, but has empty string or null as default.'); } - if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE + && $column->getNotnull() + && Type::lookupName($column->getType()) === Types::BOOLEAN + ) { // Oracle doesn't support boolean column with non-null value - if ($thing->getNotnull() && Type::lookupName($thing->getType()) === Types::BOOLEAN) { - $thing->setNotnull(false); - } + // to still allow lighter DB schemas on other providers we force it to not null + // see https://github.com/nextcloud/server/pull/55156 + $column->setNotnull(false); } $sourceColumn = null; } else { - $sourceColumn = $sourceTable->getColumn($thing->getName()); + $sourceColumn = $sourceTable->getColumn($column->getName()); } // If the column was just created OR the length changed OR the type changed // we will NOT detect invalid length if the column is not modified - if (($sourceColumn === null || $sourceColumn->getLength() !== $thing->getLength() || Type::lookupName($sourceColumn->getType()) !== Types::STRING) - && $thing->getLength() > 4000 && Type::lookupName($thing->getType()) === Types::STRING) { - throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type String, but exceeding the 4.000 length limit.'); - } - } - - foreach ($table->getIndexes() as $thing) { - if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName())) && \strlen($thing->getName()) > 30) { - throw new \InvalidArgumentException('Index name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.'); - } - } - - 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.'); + if (($sourceColumn === null || $sourceColumn->getLength() !== $column->getLength() || Type::lookupName($sourceColumn->getType()) !== Types::STRING) + && $column->getLength() > 4000 && Type::lookupName($column->getType()) === Types::STRING) { + throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $column->getName() . '" is type String, but exceeding the 4.000 length limit.'); } } @@ -628,26 +709,13 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche $defaultName = $table->getName() . '_seq'; $isUsingDefaultName = strtolower($defaultName) === $indexName; } - - if (!$isUsingDefaultName && \strlen($indexName) > 30) { - throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" is too long.'); - } - if ($isUsingDefaultName && \strlen($table->getName()) - $prefixLength >= 23) { - throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" is too long.'); - } } elseif (!$primaryKey instanceof Index && !$sourceTable instanceof Table) { /** @var LoggerInterface $logger */ - $logger = \OC::$server->get(LoggerInterface::class); + $logger = \OCP\Server::get(LoggerInterface::class); $logger->error('Table "' . $table->getName() . '" has no primary key and therefor will not behave sane in clustered setups. This will throw an exception and not be installable in a future version of Nextcloud.'); // throw new \InvalidArgumentException('Table "' . $table->getName() . '" has no primary key and therefor will not behave sane in clustered setups.'); } } - - foreach ($sequences as $sequence) { - if (!$sourceSchema->hasSequence($sequence->getName()) && \strlen($sequence->getName()) > 30) { - throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" is too long.'); - } - } } /** diff --git a/tests/lib/DB/MigrationsTest.php b/tests/lib/DB/MigrationServiceTest.php similarity index 59% rename from tests/lib/DB/MigrationsTest.php rename to tests/lib/DB/MigrationServiceTest.php index da92261b85005..c2c8cdc03cc9b 100644 --- a/tests/lib/DB/MigrationsTest.php +++ b/tests/lib/DB/MigrationServiceTest.php @@ -19,42 +19,34 @@ use OC\DB\Connection; use OC\DB\MigrationService; use OC\DB\SchemaWrapper; -use OC\Migration\MetadataManager; use OCP\App\AppPathNotFoundException; -use OCP\App\IAppManager; use OCP\IDBConnection; -use OCP\Migration\Attributes\AddColumn; -use OCP\Migration\Attributes\AddIndex; -use OCP\Migration\Attributes\ColumnType; -use OCP\Migration\Attributes\CreateTable; -use OCP\Migration\Attributes\DropColumn; -use OCP\Migration\Attributes\DropIndex; -use OCP\Migration\Attributes\DropTable; -use OCP\Migration\Attributes\IndexType; -use OCP\Migration\Attributes\ModifyColumn; use OCP\Migration\IMigrationStep; -use OCP\Server; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; /** - * Class MigrationsTest + * Class MigrationServiceTest * * @package Test\DB */ -class MigrationsTest extends \Test\TestCase { - private MigrationService|MockObject $migrationService; - private MockObject|IDBConnection $db; - private IAppManager $appManager; +class MigrationServiceTest extends \Test\TestCase { + private Connection&MockObject $db; + + private MigrationService $migrationService; protected function setUp(): void { parent::setUp(); $this->db = $this->createMock(Connection::class); - $this->db->expects($this->any())->method('getPrefix')->willReturn('test_oc_'); - $this->migrationService = new MigrationService('testing', $this->db); + $this->db + ->expects($this->any()) + ->method('getPrefix') + ->willReturn('test_oc_'); - $this->appManager = Server::get(IAppManager::class); + $this->migrationService = new MigrationService('testing', $this->db); } public function testGetters(): void { @@ -65,15 +57,14 @@ public function testGetters(): void { } public function testCore(): void { - $this->migrationService = new MigrationService('core', $this->db); + $migrationService = new MigrationService('core', $this->db); - $this->assertEquals('core', $this->migrationService->getApp()); - $this->assertEquals(\OC::$SERVERROOT . '/core/Migrations', $this->migrationService->getMigrationsDirectory()); - $this->assertEquals('OC\Core\Migrations', $this->migrationService->getMigrationsNamespace()); - $this->assertEquals('test_oc_migrations', $this->migrationService->getMigrationsTableName()); + $this->assertEquals('core', $migrationService->getApp()); + $this->assertEquals(\OC::$SERVERROOT . '/core/Migrations', $migrationService->getMigrationsDirectory()); + $this->assertEquals('OC\Core\Migrations', $migrationService->getMigrationsNamespace()); + $this->assertEquals('test_oc_migrations', $migrationService->getMigrationsTableName()); } - public function testExecuteUnknownStep(): void { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Version 20170130180000 is unknown.'); @@ -81,27 +72,25 @@ public function testExecuteUnknownStep(): void { $this->migrationService->executeStep('20170130180000'); } - public function testUnknownApp(): void { $this->expectException(AppPathNotFoundException::class); $this->expectExceptionMessage('Could not find path for unknown_bloody_app'); - $migrationService = new MigrationService('unknown_bloody_app', $this->db); + new MigrationService('unknown_bloody_app', $this->db); } - public function testExecuteStepWithUnknownClass(): void { $this->expectException(\Exception::class); $this->expectExceptionMessage('Migration step \'X\' is unknown'); - $this->migrationService = $this->getMockBuilder(MigrationService::class) + $migrationService = $this->getMockBuilder(MigrationService::class) ->onlyMethods(['findMigrations']) ->setConstructorArgs(['testing', $this->db]) ->getMock(); - $this->migrationService->expects($this->any())->method('findMigrations')->willReturn( + $migrationService->expects($this->any())->method('findMigrations')->willReturn( ['20170130180000' => 'X', '20170130180001' => 'Y', '20170130180002' => 'Z', '20170130180003' => 'A'] ); - $this->migrationService->executeStep('20170130180000'); + $migrationService->executeStep('20170130180000'); } public function testExecuteStepWithSchemaChange(): void { @@ -114,10 +103,10 @@ public function testExecuteStepWithSchemaChange(): void { ->method('migrateToSchema'); $wrappedSchema = $this->createMock(Schema::class); - $wrappedSchema->expects($this->exactly(2)) + $wrappedSchema->expects($this->atLeast(2)) ->method('getTables') ->willReturn([]); - $wrappedSchema->expects($this->exactly(2)) + $wrappedSchema->expects($this->atLeast(2)) ->method('getSequences') ->willReturn([]); @@ -135,16 +124,17 @@ public function testExecuteStepWithSchemaChange(): void { $step->expects($this->once()) ->method('postSchemaChange'); - $this->migrationService = $this->getMockBuilder(MigrationService::class) + $migrationService = $this->getMockBuilder(MigrationService::class) ->onlyMethods(['createInstance']) ->setConstructorArgs(['testing', $this->db]) ->getMock(); - $this->migrationService->expects($this->any()) + $migrationService->expects($this->any()) ->method('createInstance') ->with('20170130180000') ->willReturn($step); - $this->migrationService->executeStep('20170130180000'); + + $migrationService->executeStep('20170130180000'); } public function testExecuteStepWithoutSchemaChange(): void { @@ -165,16 +155,17 @@ public function testExecuteStepWithoutSchemaChange(): void { $step->expects($this->once()) ->method('postSchemaChange'); - $this->migrationService = $this->getMockBuilder(MigrationService::class) + $migrationService = $this->getMockBuilder(MigrationService::class) ->onlyMethods(['createInstance']) ->setConstructorArgs(['testing', $this->db]) ->getMock(); - $this->migrationService->expects($this->any()) + $migrationService->expects($this->any()) ->method('createInstance') ->with('20170130180000') ->willReturn($step); - $this->migrationService->executeStep('20170130180000'); + + $migrationService->executeStep('20170130180000'); } public static function dataGetMigration(): array { @@ -192,130 +183,160 @@ public static function dataGetMigration(): array { */ #[\PHPUnit\Framework\Attributes\DataProvider('dataGetMigration')] public function testGetMigration($alias, $expected): void { - $this->migrationService = $this->getMockBuilder(MigrationService::class) + $migrationService = $this->getMockBuilder(MigrationService::class) ->onlyMethods(['getMigratedVersions', 'findMigrations']) ->setConstructorArgs(['testing', $this->db]) ->getMock(); - $this->migrationService->expects($this->any())->method('getMigratedVersions')->willReturn( + + $migrationService->expects($this->any())->method('getMigratedVersions')->willReturn( ['20170130180000', '20170130180001'] ); - $this->migrationService->expects($this->any())->method('findMigrations')->willReturn( + $migrationService->expects($this->any())->method('findMigrations')->willReturn( ['20170130180000' => 'X', '20170130180001' => 'Y', '20170130180002' => 'Z', '20170130180003' => 'A'] ); $this->assertEquals( ['20170130180000', '20170130180001', '20170130180002', '20170130180003'], - $this->migrationService->getAvailableVersions()); + $migrationService->getAvailableVersions()); - $migration = $this->migrationService->getMigration($alias); + $migration = $migrationService->getMigration($alias); $this->assertEquals($expected, $migration); } public function testMigrate(): void { - $this->migrationService = $this->getMockBuilder(MigrationService::class) + $migrationService = $this->getMockBuilder(MigrationService::class) ->onlyMethods(['getMigratedVersions', 'findMigrations', 'executeStep']) ->setConstructorArgs(['testing', $this->db]) ->getMock(); - $this->migrationService->method('getMigratedVersions') - ->willReturn( - ['20170130180000', '20170130180001'] - ); - $this->migrationService->method('findMigrations') - ->willReturn( - ['20170130180000' => 'X', '20170130180001' => 'Y', '20170130180002' => 'Z', '20170130180003' => 'A'] - ); + + $migrationService->expects($this->any())->method('getMigratedVersions')->willReturn( + ['20170130180000', '20170130180001'] + ); + $migrationService->expects($this->any())->method('findMigrations')->willReturn( + ['20170130180000' => 'X', '20170130180001' => 'Y', '20170130180002' => 'Z', '20170130180003' => 'A'] + ); $this->assertEquals( ['20170130180000', '20170130180001', '20170130180002', '20170130180003'], - $this->migrationService->getAvailableVersions() - ); + $migrationService->getAvailableVersions()); - $calls = [ - ['20170130180002', false], - ['20170130180003', false], - ]; - $this->migrationService->expects($this->exactly(2)) + $calls = []; + $migrationService + ->expects($this->exactly(2)) ->method('executeStep') - ->willReturnCallback(function () use (&$calls): void { - $expected = array_shift($calls); - $this->assertEquals($expected, func_get_args()); + ->willReturnCallback(function (string $migration) use (&$calls) { + $calls[] = $migration; }); - $this->migrationService->migrate(); - } - - public function testEnsureOracleConstraintsValid(): void { - $column = $this->createMock(Column::class); - $column->expects($this->once()) - ->method('getName') - ->willReturn(\str_repeat('a', 30)); - $index = $this->createMock(Index::class); - $index->expects($this->once()) - ->method('getName') - ->willReturn(\str_repeat('a', 30)); + $migrationService->migrate(); + self::assertEquals(['20170130180002', '20170130180003'], $calls); + } - $foreignKey = $this->createMock(ForeignKeyConstraint::class); - $foreignKey->expects($this->once()) - ->method('getName') - ->willReturn(\str_repeat('a', 30)); + #[DataProvider('dataEnsureNamingConstraintsTableName')] + public function testEnsureNamingConstraintsTableName(string $name, int $prefixLength, bool $tableExists, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); + } $table = $this->createMock(Table::class); $table->expects($this->atLeastOnce()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); - - $sequence = $this->createMock(Sequence::class); - $sequence->expects($this->atLeastOnce()) - ->method('getName') - ->willReturn(\str_repeat('a', 30)); - - $primaryKey = $this->createMock(Index::class); - $primaryKey->expects($this->once()) - ->method('getName') - ->willReturn(\str_repeat('a', 30)); - - $table->expects($this->once()) + ->willReturn($name); + $table->expects($this->any()) ->method('getColumns') - ->willReturn([$column]); - $table->expects($this->once()) + ->willReturn([]); + $table->expects($this->any()) ->method('getIndexes') - ->willReturn([$index]); - $table->expects($this->once()) + ->willReturn([]); + $table->expects($this->any()) ->method('getForeignKeys') - ->willReturn([$foreignKey]); - $table->expects($this->once()) - ->method('getPrimaryKey') - ->willReturn($primaryKey); + ->willReturn([]); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); - $schema->expects($this->once()) + $schema->expects(self::once()) ->method('getSequences') - ->willReturn([$sequence]); + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) ->method('getTable') - ->willThrowException(new SchemaException()); + ->willReturnCallback(fn () => match($tableExists) { + false => throw new SchemaException(), + true => $table, + }); $sourceSchema->expects($this->any()) ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, $prefixLength); } - public function testEnsureOracleConstraintsValidWithPrimaryKey(): void { + public static function dataEnsureNamingConstraintsTableName(): array { + return [ + 'valid name' => [ + \str_repeat('x', 60), // table name + 3, // prefix length + false, // has this table + false, // throws + ], + 'valid name - long prefix' => [ + \str_repeat('x', 55), + 8, + false, + false, + ], + 'too long but not a new table' => [ + \str_repeat('x', 61), + 3, + true, + false, + ], + 'too long' => [ + \str_repeat('x', 61), + 3, + false, + true, + ], + 'too long with prefix' => [ + \str_repeat('x', 60), + 4, + false, + true, + ], + ]; + } + + #[DataProvider('dataEnsureNamingConstraintsPrimaryDefaultKey')] + public function testEnsureNamingConstraintsPrimaryDefaultKey(string $tableName, int $prefixLength, string $platform, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); + } + + $this->db->expects(self::atLeastOnce()) + ->method('getDatabaseProvider') + ->willReturn($platform); + + $defaultName = match ($platform) { + IDBConnection::PLATFORM_POSTGRES => $tableName . '_pkey', + IDBConnection::PLATFORM_ORACLE => $tableName . '_seq', + default => 'PRIMARY', + }; + $index = $this->createMock(Index::class); $index->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn($defaultName); + $index->expects($this->any()) + ->method('getColumns') + ->willReturn([]); $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 26)); + ->willReturn($tableName); $table->expects($this->once()) ->method('getColumns') @@ -334,7 +355,7 @@ public function testEnsureOracleConstraintsValidWithPrimaryKey(): void { $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); - $schema->expects($this->once()) + $schema->expects($this->atMost(1)) ->method('getSequences') ->willReturn([]); @@ -346,40 +367,59 @@ public function testEnsureOracleConstraintsValidWithPrimaryKey(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, $prefixLength); } - public function testEnsureOracleConstraintsValidWithPrimaryKeyDefault(): void { - $defaultName = 'PRIMARY'; - if ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_POSTGRES) { - $defaultName = \str_repeat('a', 26) . '_' . \str_repeat('b', 30) . '_seq'; - } elseif ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { - $defaultName = \str_repeat('a', 26) . '_seq'; + public static function dataEnsureNamingConstraintsPrimaryDefaultKey(): array { + foreach ([IDBConnection::PLATFORM_MYSQL, IDBConnection::PLATFORM_ORACLE, IDBConnection::PLATFORM_POSTGRES, IDBConnection::PLATFORM_SQLITE] as $engine) { + $testcases["$engine valid"] = [ + str_repeat('x', 55), + 3, + $engine, + false, + ]; + $testcases["$engine too long"] = [ + str_repeat('x', 56), // 56 (name) + 3 (prefix) + 5 ('_pkey')= 64 > 63 + 3, + $engine, + true, + ]; + $testcases["$engine too long prefix"] = [ + str_repeat('x', 55), + 4, + $engine, + true, + ]; + } + return $testcases; + } + + #[DataProvider('dataEnsureNamingConstraintsPrimaryCustomKey')] + public function testEnsureNamingConstraintsPrimaryCustomKey(string $name, int $prefixLength, bool $newIndex, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); } $index = $this->createMock(Index::class); $index->expects($this->any()) ->method('getName') - ->willReturn($defaultName); - $index->expects($this->any()) - ->method('getColumns') - ->willReturn([\str_repeat('b', 30)]); + ->willReturn($name); $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 25)); + ->willReturn('tablename'); - $table->expects($this->once()) + $table->expects($this->any()) ->method('getColumns') ->willReturn([]); - $table->expects($this->once()) + $table->expects($this->any()) ->method('getIndexes') ->willReturn([]); - $table->expects($this->once()) + $table->expects($this->any()) ->method('getForeignKeys') ->willReturn([]); - $table->expects($this->once()) + $table->expects($this->atLeastOnce()) ->method('getPrimaryKey') ->willReturn($index); @@ -394,123 +434,199 @@ public function testEnsureOracleConstraintsValidWithPrimaryKeyDefault(): void { $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) ->method('getTable') - ->willThrowException(new SchemaException()); + ->willReturnCallback(fn () => match($newIndex) { + true => throw new SchemaException(), + false => $table, + }); $sourceSchema->expects($this->any()) ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, $prefixLength); } + public static function dataEnsureNamingConstraintsPrimaryCustomKey(): array { + return [ + 'valid name' => [ + str_repeat('x', 60), + 3, + true, + false, + ], + 'valid name - prefix does not matter' => [ + str_repeat('x', 63), + 3, + true, + false, + ], + 'invalid name - but not new' => [ + str_repeat('x', 64), + 3, + false, + false, + ], + 'too long name' => [ + str_repeat('x', 64), + 3, + true, + true, + ], + ]; + } - public function testEnsureOracleConstraintsTooLongTableName(): void { - $this->expectException(\InvalidArgumentException::class); + #[DataProvider('dataEnsureNamingConstraints')] + public function testEnsureNamingConstraintsColumnName(string $name, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); + } + + $column = $this->createMock(Column::class); + $column->expects(self::atLeastOnce()) + ->method('getName') + ->willReturn($name); $table = $this->createMock(Table::class); - $table->expects($this->any()) + $table->expects(self::any()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn('valid'); + + $table->expects(self::once()) + ->method('getColumns') + ->willReturn([$column]); + $table->expects(self::atMost(1)) + ->method('getIndexes') + ->willReturn([]); + $table->expects(self::atMost(1)) + ->method('getForeignKeys') + ->willReturn([]); $schema = $this->createMock(Schema::class); - $schema->expects($this->once()) + $schema->expects(self::once()) ->method('getTables') ->willReturn([$table]); + $schema->expects(self::once()) + ->method('getSequences') + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); - $sourceSchema->expects($this->any()) + $sourceSchema->expects(self::any()) ->method('getTable') ->willThrowException(new SchemaException()); - $sourceSchema->expects($this->any()) + $sourceSchema->expects(self::any()) ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, 3); } - - public function testEnsureOracleConstraintsTooLongPrimaryWithDefault(): void { - $this->expectException(\InvalidArgumentException::class); - - $defaultName = 'PRIMARY'; - if ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_POSTGRES) { - $defaultName = \str_repeat('a', 27) . '_' . \str_repeat('b', 30) . '_seq'; - } elseif ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { - $defaultName = \str_repeat('a', 27) . '_seq'; + #[DataProvider('dataEnsureNamingConstraints')] + public function testEnsureNamingConstraintsIndexName(string $name, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); } $index = $this->createMock(Index::class); - $index->expects($this->any()) + $index->expects(self::atLeastOnce()) ->method('getName') - ->willReturn($defaultName); - $index->expects($this->any()) - ->method('getColumns') - ->willReturn([\str_repeat('b', 30)]); + ->willReturn($name); $table = $this->createMock(Table::class); - $table->expects($this->any()) + $table->expects(self::any()) ->method('getName') - ->willReturn(\str_repeat('a', 27)); + ->willReturn('valid'); - $table->expects($this->once()) + $table->expects(self::atMost(1)) ->method('getColumns') ->willReturn([]); - $table->expects($this->once()) + $table->expects(self::once()) ->method('getIndexes') - ->willReturn([]); - $table->expects($this->once()) + ->willReturn([$index]); + $table->expects(self::atMost(1)) ->method('getForeignKeys') ->willReturn([]); - $table->expects($this->once()) - ->method('getPrimaryKey') - ->willReturn($index); $schema = $this->createMock(Schema::class); - $schema->expects($this->once()) + $schema->expects(self::once()) ->method('getTables') ->willReturn([$table]); + $schema->expects(self::once()) + ->method('getSequences') + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); - $sourceSchema->expects($this->any()) + $sourceSchema->expects(self::any()) ->method('getTable') ->willThrowException(new SchemaException()); - $sourceSchema->expects($this->any()) + $sourceSchema->expects(self::any()) ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, 3); } + #[DataProvider('dataEnsureNamingConstraints')] + public function testEnsureNamingConstraintsForeignKeyName(string $name, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); + } - public function testEnsureOracleConstraintsTooLongPrimaryWithName(): void { - $this->expectException(\InvalidArgumentException::class); - - $index = $this->createMock(Index::class); - $index->expects($this->any()) + $foreignKey = $this->createMock(ForeignKeyConstraint::class); + $foreignKey->expects(self::any()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn($name); $table = $this->createMock(Table::class); - $table->expects($this->any()) + $table->expects(self::any()) ->method('getName') - ->willReturn(\str_repeat('a', 26)); + ->willReturn('valid'); - $table->expects($this->once()) + $table->expects(self::once()) ->method('getColumns') ->willReturn([]); - $table->expects($this->once()) + $table->expects(self::once()) ->method('getIndexes') ->willReturn([]); - $table->expects($this->once()) + $table->expects(self::once()) ->method('getForeignKeys') + ->willReturn([$foreignKey]); + + $schema = $this->createMock(Schema::class); + $schema->expects(self::once()) + ->method('getTables') + ->willReturn([$table]); + $schema->expects(self::once()) + ->method('getSequences') ->willReturn([]); - $table->expects($this->once()) - ->method('getPrimaryKey') - ->willReturn($index); + + $sourceSchema = $this->createMock(Schema::class); + $sourceSchema->expects(self::any()) + ->method('getTable') + ->willThrowException(new SchemaException()); + $sourceSchema->expects(self::any()) + ->method('hasSequence') + ->willReturn(false); + + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, 3); + } + + #[DataProvider('dataEnsureNamingConstraints')] + public function testEnsureNamingConstraintsSequenceName(string $name, bool $throws): void { + if ($throws) { + $this->expectException(\InvalidArgumentException::class); + } + + $sequence = $this->createMock(Sequence::class); + $sequence->expects($this->any()) + ->method('getName') + ->willReturn($name); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') - ->willReturn([$table]); + ->willReturn([]); + $schema->expects($this->once()) + ->method('getSequences') + ->willReturn([$sequence]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) @@ -520,31 +636,43 @@ public function testEnsureOracleConstraintsTooLongPrimaryWithName(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureNamingConstraints($sourceSchema, $schema, 3); } + public static function dataEnsureNamingConstraints(): array { + return [ + 'valid length' => [\str_repeat('x', 63), false], + 'too long' => [\str_repeat('x', 64), true], + ]; + } - public function testEnsureOracleConstraintsTooLongColumnName(): void { - $this->expectException(\InvalidArgumentException::class); - - $column = $this->createMock(Column::class); - $column->expects($this->any()) + public function testEnsureOracleConstraintsValid(): void { + $table = $this->createMock(Table::class); + $table->expects($this->atLeastOnce()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn('tablename'); - $table = $this->createMock(Table::class); - $table->expects($this->any()) + $primaryKey = $this->createMock(Index::class); + $primaryKey->expects($this->once()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn('primary_key'); + $column = $this->createMock(Column::class); $table->expects($this->once()) ->method('getColumns') ->willReturn([$column]); + $table->expects($this->once()) + ->method('getPrimaryKey') + ->willReturn($primaryKey); + $sequence = $this->createMock(Sequence::class); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); + $schema->expects($this->once()) + ->method('getSequences') + ->willReturn([$sequence]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) @@ -554,34 +682,34 @@ public function testEnsureOracleConstraintsTooLongColumnName(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } - - public function testEnsureOracleConstraintsTooLongIndexName(): void { - $this->expectException(\InvalidArgumentException::class); - + public function testEnsureOracleConstraintsValidWithPrimaryKey(): void { $index = $this->createMock(Index::class); $index->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn(\str_repeat('a', 30)); $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn(\str_repeat('a', 26)); $table->expects($this->once()) ->method('getColumns') ->willReturn([]); $table->expects($this->once()) - ->method('getIndexes') - ->willReturn([$index]); + ->method('getPrimaryKey') + ->willReturn($index); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); + $schema->expects($this->once()) + ->method('getSequences') + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) @@ -591,37 +719,44 @@ public function testEnsureOracleConstraintsTooLongIndexName(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } + public function testEnsureOracleConstraintsValidWithPrimaryKeyDefault(): void { + $defaultName = 'PRIMARY'; + if ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_POSTGRES) { + $defaultName = \str_repeat('a', 26) . '_' . \str_repeat('b', 30) . '_seq'; + } elseif ($this->db->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + $defaultName = \str_repeat('a', 26) . '_seq'; + } - public function testEnsureOracleConstraintsTooLongForeignKeyName(): void { - $this->expectException(\InvalidArgumentException::class); - - $foreignKey = $this->createMock(ForeignKeyConstraint::class); - $foreignKey->expects($this->any()) + $index = $this->createMock(Index::class); + $index->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn($defaultName); + $index->expects($this->any()) + ->method('getColumns') + ->willReturn([\str_repeat('b', 30)]); $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn(\str_repeat('a', 25)); $table->expects($this->once()) ->method('getColumns') ->willReturn([]); $table->expects($this->once()) - ->method('getIndexes') - ->willReturn([]); - $table->expects($this->once()) - ->method('getForeignKeys') - ->willReturn([$foreignKey]); + ->method('getPrimaryKey') + ->willReturn($index); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); + $schema->expects($this->once()) + ->method('getSequences') + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) @@ -631,10 +766,9 @@ public function testEnsureOracleConstraintsTooLongForeignKeyName(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } - public function testEnsureOracleConstraintsNoPrimaryKey(): void { $this->markTestSkipped('Test disabled for now due to multiple reasons, see https://github.com/nextcloud/server/pull/31580#issuecomment-1069182234 for details.'); $this->expectException(\InvalidArgumentException::class); @@ -642,16 +776,10 @@ public function testEnsureOracleConstraintsNoPrimaryKey(): void { $table = $this->createMock(Table::class); $table->expects($this->atLeastOnce()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn('tablename'); $table->expects($this->once()) ->method('getColumns') ->willReturn([]); - $table->expects($this->once()) - ->method('getIndexes') - ->willReturn([]); - $table->expects($this->once()) - ->method('getForeignKeys') - ->willReturn([]); $table->expects($this->once()) ->method('getPrimaryKey') ->willReturn(null); @@ -672,25 +800,31 @@ public function testEnsureOracleConstraintsNoPrimaryKey(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } - - public function testEnsureOracleConstraintsTooLongSequenceName(): void { - $this->expectException(\InvalidArgumentException::class); - - $sequence = $this->createMock(Sequence::class); - $sequence->expects($this->any()) + /** + * Alternative for testEnsureOracleConstraintsNoPrimaryKey until we enforce it. + */ + public function testEnsureOracleConstraintsNoPrimaryKeyLogging(): void { + $table = $this->createMock(Table::class); + $table->expects($this->atLeastOnce()) ->method('getName') - ->willReturn(\str_repeat('a', 31)); + ->willReturn('tablename'); + $table->expects($this->once()) + ->method('getColumns') + ->willReturn([]); + $table->expects($this->once()) + ->method('getPrimaryKey') + ->willReturn(null); $schema = $this->createMock(Schema::class); $schema->expects($this->once()) ->method('getTables') - ->willReturn([]); + ->willReturn([$table]); $schema->expects($this->once()) ->method('getSequences') - ->willReturn([$sequence]); + ->willReturn([]); $sourceSchema = $this->createMock(Schema::class); $sourceSchema->expects($this->any()) @@ -700,9 +834,13 @@ public function testEnsureOracleConstraintsTooLongSequenceName(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); - } + $logger = $this->createMock(LoggerInterface::class); + $logger->expects(self::once()) + ->method('error'); + $this->overwriteService(LoggerInterface::class, $logger); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); + } #[TestWith([true])] #[TestWith([false])] @@ -724,7 +862,7 @@ public function testEnsureOracleConstraintsBooleanNotNull(bool $isOracle): void $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn('tablename'); $table->method('getIndexes')->willReturn([]); $table->method('getForeignKeys')->willReturn([]); @@ -755,10 +893,9 @@ public function testEnsureOracleConstraintsBooleanNotNull(bool $isOracle): void ->method('setNotnull'); } - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } - public function testEnsureOracleConstraintsStringLength4000(): void { $this->expectException(\InvalidArgumentException::class); @@ -776,7 +913,7 @@ public function testEnsureOracleConstraintsStringLength4000(): void { $table = $this->createMock(Table::class); $table->expects($this->any()) ->method('getName') - ->willReturn(\str_repeat('a', 30)); + ->willReturn('tablename'); $table->expects($this->once()) ->method('getColumns') @@ -795,164 +932,6 @@ public function testEnsureOracleConstraintsStringLength4000(): void { ->method('hasSequence') ->willReturn(false); - self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]); - } - - - public function testExtractMigrationAttributes(): void { - $metadataManager = Server::get(MetadataManager::class); - $this->appManager->loadApp('testing'); - - $this->assertEquals($this->getMigrationMetadata(), json_decode(json_encode($metadataManager->extractMigrationAttributes('testing')), true)); - - $this->appManager->disableApp('testing'); - } - - public function testDeserializeMigrationMetadata(): void { - $metadataManager = Server::get(MetadataManager::class); - $this->assertEquals( - [ - 'core' => [], - 'apps' => [ - 'testing' => [ - '30000Date20240102030405' => [ - new DropTable('old_table'), - new CreateTable('new_table', - description: 'Table is used to store things, but also to get more things', - notes: ['this is a notice', 'and another one, if really needed'] - ), - new AddColumn('my_table'), - new AddColumn('my_table', 'another_field'), - new AddColumn('other_table', 'last_one', ColumnType::DATE), - new AddIndex('my_table'), - new AddIndex('my_table', IndexType::PRIMARY), - new DropColumn('other_table'), - new DropColumn('other_table', 'old_column', - description: 'field is not used anymore and replaced by \'last_one\'' - ), - new DropIndex('other_table'), - new ModifyColumn('other_table'), - new ModifyColumn('other_table', 'this_field'), - new ModifyColumn('other_table', 'this_field', ColumnType::BIGINT) - ] - ] - ] - ], - $metadataManager->getMigrationsAttributesFromReleaseMetadata( - [ - 'core' => [], - 'apps' => ['testing' => $this->getMigrationMetadata()] - ] - ) - ); - } - - private function getMigrationMetadata(): array { - return [ - '30000Date20240102030405' => [ - [ - 'class' => 'OCP\\Migration\\Attributes\\DropTable', - 'table' => 'old_table', - 'description' => '', - 'notes' => [], - 'columns' => [] - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\CreateTable', - 'table' => 'new_table', - 'description' => 'Table is used to store things, but also to get more things', - 'notes' => [ - 'this is a notice', - 'and another one, if really needed' - ], - 'columns' => [] - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\AddColumn', - 'table' => 'my_table', - 'description' => '', - 'notes' => [], - 'name' => '', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\AddColumn', - 'table' => 'my_table', - 'description' => '', - 'notes' => [], - 'name' => 'another_field', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\AddColumn', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'name' => 'last_one', - 'type' => 'date' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\AddIndex', - 'table' => 'my_table', - 'description' => '', - 'notes' => [], - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\AddIndex', - 'table' => 'my_table', - 'description' => '', - 'notes' => [], - 'type' => 'primary' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\DropColumn', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'name' => '', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\DropColumn', - 'table' => 'other_table', - 'description' => 'field is not used anymore and replaced by \'last_one\'', - 'notes' => [], - 'name' => 'old_column', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\DropIndex', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'name' => '', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'name' => 'this_field', - 'type' => '' - ], - [ - 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', - 'table' => 'other_table', - 'description' => '', - 'notes' => [], - 'name' => 'this_field', - 'type' => 'bigint' - ], - ] - ]; + $this->migrationService->ensureOracleConstraints($sourceSchema, $schema); } } diff --git a/tests/lib/Migration/MetadataManagerTest.php b/tests/lib/Migration/MetadataManagerTest.php new file mode 100644 index 0000000000000..43c9a6bcb72d6 --- /dev/null +++ b/tests/lib/Migration/MetadataManagerTest.php @@ -0,0 +1,196 @@ +appManager = Server::get(IAppManager::class); + } + + public function testExtractMigrationAttributes(): void { + $metadataManager = Server::get(MetadataManager::class); + $this->appManager->loadApp('testing'); + + $this->assertEquals( + self::getMigrationMetadata(), + json_decode(json_encode($metadataManager->extractMigrationAttributes('testing')), true), + ); + + $this->appManager->disableApp('testing'); + } + + public function testDeserializeMigrationMetadata(): void { + $metadataManager = Server::get(MetadataManager::class); + $this->assertEquals( + [ + 'core' => [], + 'apps' => [ + 'testing' => [ + '30000Date20240102030405' => [ + new DropTable('old_table'), + new CreateTable('new_table', + description: 'Table is used to store things, but also to get more things', + notes: ['this is a notice', 'and another one, if really needed'] + ), + new AddColumn('my_table'), + new AddColumn('my_table', 'another_field'), + new AddColumn('other_table', 'last_one', ColumnType::DATE), + new AddIndex('my_table'), + new AddIndex('my_table', IndexType::PRIMARY), + new DropColumn('other_table'), + new DropColumn('other_table', 'old_column', + description: 'field is not used anymore and replaced by \'last_one\'' + ), + new DropIndex('other_table'), + new ModifyColumn('other_table'), + new ModifyColumn('other_table', 'this_field'), + new ModifyColumn('other_table', 'this_field', ColumnType::BIGINT) + ] + ] + ] + ], + $metadataManager->getMigrationsAttributesFromReleaseMetadata( + [ + 'core' => [], + 'apps' => ['testing' => self::getMigrationMetadata()] + ] + ) + ); + } + + private static function getMigrationMetadata(): array { + return [ + '30000Date20240102030405' => [ + [ + 'class' => 'OCP\\Migration\\Attributes\\DropTable', + 'table' => 'old_table', + 'description' => '', + 'notes' => [], + 'columns' => [] + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\CreateTable', + 'table' => 'new_table', + 'description' => 'Table is used to store things, but also to get more things', + 'notes' + => [ + 'this is a notice', + 'and another one, if really needed' + ], + 'columns' => [] + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\AddColumn', + 'table' => 'my_table', + 'description' => '', + 'notes' => [], + 'name' => '', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\AddColumn', + 'table' => 'my_table', + 'description' => '', + 'notes' => [], + 'name' => 'another_field', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\AddColumn', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'name' => 'last_one', + 'type' => 'date' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\AddIndex', + 'table' => 'my_table', + 'description' => '', + 'notes' => [], + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\AddIndex', + 'table' => 'my_table', + 'description' => '', + 'notes' => [], + 'type' => 'primary' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\DropColumn', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'name' => '', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\DropColumn', + 'table' => 'other_table', + 'description' => 'field is not used anymore and replaced by \'last_one\'', + 'notes' => [], + 'name' => 'old_column', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\DropIndex', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'name' => '', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'name' => 'this_field', + 'type' => '' + ], + [ + 'class' => 'OCP\\Migration\\Attributes\\ModifyColumn', + 'table' => 'other_table', + 'description' => '', + 'notes' => [], + 'name' => 'this_field', + 'type' => 'bigint' + ], + ] + ]; + } +}