From 67eef32b07117653c8e59ede7be5b8d869974cc5 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 19 Mar 2025 21:03:11 +0100 Subject: [PATCH] fix(DB): support up to 63 character long table and index names We do not support Oracle 11 anymore but at least Oracle 12c (12.2). So the limitation is gone (Oracle now supports up to 128 character long names). Instead we are now limited by MySQL (64 characters) and PostgreSQL (63 characters). Signed-off-by: Ferdinand Thiessen --- build/psalm-baseline.xml | 8 - lib/private/DB/MigrationService.php | 211 +++-- ...tionsTest.php => MigrationServiceTest.php} | 765 +++++++++--------- tests/lib/Migration/MetadataManagerTest.php | 196 +++++ 4 files changed, 707 insertions(+), 473 deletions(-) rename tests/lib/DB/{MigrationsTest.php => MigrationServiceTest.php} (59%) create mode 100644 tests/lib/Migration/MetadataManagerTest.php diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 983f7429873..c3fe00becb5 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3515,14 +3515,6 @@ - - - - - - - - functionBuilder->lower($x)]]> diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 985c331570f..9eeca432077 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -17,13 +17,14 @@ use OC\App\InfoParser; 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 +48,7 @@ class MigrationService { ?LoggerInterface $logger = null, ) { $this->appName = $appName; + $this->checkOracle = false; $this->connection = $connection; if ($logger === null) { $this->logger = Server::get(LoggerInterface::class); @@ -103,7 +105,7 @@ class MigrationService { 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 +284,7 @@ class MigrationService { /** * @param string $version */ - private function markAsExecuted($version) { + private function markAsExecuted($version): void { $this->connection->insertIfNotExist('*PREFIX*migrations', [ 'app' => $this->appName, 'version' => $version @@ -343,7 +345,7 @@ class MigrationService { $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 +360,7 @@ class MigrationService { if (count($m) === 0) { return '0'; } - $migrations = array_values($m); - return @end($migrations); + return @end($m); } /** @@ -431,10 +432,11 @@ class MigrationService { 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 +474,11 @@ class MigrationService { * @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 +486,9 @@ class MigrationService { } } + if (!$s instanceof IMigrationStep) { + throw new \InvalidArgumentException('Not a valid migration'); + } return $s; } @@ -497,7 +499,7 @@ class MigrationService { * @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 +514,11 @@ class MigrationService { 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 +534,108 @@ class MigrationService { } /** + * 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 +645,47 @@ class MigrationService { * - 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 +708,13 @@ class MigrationService { $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 da92261b850..c2c8cdc03cc 100644 --- a/tests/lib/DB/MigrationsTest.php +++ b/tests/lib/DB/MigrationServiceTest.php @@ -19,42 +19,34 @@ use Doctrine\DBAL\Types\Type; 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 @@ class MigrationsTest extends \Test\TestCase { } 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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { */ #[\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 @@ class MigrationsTest extends \Test\TestCase { $schema->expects($this->once()) ->method('getTables') ->willReturn([$table]); - $schema->expects($this->once()) + $schema->expects($this->atMost(1)) ->method('getSequences') ->willReturn([]); @@ -346,40 +367,59 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { ->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 @@ class MigrationsTest extends \Test\TestCase { $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 @@ class MigrationsTest extends \Test\TestCase { ->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 00000000000..43c9a6bcb72 --- /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' + ], + ] + ]; + } +}