From 252dffcf62f7fe6c4ceab907093ab453f6e7228c Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Sun, 19 Apr 2020 13:05:28 +0200 Subject: [PATCH 1/7] Fix form mapping to uninitialized properties --- .../Core/DataMapper/PropertyPathMapper.php | 26 +++++++++++++++++-- .../DataMapper/PropertyPathMapperTest.php | 17 ++++++++++++ .../Fixtures/TypehintedPropertiesCar.php | 8 ++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Form/Tests/Fixtures/TypehintedPropertiesCar.php diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index 657d9d63bec26..22b9c71de5f67 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -13,6 +13,7 @@ use Symfony\Component\Form\DataMapperInterface; use Symfony\Component\Form\Exception\UnexpectedTypeException; +use Symfony\Component\PropertyAccess\Exception\AccessException; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; @@ -76,16 +77,37 @@ public function mapFormsToData($forms, &$data) $propertyValue = $form->getData(); // If the field is of type DateTimeInterface and the data is the same skip the update to // keep the original object hash - if ($propertyValue instanceof \DateTimeInterface && $propertyValue == $this->propertyAccessor->getValue($data, $propertyPath)) { + if ($propertyValue instanceof \DateTimeInterface && $propertyValue == $this->getPropertyValue($data, $propertyPath)) { continue; } // If the data is identical to the value in $data, we are // dealing with a reference - if (!\is_object($data) || !$config->getByReference() || $propertyValue !== $this->propertyAccessor->getValue($data, $propertyPath)) { + if (!\is_object($data) || !$config->getByReference() || $propertyValue !== $this->getPropertyValue($data, $propertyPath)) { $this->propertyAccessor->setValue($data, $propertyPath, $propertyValue); } } } } + + /** + * Get the property value per PropertyAccessor. + * Treat uninitialized properties as null. + * + * @param object|array $objectOrArray The object or array to traverse + * @param string|PropertyPathInterface $propertyPath The property path to read + * + * @return mixed The value at the end of the property path + * + * @throws Exception\InvalidArgumentException If the property path is invalid + * @throws Exception\UnexpectedTypeException If a value within the path is neither object nor array + */ + private function getPropertyValue($data, $propertyPath) + { + try { + return $this->propertyAccessor->getValue($data, $propertyPath); + } catch (AccessException $e) { + return null; + } + } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index da351295c381e..536f479328537 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -17,6 +17,7 @@ use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper; use Symfony\Component\Form\Form; use Symfony\Component\Form\FormConfigBuilder; +use Symfony\Component\Form\Tests\Fixtures\TypehintedPropertiesCar; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; use Symfony\Component\PropertyAccess\PropertyPath; @@ -300,6 +301,22 @@ public function testMapFormsToDataIgnoresDisabled() $this->assertSame($initialEngine, $car->engine); } + /** + * @requires PHP >= 7.4 + */ + public function testMapFormsToUninitializedProperties() + { + $engine = new \stdClass(); + $car = new TypehintedPropertiesCar(); + $config = new FormConfigBuilder('engine', \stdClass::class, $this->dispatcher); + $config->setData($engine); + $form = new SubmittedForm($config); + + $this->mapper->mapFormsToData([$form], $car); + + $this->assertSame($engine, $car->engine); + } + /** * @dataProvider provideDate */ diff --git a/src/Symfony/Component/Form/Tests/Fixtures/TypehintedPropertiesCar.php b/src/Symfony/Component/Form/Tests/Fixtures/TypehintedPropertiesCar.php new file mode 100644 index 0000000000000..df1ebe2c9bf1f --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/TypehintedPropertiesCar.php @@ -0,0 +1,8 @@ + Date: Sun, 19 Apr 2020 13:10:05 +0200 Subject: [PATCH 2/7] Ignore uninitialized properties when mapping to form --- .../Core/DataMapper/PropertyPathMapper.php | 6 +++++- .../DataMapper/PropertyPathMapperTest.php | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index 22b9c71de5f67..5428baf9ce255 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -47,7 +47,11 @@ public function mapDataToForms($data, $forms) $config = $form->getConfig(); if (!$empty && null !== $propertyPath && $config->getMapped()) { - $form->setData($this->propertyAccessor->getValue($data, $propertyPath)); + try { + $form->setData($this->propertyAccessor->getValue($data, $propertyPath)); + } catch (AccessException $e) { + // Skip unitialized properties on $data + } } else { $form->setData($config->getData()); } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index 536f479328537..763913c90c839 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -114,6 +114,26 @@ public function testMapDataToFormsIgnoresUnmapped() $this->assertNull($form->getData()); } + /** + * @requires PHP >= 7.4 + */ + public function testMapDataToFormsIgnoresUninitializedProperties() + { + $engineForm = new Form(new FormConfigBuilder('engine', \stdClass::class, $this->dispatcher)); + $colorConfig = new FormConfigBuilder('color', \stdClass::class, $this->dispatcher); + $color = new \stdClass(); + $colorConfig->setData($color); + $colorForm = new Form($colorConfig); + + $car = new TypehintedPropertiesCar(); + $car->engine = new \stdClass(); + + $this->mapper->mapDataToForms($car, [$engineForm, $colorForm]); + + $this->assertSame($car->engine, $engineForm->getData()); + $this->assertSame($color, $colorForm->getData()); + } + public function testMapDataToFormsSetsDefaultDataIfPassedDataIsNull() { $default = new \stdClass(); From 6c3aaf28c02b398bfc840aaad09bb6b1a04acdc8 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Sun, 19 Apr 2020 13:26:06 +0200 Subject: [PATCH 3/7] Fix form mapping tests --- .../Extension/Core/DataMapper/PropertyPathMapperTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index 763913c90c839..f3469af8cbb7d 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -314,7 +314,7 @@ public function testMapFormsToDataIgnoresDisabled() $config->setPropertyPath($propertyPath); $config->setData($engine); $config->setDisabled(true); - $form = new Form($config); + $form = new SubmittedForm($config); $this->mapper->mapFormsToData([$form], $car); @@ -376,7 +376,7 @@ public function isSubmitted() } } -class NotSynchronizedForm extends Form +class NotSynchronizedForm extends SubmittedForm { public function isSynchronized() { From 39b4abe0d9e7248b45a75b430e174f1d1d0e26a1 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Sun, 19 Apr 2020 20:09:17 +0200 Subject: [PATCH 4/7] Be specific about ignored AccessExceptions when mapping to form --- .../Form/Extension/Core/DataMapper/PropertyPathMapper.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index 5428baf9ce255..91ef45253bb49 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -14,6 +14,7 @@ use Symfony\Component\Form\DataMapperInterface; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\PropertyAccess\Exception\AccessException; +use Symfony\Component\PropertyAccess\Exception\UninitializedPropertyException; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; @@ -51,6 +52,12 @@ public function mapDataToForms($data, $forms) $form->setData($this->propertyAccessor->getValue($data, $propertyPath)); } catch (AccessException $e) { // Skip unitialized properties on $data + if (!$e instanceof UninitializedPropertyException + // For versions without UninitializedPropertyException check the exception message + && (class_exists(UninitializedPropertyException::class) || false === strpos($e->getMessage(), 'You should initialize it')) + ) { + throw $e; + } } } else { $form->setData($config->getData()); From 19679dcbeb600a4164ff7b800599c2eff6664610 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Fri, 24 Apr 2020 12:54:18 +0200 Subject: [PATCH 5/7] Fix test for old phpunit --- .../Extension/Core/DataMapper/PropertyPathMapperTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index f3469af8cbb7d..7b76b312b5a41 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -115,7 +115,7 @@ public function testMapDataToFormsIgnoresUnmapped() } /** - * @requires PHP >= 7.4 + * @requires PHP 7.4 */ public function testMapDataToFormsIgnoresUninitializedProperties() { @@ -322,7 +322,7 @@ public function testMapFormsToDataIgnoresDisabled() } /** - * @requires PHP >= 7.4 + * @requires PHP 7.4 */ public function testMapFormsToUninitializedProperties() { From a975d5e7d278d8e23e52d6e5a0e18c3ef9f276ff Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Fri, 24 Apr 2020 13:45:13 +0200 Subject: [PATCH 6/7] Fix test for symfony/property-access 2.8 --- .../Extension/Core/DataMapper/PropertyPathMapperTest.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index 7b76b312b5a41..f98beecb3f229 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -120,10 +120,7 @@ public function testMapDataToFormsIgnoresUnmapped() public function testMapDataToFormsIgnoresUninitializedProperties() { $engineForm = new Form(new FormConfigBuilder('engine', \stdClass::class, $this->dispatcher)); - $colorConfig = new FormConfigBuilder('color', \stdClass::class, $this->dispatcher); - $color = new \stdClass(); - $colorConfig->setData($color); - $colorForm = new Form($colorConfig); + $colorForm = new Form(new FormConfigBuilder('color', \stdClass::class, $this->dispatcher)); $car = new TypehintedPropertiesCar(); $car->engine = new \stdClass(); @@ -131,7 +128,6 @@ public function testMapDataToFormsIgnoresUninitializedProperties() $this->mapper->mapDataToForms($car, [$engineForm, $colorForm]); $this->assertSame($car->engine, $engineForm->getData()); - $this->assertSame($color, $colorForm->getData()); } public function testMapDataToFormsSetsDefaultDataIfPassedDataIsNull() From db718c0767bd6f28104818d642b517b87b276e6c Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Tue, 19 May 2020 11:12:12 +0200 Subject: [PATCH 7/7] Catch only UninitializedPropertyException when getting a value per PropertyAcessor --- .../Core/DataMapper/PropertyPathMapper.php | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index 91ef45253bb49..ffbd365898152 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -52,12 +52,7 @@ public function mapDataToForms($data, $forms) $form->setData($this->propertyAccessor->getValue($data, $propertyPath)); } catch (AccessException $e) { // Skip unitialized properties on $data - if (!$e instanceof UninitializedPropertyException - // For versions without UninitializedPropertyException check the exception message - && (class_exists(UninitializedPropertyException::class) || false === strpos($e->getMessage(), 'You should initialize it')) - ) { - throw $e; - } + $this->catchUninitializedPropertyException($e); } } else { $form->setData($config->getData()); @@ -118,7 +113,24 @@ private function getPropertyValue($data, $propertyPath) try { return $this->propertyAccessor->getValue($data, $propertyPath); } catch (AccessException $e) { + // The following line might be removed in future versions + // See https://github.com/symfony/symfony/issues/36754 + $this->catchUninitializedPropertyException($e); + return null; } } + + /** + * Throw everything but UninitializedPropertyException. + */ + private function catchUninitializedPropertyException(AccessException $e) + { + if (!$e instanceof UninitializedPropertyException + // For versions without UninitializedPropertyException check the exception message + && (class_exists(UninitializedPropertyException::class) || false === strpos($e->getMessage(), 'You should initialize it')) + ) { + throw $e; + } + } }