Skip to content

Commit

Permalink
[Live] Ignore problems with writing bad types for writable paths
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
weaverryan committed Apr 14, 2023
1 parent 8eb4e08 commit 1cd97fd
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 12 deletions.
24 changes: 12 additions & 12 deletions src/LiveComponent/src/LiveComponentHydrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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).
}
}

Expand Down
47 changes: 47 additions & 0 deletions src/LiveComponent/tests/Fixtures/Entity/CategoryFixtureEntity.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* 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;
}
}
14 changes: 14 additions & 0 deletions src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1cd97fd

Please sign in to comment.