From ee8dc496d915f71f58246f5dd6585c5a22a32eec Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Sun, 19 Nov 2017 00:57:54 +0100 Subject: [PATCH 1/2] Fix applying collation on foreign key columns --- lib/Doctrine/ORM/Tools/SchemaTool.php | 38 ++++++++++-------- .../Tests/Models/Forum/ForumCategory.php | 2 +- .../Tests/ORM/Tools/SchemaToolTest.php | 40 +++++++++++++++++++ 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 1950446e7c8..423ed3b5320 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -45,6 +45,8 @@ */ class SchemaTool { + private const KNOWN_COLUMN_OPTIONS = ['comment', 'unsigned', 'fixed', 'default']; + /** * @var \Doctrine\ORM\EntityManagerInterface */ @@ -467,19 +469,8 @@ private function gatherColumn($class, array $mapping, Table $table) $options['columnDefinition'] = $mapping['columnDefinition']; } - if (isset($mapping['options'])) { - $knownOptions = ['comment', 'unsigned', 'fixed', 'default']; - - foreach ($knownOptions as $knownOption) { - if (array_key_exists($knownOption, $mapping['options'])) { - $options[$knownOption] = $mapping['options'][$knownOption]; - - unset($mapping['options'][$knownOption]); - } - } - - $options['customSchemaOptions'] = $mapping['options']; - } + // the 'default' option can be overwritten here + $options = $this->gatherColumnOptions($mapping) + $options; if ($class->isIdGeneratorIdentity() && $class->getIdentifierFieldNames() == [$mapping['fieldName']]) { $options['autoincrement'] = true; @@ -690,9 +681,7 @@ private function gatherRelationJoinColumns( $columnOptions['notnull'] = ! $joinColumn['nullable']; } - if (isset($fieldMapping['options'])) { - $columnOptions['options'] = $fieldMapping['options']; - } + $columnOptions = $columnOptions + $this->gatherColumnOptions($fieldMapping); if ($fieldMapping['type'] == "string" && isset($fieldMapping['length'])) { $columnOptions['length'] = $fieldMapping['length']; @@ -745,6 +734,23 @@ private function gatherRelationJoinColumns( } } + /** + * @param mixed[] $mapping + * + * @return mixed[] + */ + private function gatherColumnOptions(array $mapping) : array + { + if (! isset($mapping['options'])) { + return []; + } + + $options = array_intersect_key($mapping['options'], array_flip(self::KNOWN_COLUMN_OPTIONS)); + $options['customSchemaOptions'] = array_diff_key($mapping['options'], $options); + + return $options; + } + /** * Drops the database schema for the given classes. * diff --git a/tests/Doctrine/Tests/Models/Forum/ForumCategory.php b/tests/Doctrine/Tests/Models/Forum/ForumCategory.php index f04f128dce8..6a268d78cc7 100644 --- a/tests/Doctrine/Tests/Models/Forum/ForumCategory.php +++ b/tests/Doctrine/Tests/Models/Forum/ForumCategory.php @@ -9,7 +9,7 @@ class ForumCategory { /** - * @Column(type="integer") + * @Column(type="string", length=8, options={"fixed":true, "collation":"latin1_bin", "foo":"bar"}) * @Id */ private $id; diff --git a/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php b/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php index 6750442e8a6..29ed1e9c884 100644 --- a/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php @@ -18,6 +18,8 @@ use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedDerivedIdentityClass; use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedDerivedRootClass; use Doctrine\Tests\Models\Forum\ForumAvatar; +use Doctrine\Tests\Models\Forum\ForumBoard; +use Doctrine\Tests\Models\Forum\ForumCategory; use Doctrine\Tests\Models\Forum\ForumUser; use Doctrine\Tests\Models\NullDefault\NullDefaultColumn; use Doctrine\Tests\OrmTestCase; @@ -86,6 +88,44 @@ public function testPassColumnDefinitionToJoinColumn() $this->assertEquals($customColumnDef, $table->getColumn('avatar_id')->getColumnDefinition()); } + public function testPassColumnOptionsToJoinColumn() + { + $em = $this->_getTestEntityManager(); + $schemaTool = new SchemaTool($em); + + $category = $em->getClassMetadata(ForumCategory::class); + $board = $em->getClassMetadata(ForumBoard::class); + + $classes = [$category, $board]; + + $schema = $schemaTool->getSchemaFromMetadata($classes); + + self::assertTrue($schema->hasTable('forum_categories')); + self::assertTrue($schema->hasTable('forum_boards')); + + $tableCategory = $schema->getTable('forum_categories'); + $tableBoard = $schema->getTable('forum_boards'); + + self::assertTrue($tableBoard->hasColumn('category_id')); + + self::assertSame( + $tableCategory->getColumn('id')->getFixed(), + $tableBoard->getColumn('category_id')->getFixed(), + 'Foreign key/join column should have the same value of option `fixed` as the referenced column' + ); + + self::assertEquals( + $tableCategory->getColumn('id')->getCustomSchemaOptions(), + $tableBoard->getColumn('category_id')->getCustomSchemaOptions(), + 'Foreign key/join column should have the same custom options as the referenced column' + ); + + self::assertEquals( + ['collation' => 'latin1_bin', 'foo' => 'bar'], + $tableBoard->getColumn('category_id')->getCustomSchemaOptions() + ); + } + /** * @group DDC-283 */ From 0be52b0087938912a262bddd7656bc7ee227fd45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 21 Nov 2018 00:20:20 +0100 Subject: [PATCH 2/2] Isolate entities used by the new test To ensure we don't have any unintended side-effect. --- .../Tests/Models/Forum/ForumCategory.php | 2 +- .../Tests/ORM/Tools/SchemaToolTest.php | 61 +++++++++++++++---- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/tests/Doctrine/Tests/Models/Forum/ForumCategory.php b/tests/Doctrine/Tests/Models/Forum/ForumCategory.php index 6a268d78cc7..f04f128dce8 100644 --- a/tests/Doctrine/Tests/Models/Forum/ForumCategory.php +++ b/tests/Doctrine/Tests/Models/Forum/ForumCategory.php @@ -9,7 +9,7 @@ class ForumCategory { /** - * @Column(type="string", length=8, options={"fixed":true, "collation":"latin1_bin", "foo":"bar"}) + * @Column(type="integer") * @Id */ private $id; diff --git a/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php b/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php index 29ed1e9c884..ea520d8fc73 100644 --- a/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php @@ -88,23 +88,23 @@ public function testPassColumnDefinitionToJoinColumn() $this->assertEquals($customColumnDef, $table->getColumn('avatar_id')->getColumnDefinition()); } - public function testPassColumnOptionsToJoinColumn() + /** + * @group 6830 + */ + public function testPassColumnOptionsToJoinColumn() : void { $em = $this->_getTestEntityManager(); - $schemaTool = new SchemaTool($em); - - $category = $em->getClassMetadata(ForumCategory::class); - $board = $em->getClassMetadata(ForumBoard::class); - - $classes = [$category, $board]; + $category = $em->getClassMetadata(GH6830Category::class); + $board = $em->getClassMetadata(GH6830Board::class); - $schema = $schemaTool->getSchemaFromMetadata($classes); + $schemaTool = new SchemaTool($em); + $schema = $schemaTool->getSchemaFromMetadata([$category, $board]); - self::assertTrue($schema->hasTable('forum_categories')); - self::assertTrue($schema->hasTable('forum_boards')); + self::assertTrue($schema->hasTable('GH6830Category')); + self::assertTrue($schema->hasTable('GH6830Board')); - $tableCategory = $schema->getTable('forum_categories'); - $tableBoard = $schema->getTable('forum_boards'); + $tableCategory = $schema->getTable('GH6830Category'); + $tableBoard = $schema->getTable('GH6830Board'); self::assertTrue($tableBoard->hasColumn('category_id')); @@ -362,3 +362,40 @@ class SecondEntity */ public $name; } + +/** + * @Entity + */ +class GH6830Board +{ + /** + * @Id + * @Column(type="integer") + */ + public $id; + + /** + * @ManyToOne(targetEntity=GH6830Category::class, inversedBy="boards") + * @JoinColumn(name="category_id", referencedColumnName="id") + */ + public $category; +} + +/** + * @Entity + */ +class GH6830Category +{ + /** + * @Id + * @Column(type="string", length=8, options={"fixed":true, "collation":"latin1_bin", "foo":"bar"}) + * + * @var string + */ + public $id; + + /** + * @OneToMany(targetEntity=GH6830Board::class, mappedBy="category") + */ + public $boards; +}