Skip to content

Commit

Permalink
feature #788 [Live] Ignore problems with writing bad types for writab…
Browse files Browse the repository at this point in the history
…le paths (weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Live] Ignore problems with writing bad types for writable paths

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| Tickets       | None
| License       | MIT

Found when building some components. The motivating use-case is where you have a writable property that starts null but the setter method does NOT allow null (this is common with entity properties and methods). 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. Without this, you would need to make your setter methods allow null (e.g. `setName(?string $name)`) *just* because the original property starts as null.

Cheers!

Commits
-------

1cd97fd [Live] Ignore problems with writing bad types for writable paths
  • Loading branch information
weaverryan committed Apr 14, 2023
2 parents 1d07901 + 1cd97fd commit cc414dc
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 cc414dc

Please sign in to comment.