From 1cd97fd14da84a01d9fbe85059623c2bcbbf0090 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 14 Apr 2023 09:12:43 -0400 Subject: [PATCH] [Live] Ignore problems with writing bad types for writable paths The motivating use-case is where you have a writable property that starts null but the setter method does NOT allow null. When that null is rehydrated later, we attempt to call the setter with a null value, which explodes. This makes for a better UX... though with a sprinkle of magic to make it happen. --- .../src/LiveComponentHydrator.php | 24 +++++----- .../Fixtures/Entity/CategoryFixtureEntity.php | 47 +++++++++++++++++++ .../Integration/LiveComponentHydratorTest.php | 14 ++++++ 3 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 src/LiveComponent/tests/Fixtures/Entity/CategoryFixtureEntity.php diff --git a/src/LiveComponent/src/LiveComponentHydrator.php b/src/LiveComponent/src/LiveComponentHydrator.php index 7c4a5c0d21a..ab3e1fc046f 100644 --- a/src/LiveComponent/src/LiveComponentHydrator.php +++ b/src/LiveComponent/src/LiveComponentHydrator.php @@ -159,7 +159,8 @@ public function hydrate(object $component, array $props, array $updatedProps, Li ); /* - | 2) Set UPDATED "writable paths" data for this LiveProp. + | 2) Set ORIGINAL "writable paths" data for this LiveProp. + | (which may represent data that was updated on a previous request) */ $originalWritablePaths = $this->calculateWritablePaths($propMetadata, $propertyValue, $dehydratedOriginalProps, $frontendName, \get_class($component)); $propertyValue = $this->setWritablePaths( @@ -194,16 +195,12 @@ public function hydrate(object $component, array $props, array $updatedProps, Li | 4) Set UPDATED "writable paths" data for this LiveProp. */ $updatedWritablePaths = $this->calculateWritablePaths($propMetadata, $propertyValue, $dehydratedUpdatedProps, $frontendName, \get_class($component)); - try { - $propertyValue = $this->setWritablePaths( - $updatedWritablePaths, - $frontendName, - $propertyValue, - $dehydratedUpdatedProps, - ); - } catch (HydrationException $e) { - // swallow this: it's bad data from the user - } + $propertyValue = $this->setWritablePaths( + $updatedWritablePaths, + $frontendName, + $propertyValue, + $dehydratedUpdatedProps, + ); try { $this->propertyAccessor->setValue($component, $propMetadata->getName(), $propertyValue); @@ -316,7 +313,10 @@ private function setWritablePaths(array $writablePaths, string $frontendPropName $writablePathData ); } catch (PropertyAccessExceptionInterface $exception) { - throw new HydrationException(sprintf('The model "%s.%s" was sent for update, but it could not be set: %s', $frontendPropName, $writablePath, $exception->getMessage()), $exception); + // The user likely sent bad data (e.g. string for a number field). + // Simply fail to set the field and use the previous value + // (this can also happen with original, writable data - e.g. if + // a property is "null", but the setter method doesn't allow null). } } diff --git a/src/LiveComponent/tests/Fixtures/Entity/CategoryFixtureEntity.php b/src/LiveComponent/tests/Fixtures/Entity/CategoryFixtureEntity.php new file mode 100644 index 00000000000..3bbfcf5daad --- /dev/null +++ b/src/LiveComponent/tests/Fixtures/Entity/CategoryFixtureEntity.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Fixtures\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class CategoryFixtureEntity +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private ?int $id = null; + + /** + * @ORM\Column(type="string") + */ + private ?string $name = null; + + public function getId(): ?int + { + return $this->id; + } + + public function getName(): ?string + { + return $this->name; + } + + public function setName(string $name) + { + $this->name = $name; + } +} diff --git a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php index ca4901b7e10..1133024a5f3 100644 --- a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php +++ b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php @@ -23,6 +23,7 @@ use Symfony\UX\LiveComponent\Tests\Fixtures\Dto\Embeddable2; use Symfony\UX\LiveComponent\Tests\Fixtures\Dto\Money; use Symfony\UX\LiveComponent\Tests\Fixtures\Dto\Temperature; +use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\CategoryFixtureEntity; use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\Embeddable1; use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\Entity1; use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\Entity2; @@ -1066,6 +1067,19 @@ public function provideInvalidHydrationTests(): iterable $this->assertSame(1, $object->nonNullableInt->value); }); }, 80100]; + + yield 'writable_path_with_type_problem_ignored' => [function () { + return HydrationTest::create(new class() { + #[LiveProp(writable: ['name'])] + public CategoryFixtureEntity $category; + }) + ->mountWith(['category' => new CategoryFixtureEntity()]) + ->assertObjectAfterHydration(function (object $object) { + // dehydrating category.name=null does not cause issues + // even though setName() does not allow null + $this->assertNull($object->category->getName()); + }); + }]; } public function testHydrationFailsIfChecksumMissing(): void