From cdf4af5f2747aafd3c24b04799c37650dd7a3d42 Mon Sep 17 00:00:00 2001 From: Alexander Guz Date: Wed, 16 Nov 2016 17:56:07 +0100 Subject: [PATCH 1/4] Added unit test for boolean option values. It fail now. In `XmlDriver::_parseOptions` we need somehow to maintain a list of options, that are supposed to be boolean, and then call `$this->evaluateBoolean()` on them. --- .../Tests/ORM/Mapping/XmlMappingDriverTest.php | 11 +++++++++-- .../xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index 735c16ed5..213eba30b 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -34,7 +34,7 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest } /** - * @expectedException Doctrine\ORM\Cache\CacheException + * @expectedException \Doctrine\ORM\Cache\CacheException * @expectedExceptionMessage Entity association field "Doctrine\Tests\ORM\Mapping\XMLSLC#foo" not configured as part of the second-level cache. */ public function testFailingSecondLevelCacheAssociation() @@ -132,7 +132,7 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest /** * @group DDC-1468 * - * @expectedException Doctrine\Common\Persistence\Mapping\MappingException + * @expectedException \Doctrine\Common\Persistence\Mapping\MappingException * @expectedExceptionMessage Invalid mapping file 'Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml' for class 'Doctrine\Tests\Models\Generic\SerializationModel'. */ public function testInvalidMappingFileException() @@ -140,6 +140,13 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest $this->createClassMetadata('Doctrine\Tests\Models\Generic\SerializationModel'); } + public function testBooleanValuesForOptionIsSetCorrectly() + { + $class = $this->createClassMetadata('Doctrine\Tests\ORM\Mapping\User'); + $this->assertInternalType('bool', $class->fieldMappings['name']['options']['bool_opt']); + $this->assertFalse($class->fieldMappings['name']['options']['bool_opt']); + } + /** * @param string $xmlMappingFile * @dataProvider dataValidSchema diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml index 28b1e0571..a4a0fcef7 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml @@ -50,6 +50,7 @@ + From 8580f02c6ac78d1ec3811d43025792abb21d4d47 Mon Sep 17 00:00:00 2001 From: Alexander Guz Date: Wed, 16 Nov 2016 18:07:58 +0100 Subject: [PATCH 2/4] #6129 Use User::class to get metadata instead of string. --- tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index 213eba30b..b8d638e78 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -140,9 +140,12 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest $this->createClassMetadata('Doctrine\Tests\Models\Generic\SerializationModel'); } + /** + * @group #6129 + */ public function testBooleanValuesForOptionIsSetCorrectly() { - $class = $this->createClassMetadata('Doctrine\Tests\ORM\Mapping\User'); + $class = $this->createClassMetadata(User::class); $this->assertInternalType('bool', $class->fieldMappings['name']['options']['bool_opt']); $this->assertFalse($class->fieldMappings['name']['options']['bool_opt']); } From 7bf206adb46dcb2fba3d070caac483d8ff7888b1 Mon Sep 17 00:00:00 2001 From: Alexander Guz Date: Wed, 16 Nov 2016 20:01:11 +0100 Subject: [PATCH 3/4] #6129 Moved test to AbstractMappingDriverTest. --- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 7 +++- .../ORM/Mapping/AbstractMappingDriverTest.php | 39 +++++++++++++++---- .../ORM/Mapping/XmlMappingDriverTest.php | 10 ----- .../php/Doctrine.Tests.ORM.Mapping.User.php | 4 +- .../Doctrine.Tests.ORM.Mapping.User.dcm.xml | 3 +- .../Doctrine.Tests.ORM.Mapping.User.dcm.yml | 2 + 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index 00de4f2ea..90fba4abf 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -653,6 +653,8 @@ class XmlDriver extends FileDriver { $array = array(); + $booleanOptions = ['unsigned', 'fixed']; + /* @var $option SimpleXMLElement */ foreach ($options as $option) { if ($option->count()) { @@ -664,7 +666,10 @@ class XmlDriver extends FileDriver $attr = $option->attributes(); if (isset($attr->name)) { - $array[(string) $attr->name] = $value; + $attrName = (string) $attr->name; + $array[$attrName] = in_array($attrName, $booleanOptions) + ? $this->evaluateBoolean($value) + : $value; } else { $array[] = $value; } diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index 8d90931f9..3cc86c77a 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -210,11 +210,14 @@ abstract class AbstractMappingDriverTest extends OrmTestCase /** * @depends testEntityTableNameAndInheritance + * * @param ClassMetadata $class + * + * @return ClassMetadata */ - public function testFieldOptions($class) + public function testFieldOptions(ClassMetadata $class) { - $expected = array('foo' => 'bar', 'baz' => array('key' => 'val')); + $expected = ['foo' => 'bar', 'baz' => ['key' => 'val'], 'fixed' => false]; $this->assertEquals($expected, $class->fieldMappings['name']['options']); return $class; @@ -226,7 +229,7 @@ abstract class AbstractMappingDriverTest extends OrmTestCase */ public function testIdFieldOptions($class) { - $this->assertEquals(array('foo' => 'bar'), $class->fieldMappings['id']['options']); + $this->assertEquals(['foo' => 'bar', 'unsigned' => false], $class->fieldMappings['id']['options']); return $class; } @@ -244,6 +247,26 @@ abstract class AbstractMappingDriverTest extends OrmTestCase return $class; } + /** + * @group #6129 + * + * @depends testIdentifier + * + * @param ClassMetadata $class + * + * @return ClassMetadata + */ + public function testBooleanValuesForOptionIsSetCorrectly(ClassMetadata $class) + { + $this->assertInternalType('bool', $class->fieldMappings['id']['options']['unsigned']); + $this->assertFalse($class->fieldMappings['id']['options']['unsigned']); + + $this->assertInternalType('bool', $class->fieldMappings['name']['options']['fixed']); + $this->assertFalse($class->fieldMappings['name']['options']['fixed']); + + return $class; + } + /** * @depends testIdentifier * @param ClassMetadata $class @@ -1048,14 +1071,14 @@ class User { /** * @Id - * @Column(type="integer", options={"foo": "bar"}) + * @Column(type="integer", options={"foo": "bar", "unsigned": false}) * @generatedValue(strategy="AUTO") * @SequenceGenerator(sequenceName="tablename_seq", initialValue=1, allocationSize=100) **/ public $id; /** - * @Column(length=50, nullable=true, unique=true, options={"foo": "bar", "baz": {"key": "val"}}) + * @Column(length=50, nullable=true, unique=true, options={"foo": "bar", "baz": {"key": "val"}, "fixed": false}) */ public $name; @@ -1129,7 +1152,7 @@ class User 'fieldName' => 'id', 'type' => 'integer', 'columnName' => 'id', - 'options' => array('foo' => 'bar'), + 'options' => array('foo' => 'bar', 'unsigned' => false), )); $metadata->mapField(array( 'fieldName' => 'name', @@ -1138,7 +1161,7 @@ class User 'unique' => true, 'nullable' => true, 'columnName' => 'name', - 'options' => array('foo' => 'bar', 'baz' => array('key' => 'val')), + 'options' => array('foo' => 'bar', 'baz' => array('key' => 'val'), 'fixed' => false), )); $metadata->mapField(array( 'fieldName' => 'email', @@ -1474,4 +1497,4 @@ class SingleTableEntityIncompleteDiscriminatorColumnMapping class SingleTableEntityIncompleteDiscriminatorColumnMappingSub1 extends SingleTableEntityIncompleteDiscriminatorColumnMapping {} class SingleTableEntityIncompleteDiscriminatorColumnMappingSub2 - extends SingleTableEntityIncompleteDiscriminatorColumnMapping {} \ No newline at end of file + extends SingleTableEntityIncompleteDiscriminatorColumnMapping {} diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index b8d638e78..ae188057d 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -140,16 +140,6 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest $this->createClassMetadata('Doctrine\Tests\Models\Generic\SerializationModel'); } - /** - * @group #6129 - */ - public function testBooleanValuesForOptionIsSetCorrectly() - { - $class = $this->createClassMetadata(User::class); - $this->assertInternalType('bool', $class->fieldMappings['name']['options']['bool_opt']); - $this->assertFalse($class->fieldMappings['name']['options']['bool_opt']); - } - /** * @param string $xmlMappingFile * @dataProvider dataValidSchema diff --git a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.User.php b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.User.php index 67dbd752d..5bcc6d029 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.User.php +++ b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.User.php @@ -19,7 +19,7 @@ $metadata->mapField(array( 'fieldName' => 'id', 'type' => 'integer', 'columnName' => 'id', - 'options' => array('foo' => 'bar'), + 'options' => array('foo' => 'bar', 'unsigned' => false), )); $metadata->mapField(array( 'fieldName' => 'name', @@ -28,7 +28,7 @@ $metadata->mapField(array( 'unique' => true, 'nullable' => true, 'columnName' => 'name', - 'options' => array('foo' => 'bar', 'baz' => array('key' => 'val')), + 'options' => array('foo' => 'bar', 'baz' => array('key' => 'val'), 'fixed' => false), )); $metadata->mapField(array( 'fieldName' => 'email', diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml index a4a0fcef7..bbf29aeaf 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml @@ -41,6 +41,7 @@ + @@ -50,7 +51,7 @@ - + diff --git a/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml index 457b24eea..e3a32ce76 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml +++ b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml @@ -18,6 +18,7 @@ Doctrine\Tests\ORM\Mapping\User: initialValue: 1 options: foo: bar + unsigned: false fields: name: type: string @@ -28,6 +29,7 @@ Doctrine\Tests\ORM\Mapping\User: foo: bar baz: key: val + fixed: false email: type: string column: user_email From 8d433cdb394eaaabcf0a97908b47885b48a101fb Mon Sep 17 00:00:00 2001 From: Alexander Guz Date: Thu, 17 Nov 2016 21:05:58 +0100 Subject: [PATCH 4/4] #6129 Fixed code style and @depends in test. --- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 10 ++++------ .../Tests/ORM/Mapping/AbstractMappingDriverTest.php | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index 90fba4abf..3d8499180 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -653,8 +653,6 @@ class XmlDriver extends FileDriver { $array = array(); - $booleanOptions = ['unsigned', 'fixed']; - /* @var $option SimpleXMLElement */ foreach ($options as $option) { if ($option->count()) { @@ -663,11 +661,11 @@ class XmlDriver extends FileDriver $value = (string) $option; } - $attr = $option->attributes(); + $attributes = $option->attributes(); - if (isset($attr->name)) { - $attrName = (string) $attr->name; - $array[$attrName] = in_array($attrName, $booleanOptions) + if (isset($attributes->name)) { + $nameAttribute = (string) $attributes->name; + $array[$nameAttribute] = in_array($nameAttribute, ['unsigned', 'fixed']) ? $this->evaluateBoolean($value) : $value; } else { diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index 3cc86c77a..c4fd1381d 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -250,7 +250,7 @@ abstract class AbstractMappingDriverTest extends OrmTestCase /** * @group #6129 * - * @depends testIdentifier + * @depends testLoadMapping * * @param ClassMetadata $class *