From a5d18b438c7627e87794ced67d117062aa335fdf Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 17:11:59 +0300 Subject: [PATCH 1/7] Fixed discriminator serialization when base class is in the map --- src/JMS/Serializer/Metadata/ClassMetadata.php | 83 +++++++++++-------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/src/JMS/Serializer/Metadata/ClassMetadata.php b/src/JMS/Serializer/Metadata/ClassMetadata.php index 7ebf9a73b..795f103a0 100644 --- a/src/JMS/Serializer/Metadata/ClassMetadata.php +++ b/src/JMS/Serializer/Metadata/ClassMetadata.php @@ -61,6 +61,8 @@ public function setDiscriminator($fieldName, array $map, array $groups = array() $this->discriminatorFieldName = $fieldName; $this->discriminatorMap = $map; $this->discriminatorGroups = $groups; + + $this->handleDiscriminatorProperty(); } /** @@ -167,40 +169,7 @@ public function merge(MergeableInterface $object) $this->discriminatorBaseClass = $object->discriminatorBaseClass; } - if ($this->discriminatorMap && !$this->reflection->isAbstract()) { - if (false === $typeValue = array_search($this->name, $this->discriminatorMap, true)) { - throw new \LogicException(sprintf( - 'The sub-class "%s" is not listed in the discriminator of the base class "%s".', - $this->name, - $this->discriminatorBaseClass - )); - } - - $this->discriminatorValue = $typeValue; - - if (isset($this->propertyMetadata[$this->discriminatorFieldName]) - && !$this->propertyMetadata[$this->discriminatorFieldName] instanceof StaticPropertyMetadata - ) { - throw new \LogicException(sprintf( - 'The discriminator field name "%s" of the base-class "%s" conflicts with a regular property of the sub-class "%s".', - $this->discriminatorFieldName, - $this->discriminatorBaseClass, - $this->name - )); - } - - $discriminatorProperty = new StaticPropertyMetadata( - $this->name, - $this->discriminatorFieldName, - $typeValue, - $this->discriminatorGroups - ); - $discriminatorProperty->serializedName = $this->discriminatorFieldName; - $discriminatorProperty->xmlAttribute = $this->xmlDiscriminatorAttribute; - $discriminatorProperty->xmlElementCData = $this->xmlDiscriminatorCData; - $discriminatorProperty->xmlNamespace = $this->xmlDiscriminatorNamespace; - $this->propertyMetadata[$this->discriminatorFieldName] = $discriminatorProperty; - } + $this->handleDiscriminatorProperty(); $this->sortProperties(); } @@ -297,6 +266,52 @@ public function unserialize($str) parent::unserialize($parentStr); } + private function handleDiscriminatorProperty() + { + if ( + $this->discriminatorMap + && !$this->reflection->isAbstract() + && !$this->reflection->isInterface() + ) { + if ( + false === ($typeValue = array_search($this->name, $this->discriminatorMap, true)) + && $this->discriminatorBaseClass !== $this->name + ) { + throw new \LogicException(sprintf( + 'The sub-class "%s" is not listed in the discriminator of the base class "%s".', + $this->name, + $this->discriminatorBaseClass + )); + } + + $this->discriminatorValue = $typeValue; + + if (isset($this->propertyMetadata[$this->discriminatorFieldName]) + && !$this->propertyMetadata[$this->discriminatorFieldName] instanceof StaticPropertyMetadata + ) { + throw new \LogicException(sprintf( + 'The discriminator field name "%s" of the base-class "%s" conflicts with a regular property of the sub-class "%s".', + $this->discriminatorFieldName, + $this->discriminatorBaseClass, + $this->name + )); + } + + $discriminatorProperty = new StaticPropertyMetadata( + $this->name, + $this->discriminatorFieldName, + $typeValue, + $this->discriminatorGroups + ); + $discriminatorProperty->serializedName = $this->discriminatorFieldName; + $discriminatorProperty->xmlAttribute = $this->xmlDiscriminatorAttribute; + $discriminatorProperty->xmlElementCData = $this->xmlDiscriminatorCData; + $discriminatorProperty->xmlNamespace = $this->xmlDiscriminatorNamespace; + $this->propertyMetadata[$this->discriminatorFieldName] = $discriminatorProperty; + } + + } + private function sortProperties() { switch ($this->accessorOrder) { From 6e790948df5b44ff5aed0c9e79873b62d9d5aff3 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Fri, 27 Jul 2018 14:41:44 +0300 Subject: [PATCH 2/7] Added tests when parent class is included into discriminator map --- tests/Fixtures/Discriminator/ImagePost.php | 9 ++++++ tests/Fixtures/Discriminator/Post.php | 24 +++++++++++++++ tests/Metadata/Driver/BaseDriverTest.php | 29 +++++++++++++++++++ .../Driver/xml/Discriminator.ImagePost.xml | 5 ++++ .../Driver/xml/Discriminator.Post.xml | 8 +++++ .../Driver/yml/Discriminator.ImagePost.yml | 1 + .../Driver/yml/Discriminator.Post.yml | 10 +++++++ 7 files changed, 86 insertions(+) create mode 100644 tests/Fixtures/Discriminator/ImagePost.php create mode 100644 tests/Fixtures/Discriminator/Post.php create mode 100644 tests/Metadata/Driver/xml/Discriminator.ImagePost.xml create mode 100644 tests/Metadata/Driver/xml/Discriminator.Post.xml create mode 100644 tests/Metadata/Driver/yml/Discriminator.ImagePost.yml create mode 100644 tests/Metadata/Driver/yml/Discriminator.Post.yml diff --git a/tests/Fixtures/Discriminator/ImagePost.php b/tests/Fixtures/Discriminator/ImagePost.php new file mode 100644 index 000000000..749ca96e8 --- /dev/null +++ b/tests/Fixtures/Discriminator/ImagePost.php @@ -0,0 +1,9 @@ +title = $title; + } +} diff --git a/tests/Metadata/Driver/BaseDriverTest.php b/tests/Metadata/Driver/BaseDriverTest.php index d6d971fc8..c9cc15892 100644 --- a/tests/Metadata/Driver/BaseDriverTest.php +++ b/tests/Metadata/Driver/BaseDriverTest.php @@ -183,6 +183,23 @@ public function testLoadDiscriminator() ); } + public function testLoadDiscriminatorWhenParentIsInDiscriminatorMap() + { + /** @var ClassMetadata $m */ + $m = $this->getDriver()->loadMetadataForClass(new \ReflectionClass('JMS\Serializer\Tests\Fixtures\Discriminator\Post')); + + self::assertNotNull($m); + self::assertEquals('type', $m->discriminatorFieldName); + self::assertEquals($m->name, $m->discriminatorBaseClass); + self::assertEquals( + [ + 'post' => 'JMS\Serializer\Tests\Fixtures\Discriminator\Post', + 'image_post' => 'JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost', + ], + $m->discriminatorMap + ); + } + public function testLoadXmlDiscriminator() { /** @var $m ClassMetadata */ @@ -281,6 +298,18 @@ public function testLoadDiscriminatorSubClass() $this->assertEquals(array(), $m->discriminatorMap); } + public function testLoadDiscriminatorSubClassWhenParentIsInDiscriminatorMap() + { + /** @var ClassMetadata $m */ + $m = $this->getDriver()->loadMetadataForClass(new \ReflectionClass('JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost')); + + self::assertNotNull($m); + self::assertNull($m->discriminatorValue); + self::assertNull($m->discriminatorBaseClass); + self::assertNull($m->discriminatorFieldName); + self::assertEquals([], $m->discriminatorMap); + } + public function testLoadXmlObjectWithNamespacesMetadata() { $m = $this->getDriver()->loadMetadataForClass(new \ReflectionClass('JMS\Serializer\Tests\Fixtures\ObjectWithXmlNamespaces')); diff --git a/tests/Metadata/Driver/xml/Discriminator.ImagePost.xml b/tests/Metadata/Driver/xml/Discriminator.ImagePost.xml new file mode 100644 index 000000000..5e23103a3 --- /dev/null +++ b/tests/Metadata/Driver/xml/Discriminator.ImagePost.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/Metadata/Driver/xml/Discriminator.Post.xml b/tests/Metadata/Driver/xml/Discriminator.Post.xml new file mode 100644 index 000000000..d213a993d --- /dev/null +++ b/tests/Metadata/Driver/xml/Discriminator.Post.xml @@ -0,0 +1,8 @@ + + + + JMS\Serializer\Tests\Fixtures\Discriminator\Post + JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost + + + diff --git a/tests/Metadata/Driver/yml/Discriminator.ImagePost.yml b/tests/Metadata/Driver/yml/Discriminator.ImagePost.yml new file mode 100644 index 000000000..07b713ca1 --- /dev/null +++ b/tests/Metadata/Driver/yml/Discriminator.ImagePost.yml @@ -0,0 +1 @@ +JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost: { } diff --git a/tests/Metadata/Driver/yml/Discriminator.Post.yml b/tests/Metadata/Driver/yml/Discriminator.Post.yml new file mode 100644 index 000000000..c2f59ea1c --- /dev/null +++ b/tests/Metadata/Driver/yml/Discriminator.Post.yml @@ -0,0 +1,10 @@ +JMS\Serializer\Tests\Fixtures\Discriminator\Post: + discriminator: + field_name: type + map: + post: JMS\Serializer\Tests\Fixtures\Discriminator\Post + image_post: JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost + + properties: + title: + type: string From cec725cac70a37a0b1feb79184a738f85043aad5 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 16:40:09 +0300 Subject: [PATCH 3/7] Added serialization and deserialization tests --- tests/Serializer/BaseSerializationTest.php | 37 +++++++++++++++++++ tests/Serializer/JsonSerializationTest.php | 3 ++ tests/Serializer/xml/image_post.xml | 5 +++ .../xml/image_post_without_type.xml | 4 ++ tests/Serializer/xml/post.xml | 5 +++ 5 files changed, 54 insertions(+) create mode 100644 tests/Serializer/xml/image_post.xml create mode 100644 tests/Serializer/xml/image_post_without_type.xml create mode 100644 tests/Serializer/xml/post.xml diff --git a/tests/Serializer/BaseSerializationTest.php b/tests/Serializer/BaseSerializationTest.php index eae16d803..db4f48e82 100644 --- a/tests/Serializer/BaseSerializationTest.php +++ b/tests/Serializer/BaseSerializationTest.php @@ -47,7 +47,9 @@ use JMS\Serializer\Tests\Fixtures\CustomDeserializationObject; use JMS\Serializer\Tests\Fixtures\DateTimeArraysObject; use JMS\Serializer\Tests\Fixtures\Discriminator\Car; +use JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost; use JMS\Serializer\Tests\Fixtures\Discriminator\Moped; +use JMS\Serializer\Tests\Fixtures\Discriminator\Post; use JMS\Serializer\Tests\Fixtures\Garage; use JMS\Serializer\Tests\Fixtures\GetSetObject; use JMS\Serializer\Tests\Fixtures\GroupsObject; @@ -1237,6 +1239,14 @@ public function testPolymorphicObjects() $this->getContent('car'), $this->serialize(new Car(5)) ); + self::assertEquals( + $this->getContent('post'), + $this->serialize(new Post('Post Title')) + ); + self::assertEquals( + $this->getContent('image_post'), + $this->serialize(new ImagePost('Image Post Title')) + ); if ($this->hasDeserializer()) { $this->assertEquals( @@ -1265,6 +1275,33 @@ public function testPolymorphicObjects() ), 'Class is resolved correctly when concrete sub-class is used and no type is defined.' ); + + self::assertEquals( + new Post('Post Title'), + $this->deserialize( + $this->getContent('post'), + 'JMS\Serializer\Tests\Fixtures\Discriminator\Post' + ), + 'Class is resolved correctly when parent class is used and type is set.' + ); + + self::assertEquals( + new ImagePost('Image Post Title'), + $this->deserialize( + $this->getContent('image_post'), + 'JMS\Serializer\Tests\Fixtures\Discriminator\Post' + ), + 'Class is resolved correctly when least supertype is used.' + ); + + self::assertEquals( + new ImagePost('Image Post Title'), + $this->deserialize( + $this->getContent('image_post'), + 'JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost' + ), + 'Class is resolved correctly when concrete sub-class is used and no type is defined.' + ); } } diff --git a/tests/Serializer/JsonSerializationTest.php b/tests/Serializer/JsonSerializationTest.php index d335f423a..d19fd8bff 100644 --- a/tests/Serializer/JsonSerializationTest.php +++ b/tests/Serializer/JsonSerializationTest.php @@ -93,6 +93,9 @@ protected function getContent($key) $outputs['date_interval'] = '"PT45M"'; $outputs['car'] = '{"km":5,"type":"car"}'; $outputs['car_without_type'] = '{"km":5}'; + $outputs['post'] = '{"type":"post","title":"Post Title"}'; + $outputs['image_post'] = '{"type":"image_post","title":"Image Post Title"}'; + $outputs['image_post_without_type'] = '{"title":"Image Post Title"}'; $outputs['garage'] = '{"vehicles":[{"km":3,"type":"car"},{"km":1,"type":"moped"}]}'; $outputs['tree'] = '{"tree":{"children":[{"children":[{"children":[],"foo":"bar"}],"foo":"bar"}],"foo":"bar"}}'; $outputs['nullable_arrays'] = '{"empty_inline":[],"not_empty_inline":["not_empty_inline"],"empty_not_inline":[],"not_empty_not_inline":["not_empty_not_inline"],"empty_not_inline_skip":[],"not_empty_not_inline_skip":["not_empty_not_inline_skip"]}'; diff --git a/tests/Serializer/xml/image_post.xml b/tests/Serializer/xml/image_post.xml new file mode 100644 index 000000000..5b6616ade --- /dev/null +++ b/tests/Serializer/xml/image_post.xml @@ -0,0 +1,5 @@ + + + + <![CDATA[Image Post Title]]> + diff --git a/tests/Serializer/xml/image_post_without_type.xml b/tests/Serializer/xml/image_post_without_type.xml new file mode 100644 index 000000000..ea3826397 --- /dev/null +++ b/tests/Serializer/xml/image_post_without_type.xml @@ -0,0 +1,4 @@ + + + <![CDATA[Image Post Title]]> + diff --git a/tests/Serializer/xml/post.xml b/tests/Serializer/xml/post.xml new file mode 100644 index 000000000..17af3666d --- /dev/null +++ b/tests/Serializer/xml/post.xml @@ -0,0 +1,5 @@ + + + + <![CDATA[Post Title]]> + From 9d5967fac221726a40c14b128d8ab2d1c7707048 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 17:12:53 +0300 Subject: [PATCH 4/7] Fix yaml serialization, php metadata tests --- .../Driver/php/Discriminator.ImagePost.php | 7 +++++++ tests/Metadata/Driver/php/Discriminator.Post.php | 16 ++++++++++++++++ tests/Serializer/yml/image_post.yml | 2 ++ tests/Serializer/yml/image_post_without_type.yml | 1 + tests/Serializer/yml/post.yml | 2 ++ 5 files changed, 28 insertions(+) create mode 100644 tests/Metadata/Driver/php/Discriminator.ImagePost.php create mode 100644 tests/Metadata/Driver/php/Discriminator.Post.php create mode 100644 tests/Serializer/yml/image_post.yml create mode 100644 tests/Serializer/yml/image_post_without_type.yml create mode 100644 tests/Serializer/yml/post.yml diff --git a/tests/Metadata/Driver/php/Discriminator.ImagePost.php b/tests/Metadata/Driver/php/Discriminator.ImagePost.php new file mode 100644 index 000000000..22e36fe15 --- /dev/null +++ b/tests/Metadata/Driver/php/Discriminator.ImagePost.php @@ -0,0 +1,7 @@ +setDiscriminator('type', array( + 'post' => 'JMS\Serializer\Tests\Fixtures\Discriminator\Post', + 'image_post' => 'JMS\Serializer\Tests\Fixtures\Discriminator\ImagePost', +)); + +$title = new PropertyMetadata('JMS\Serializer\Tests\Fixtures\Discriminator\Post', 'title'); +$title->setType('string'); +$metadata->addPropertyMetadata($title); + +return $metadata; diff --git a/tests/Serializer/yml/image_post.yml b/tests/Serializer/yml/image_post.yml new file mode 100644 index 000000000..1b1945fca --- /dev/null +++ b/tests/Serializer/yml/image_post.yml @@ -0,0 +1,2 @@ +type: image_post +title: 'Image Post Title' diff --git a/tests/Serializer/yml/image_post_without_type.yml b/tests/Serializer/yml/image_post_without_type.yml new file mode 100644 index 000000000..3417e47ad --- /dev/null +++ b/tests/Serializer/yml/image_post_without_type.yml @@ -0,0 +1 @@ +title: 'Image Post Title' diff --git a/tests/Serializer/yml/post.yml b/tests/Serializer/yml/post.yml new file mode 100644 index 000000000..54f042b51 --- /dev/null +++ b/tests/Serializer/yml/post.yml @@ -0,0 +1,2 @@ +type: post +title: 'Post Title' From b63bdca9dca30a602ed8d7b4210a64156b961cc1 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 17:16:39 +0300 Subject: [PATCH 5/7] Fix CS --- src/JMS/Serializer/Metadata/ClassMetadata.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/JMS/Serializer/Metadata/ClassMetadata.php b/src/JMS/Serializer/Metadata/ClassMetadata.php index 795f103a0..12b9526d1 100644 --- a/src/JMS/Serializer/Metadata/ClassMetadata.php +++ b/src/JMS/Serializer/Metadata/ClassMetadata.php @@ -309,7 +309,6 @@ private function handleDiscriminatorProperty() $discriminatorProperty->xmlNamespace = $this->xmlDiscriminatorNamespace; $this->propertyMetadata[$this->discriminatorFieldName] = $discriminatorProperty; } - } private function sortProperties() From 475166835310445685c589c0233b517b226a12b7 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 17:20:39 +0300 Subject: [PATCH 6/7] PHP 5.5 compatibility --- tests/Fixtures/Discriminator/ImagePost.php | 2 -- tests/Fixtures/Discriminator/Post.php | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/Fixtures/Discriminator/ImagePost.php b/tests/Fixtures/Discriminator/ImagePost.php index 749ca96e8..864362c42 100644 --- a/tests/Fixtures/Discriminator/ImagePost.php +++ b/tests/Fixtures/Discriminator/ImagePost.php @@ -1,7 +1,5 @@ title = $title; } From de100052bdeec66b370b162a025df0b859ac16e1 Mon Sep 17 00:00:00 2001 From: Alexandr Zolotukhin Date: Sun, 29 Jul 2018 18:27:57 +0300 Subject: [PATCH 7/7] Throw deprecation notice on non-abstract parent when it's not in the map --- src/JMS/Serializer/Metadata/ClassMetadata.php | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/JMS/Serializer/Metadata/ClassMetadata.php b/src/JMS/Serializer/Metadata/ClassMetadata.php index 12b9526d1..a2c9b17fd 100644 --- a/src/JMS/Serializer/Metadata/ClassMetadata.php +++ b/src/JMS/Serializer/Metadata/ClassMetadata.php @@ -268,20 +268,22 @@ public function unserialize($str) private function handleDiscriminatorProperty() { - if ( - $this->discriminatorMap - && !$this->reflection->isAbstract() - && !$this->reflection->isInterface() - ) { - if ( - false === ($typeValue = array_search($this->name, $this->discriminatorMap, true)) - && $this->discriminatorBaseClass !== $this->name - ) { - throw new \LogicException(sprintf( - 'The sub-class "%s" is not listed in the discriminator of the base class "%s".', - $this->name, - $this->discriminatorBaseClass - )); + if ($this->discriminatorMap && !$this->reflection->isAbstract() && !$this->reflection->isInterface()) { + if (false === $typeValue = array_search($this->name, $this->discriminatorMap, true)) { + if ($this->discriminatorBaseClass === $this->name) { + @trigger_error( + 'Discriminator map was configured on non-abstract parent class but parent class' + .' was not included into discriminator map. It will throw exception in next major version.' + .' Either declare parent as abstract class or add it into discriminator map.', + E_USER_DEPRECATED + ); + } else { + throw new \LogicException(sprintf( + 'The sub-class "%s" is not listed in the discriminator of the base class "%s".', + $this->name, + $this->discriminatorBaseClass + )); + } } $this->discriminatorValue = $typeValue;