From 3696ef5b965e5d3e44197479eaf310e26288151c Mon Sep 17 00:00:00 2001
From: Joas Schilling <coding@schilljs.com>
Date: Wed, 11 Nov 2020 14:34:24 +0100
Subject: [PATCH] Don't allow Notnull for boolean columns

Signed-off-by: Joas Schilling <coding@schilljs.com>
---
 lib/private/DB/MigrationService.php |  5 ++++
 tests/lib/DB/MigrationsTest.php     | 41 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php
index 852ee8b701..4957706bb1 100644
--- a/lib/private/DB/MigrationService.php
+++ b/lib/private/DB/MigrationService.php
@@ -562,6 +562,7 @@ class MigrationService {
 	 * 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
@@ -590,6 +591,10 @@ class MigrationService {
 					&& $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 ($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".');
+				}
 			}
 
 			foreach ($table->getIndexes() as $thing) {
diff --git a/tests/lib/DB/MigrationsTest.php b/tests/lib/DB/MigrationsTest.php
index 9b6dec006d..c8b1af3e08 100644
--- a/tests/lib/DB/MigrationsTest.php
+++ b/tests/lib/DB/MigrationsTest.php
@@ -18,6 +18,7 @@ use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaException;
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Types\Type;
 use OC\DB\Connection;
 use OC\DB\MigrationService;
 use OC\DB\SchemaWrapper;
@@ -632,4 +633,44 @@ class MigrationsTest extends \Test\TestCase {
 
 		self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
 	}
+
+
+	public function testEnsureOracleIdentifierLengthLimitBooleanNotNull() {
+		$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, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
+	}
 }