mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #24055 from nextcloud/bugfix/noid/enfore-no-notnull-for-boolean-to-store-false 

Enforce no notnull for boolean to store false
This commit is contained in:
Morris Jobke 2021-04-01 18:30:26 +02:00 committed by GitHub
parent 507facda8b f9d4fa2d38
commit 5fb909faa5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 88 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
lib/private/DB/MigrationService.php
View File

@ -459,7 +459,7 @@ class MigrationService {
$targetSchema = $toSchema->getWrappedSchema();
if ($this->checkOracle) {
$beforeSchema = $this->connection->createSchema();
$this->ensureOracleIdentifierLengthLimit($beforeSchema, $targetSchema, strlen($this->connection->getPrefix()));
$this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix()));
}
$this->connection->migrateToSchema($targetSchema);
$toSchema->performDropTableCalls();
@ -536,7 +536,7 @@ class MigrationService {
$targetSchema = $toSchema->getWrappedSchema();
if ($this->checkOracle) {
$sourceSchema = $this->connection->createSchema();
$this->ensureOracleIdentifierLengthLimit($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
$this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
}
$this->connection->migrateToSchema($targetSchema);
$toSchema->performDropTableCalls();
@ -551,7 +551,25 @@ class MigrationService {
$this->markAsExecuted($version);
}
public function ensureOracleIdentifierLengthLimit(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {
/**
* Naming constraints:
* - Tables names must be 30 chars or shorter (27 + oc_ prefix)
* - Column names must be 30 chars or shorter
* - Index names must be 30 chars or shorter
* - Sequence names must be 30 chars or shorter
* - Primary key names must be set or the table name 23 chars or shorter
*
* Data constraints:
* - Columns with "NotNull" can not have empty string as default value
* - Columns with "NotNull" can not have number 0 as default value
* - Columns with type "bool" (which is in fact integer of length 1) can not be "NotNull" as it can not store 0/false
*
* @param Schema $sourceSchema
* @param Schema $targetSchema
* @param int $prefixLength
* @throws \Doctrine\DBAL\Exception
*/
public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {
$sequences = $targetSchema->getSequences();
foreach ($targetSchema->getTables() as $table) {
@ -571,7 +589,11 @@ class MigrationService {
if ($thing->getNotnull() && $thing->getDefault() === ''
&& $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) {
throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
}
if ($thing->getNotnull() && $thing->getType()->getName() === Types::BOOLEAN) {
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type Bool and also NotNull, so it can not store "false".');
}
}
@ -583,7 +605,7 @@ class MigrationService {
foreach ($table->getForeignKeys() as $thing) {
if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) && \strlen($thing->getName()) > 30) {
throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
}
}

81
tests/lib/DB/MigrationsTest.php
View File

@ -18,6 +18,7 @@ use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\SchemaException;
use Doctrine\DBAL\Schema\Sequence;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\Type;
use OC\DB\Connection;
use OC\DB\MigrationService;
use OC\DB\SchemaWrapper;
@ -219,7 +220,7 @@ class MigrationsTest extends \Test\TestCase {
$this->migrationService->migrate();
}
public function testEnsureOracleIdentifierLengthLimitValid() {
public function testEnsureOracleConstraintsValid() {
$column = $this->createMock(Column::class);
$column->expects($this->once())
->method('getName')
@ -274,10 +275,10 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKey() {
public function testEnsureOracleConstraintsValidWithPrimaryKey() {
$index = $this->createMock(Index::class);
$index->expects($this->any())
->method('getName')
@ -317,10 +318,10 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKeyDefault() {
public function testEnsureOracleConstraintsValidWithPrimaryKeyDefault() {
$defaultName = 'PRIMARY';
if ($this->db->getDatabasePlatform() instanceof PostgreSqlPlatform) {
$defaultName = \str_repeat('a', 26) . '_' . \str_repeat('b', 30) . '_seq';
@ -370,11 +371,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongTableName() {
public function testEnsureOracleConstraintsTooLongTableName() {
$this->expectException(\InvalidArgumentException::class);
$table = $this->createMock(Table::class);
@ -395,11 +396,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithDefault() {
public function testEnsureOracleConstraintsTooLongPrimaryWithDefault() {
$this->expectException(\InvalidArgumentException::class);
$defaultName = 'PRIMARY';
@ -448,11 +449,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithName() {
public function testEnsureOracleConstraintsTooLongPrimaryWithName() {
$this->expectException(\InvalidArgumentException::class);
$index = $this->createMock(Index::class);
@ -491,11 +492,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongColumnName() {
public function testEnsureOracleConstraintsTooLongColumnName() {
$this->expectException(\InvalidArgumentException::class);
$column = $this->createMock(Column::class);
@ -525,11 +526,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongIndexName() {
public function testEnsureOracleConstraintsTooLongIndexName() {
$this->expectException(\InvalidArgumentException::class);
$index = $this->createMock(Index::class);
@ -562,11 +563,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongForeignKeyName() {
public function testEnsureOracleConstraintsTooLongForeignKeyName() {
$this->expectException(\InvalidArgumentException::class);
$foreignKey = $this->createMock(ForeignKeyConstraint::class);
@ -602,11 +603,11 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleIdentifierLengthLimitTooLongSequenceName() {
public function testEnsureOracleConstraintsTooLongSequenceName() {
$this->expectException(\InvalidArgumentException::class);
$sequence = $this->createMock(Sequence::class);
@ -630,6 +631,46 @@ class MigrationsTest extends \Test\TestCase {
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
public function testEnsureOracleConstraintsBooleanNotNull() {
$this->expectException(\InvalidArgumentException::class);
$column = $this->createMock(Column::class);
$column->expects($this->any())
->method('getName')
->willReturn('aaaa');
$column->expects($this->any())
->method('getType')
->willReturn(Type::getType('boolean'));
$column->expects($this->any())
->method('getNotnull')
->willReturn(true);
$table = $this->createMock(Table::class);
$table->expects($this->any())
->method('getName')
->willReturn(\str_repeat('a', 30));
$table->expects($this->once())
->method('getColumns')
->willReturn([$column]);
$schema = $this->createMock(Schema::class);
$schema->expects($this->once())
->method('getTables')
->willReturn([$table]);
$sourceSchema = $this->createMock(Schema::class);
$sourceSchema->expects($this->any())
->method('getTable')
->willThrowException(new SchemaException());
$sourceSchema->expects($this->any())
->method('hasSequence')
->willReturn(false);
self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
}
}