From 6796a536a8b77f41ca393cbfa66bcc9827980976 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2023 15:27:38 -0400 Subject: [PATCH] [Live] Complex hydration fixes for 2.8 --- src/LiveComponent/composer.json | 1 + src/LiveComponent/doc/index.rst | 7 +- .../src/LiveComponentHydrator.php | 292 +++++++++++++++--- .../Metadata/LiveComponentMetadataFactory.php | 1 - .../src/Metadata/LivePropMetadata.php | 6 - .../Normalizer/DoctrineObjectNormalizer.php | 43 +-- .../Fixtures/Entity/TodoItemFixtureEntity.php | 69 +++++ .../Fixtures/Entity/TodoListFixtureEntity.php | 72 +++++ .../Integration/LiveComponentHydratorTest.php | 132 +++++++- .../LiveComponentDemoController.php | 4 +- ux.symfony.com/src/Form/TodoItemForm.php | 6 +- ux.symfony.com/src/Form/TodoListForm.php | 3 + .../form_collection_type.html.twig | 2 +- 13 files changed, 547 insertions(+), 91 deletions(-) create mode 100644 src/LiveComponent/tests/Fixtures/Entity/TodoItemFixtureEntity.php create mode 100644 src/LiveComponent/tests/Fixtures/Entity/TodoListFixtureEntity.php diff --git a/src/LiveComponent/composer.json b/src/LiveComponent/composer.json index 4d8a798b8a3..80dc4668999 100644 --- a/src/LiveComponent/composer.json +++ b/src/LiveComponent/composer.json @@ -35,6 +35,7 @@ "doctrine/annotations": "^1.0", "doctrine/doctrine-bundle": "^2.0", "doctrine/orm": "^2.7", + "phpdocumentor/reflection-docblock": "5.x-dev", "symfony/dependency-injection": "^5.4|^6.0", "symfony/form": "^5.4|^6.0", "symfony/framework-bundle": "^5.4|^6.0", diff --git a/src/LiveComponent/doc/index.rst b/src/LiveComponent/doc/index.rst index 2d30958d253..54f52145bc5 100644 --- a/src/LiveComponent/doc/index.rst +++ b/src/LiveComponent/doc/index.rst @@ -541,8 +541,11 @@ the ``Context`` attribute from Symfony's serializer:: If your property has writable paths, those will be normalized/denormalized using the same `Context` set on the property itself. -Or, you can take full control over the (de)hydration process by setting the ``hydrateWith`` -and ``dehydrateWith`` options on ``LiveProp``. For example:: +Using the serializer isn't meant to work out-of-the-box in every possible situation +and it's always simpler to use scalar `LiveProp` values instead of complex objects. +If you're having (de)hydrating a complex object, you can take full control by +setting the ``hydrateWith`` and ``dehydrateWith`` options on ``LiveProp``. For +example:: class ComponentWithAddressDto { diff --git a/src/LiveComponent/src/LiveComponentHydrator.php b/src/LiveComponent/src/LiveComponentHydrator.php index 18fb3c9c8bb..44864af51c6 100644 --- a/src/LiveComponent/src/LiveComponentHydrator.php +++ b/src/LiveComponent/src/LiveComponentHydrator.php @@ -18,9 +18,16 @@ use Symfony\Component\PropertyAccess\PropertyAccessorInterface; use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface; use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Serializer\Encoder\JsonEncoder; use Symfony\Component\Serializer\Exception\ExceptionInterface; +use Symfony\Component\Serializer\Exception\ExtraAttributesException; +use Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException; +use Symfony\Component\Serializer\Exception\NotNormalizableValueException; +use Symfony\Component\Serializer\Exception\PartialDenormalizationException; use Symfony\Component\Serializer\Mapping\AttributeMetadataInterface; use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface; +use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; +use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; @@ -93,11 +100,11 @@ public function dehydrate(object $component, ComponentAttributes $attributes, Li throw new \LogicException(sprintf('The "%s" component has a dehydrateMethod of "%s" but the method does not exist.', \get_class($component), $method)); } $dehydratedValue = $component->$method($dehydratedValue); - } elseif (\is_object($rawPropertyValue)) { + } elseif (\is_object($rawPropertyValue) || \is_array($rawPropertyValue)) { $dehydratedValue = $this->normalizer->normalize( $dehydratedValue, 'json', - array_merge([self::LIVE_CONTEXT => true], $normalizationContext) + $normalizationContext ); } @@ -128,7 +135,7 @@ public function dehydrate(object $component, ComponentAttributes $attributes, Li $pathValue = $this->normalizer->normalize( $pathValue, 'json', - array_merge([self::LIVE_CONTEXT => true], $normalizationContext) + $normalizationContext, ); $this->preventArrayDehydratedValueForObjectThatIsWritable($pathValue, $originalPathValueClass, false, sprintf('%s.%s', $propMetadata->getName(), $path), false); @@ -179,12 +186,15 @@ public function hydrate(object $component, array $props, array $updatedProps, Li continue; } + $types = $this->propertyTypeExtractor->getTypes(\get_class($component), $propMetadata->getName()); + /* | 1) Hydrate and set ORIGINAL data for this LiveProp. */ $propertyValue = $this->hydrateLiveProp( $component, $propMetadata, + $types, $dehydratedOriginalProps->getPropValue($frontendName), ); @@ -216,6 +226,7 @@ public function hydrate(object $component, array $props, array $updatedProps, Li $propertyValue = $this->hydrateLiveProp( $component, $propMetadata, + $types, $dehydratedUpdatedProps->getPropValue($frontendName), ); } catch (HydrationException $e) { @@ -357,50 +368,236 @@ private function adjustPropertyPathForData(mixed $rawPropertyValue, string $prop return $finalPropertyPath; } - private function hydrateValue(mixed $value, ?string $type, bool $allowsNull, bool $isBuiltIn, array $denormalizationContext, string $fullPathForErrors, bool $isWritable, string $componentClass): mixed + private function hydrateValue(mixed $data, array $types, array $context, string $fullPathForErrors, bool $isWritable, string $componentClass): mixed { - if (\is_string($value) && \in_array($type, ['int', 'float', 'bool'], true)) { - return self::coerceScalarValue($value, $type, $allowsNull); + try { + return $this->doHydrateValue($data, $types, $context, $fullPathForErrors, $isWritable, $componentClass); + } catch (PartialDenormalizationException $exception) { + // Swallow these: at least one sub path on the object could not be hydrated. + // This is a tricky situation: it could be caused by user error, it even + // because a property was an object, a null property was deserialized + // to null, but its setter only allow non-null values. It's balancing + // act of being friendly to use and not hiding errors. + // in the future, we could collect the errors and display them somewhere + return $exception->getData(); + } catch (ExceptionInterface $exception) { + $json = json_encode($data); + $message = sprintf(<< %s + +Fix the error, or use the hydrateWith/dehydrateWith options to control the (de)hydration process.' +EOF, + $componentClass, + $fullPathForErrors, + $exception->getMessage() + ); + + // unless the data is gigantic, include it in the error to help + if (\strlen($json) < 1000) { + $message .= sprintf(<< $e->value, $type::cases()))) { - return null; + /** + * Modeled after AbstractObjectNormalizer::validateAndDenormalize(). + * + * @param Type[] $types + */ + private function doHydrateValue(mixed $data, array $types, array $context, string $fullPathForErrors, bool $isWritable, string $currentClass): mixed + { + $format = 'json'; + $context = array_merge([ + self::LIVE_CONTEXT => true, + // prevents NotNormalizableValueException from being thrown + // e.g. if we're denormalizing into an object, and one value + // fails to be set (e.g. because of a type mismatch), don't + // stop the entire process: continue + // In the future, we could collect these and do something with them + DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true, + ], $context); + + $expectedTypes = []; + $isUnionType = \count($types) > 1; + $extraAttributesException = null; + $missingConstructorArgumentException = null; + foreach ($types as $type) { + if (null === $data && $type->isNullable()) { + return null; + } + + /* START CUSTOM */ + // coerce scalar values to the correct type instead of just ignoring them + if (\is_string($data) && \in_array($type->getBuiltinType(), ['int', 'float', 'bool'], true)) { + return self::coerceScalarValue($data, $type->getBuiltinType(), $type->isNullable()); + } + + $typeClass = $type->getClassName(); + if ($typeClass && $type->isNullable() && is_a($typeClass, \BackedEnum::class, true) && !\in_array($data, array_map(fn (\BackedEnum $e) => $e->value, $typeClass::cases()))) { + return null; + } + /* END CUSTOM */ + + $collectionValueType = $type->isCollection() ? $type->getCollectionValueTypes()[0] ?? null : null; + + /* START DUPLICATION */ + try { + if (null !== $collectionValueType && Type::BUILTIN_TYPE_OBJECT === $collectionValueType->getBuiltinType()) { + $builtinType = Type::BUILTIN_TYPE_OBJECT; + $class = $collectionValueType->getClassName().'[]'; + + if (\count($collectionKeyType = $type->getCollectionKeyTypes()) > 0) { + [$context['key_type']] = $collectionKeyType; + } + + $context['value_type'] = $collectionValueType; + } elseif ($type->isCollection() && \count($collectionValueType = $type->getCollectionValueTypes()) > 0 && Type::BUILTIN_TYPE_ARRAY === $collectionValueType[0]->getBuiltinType()) { + // get inner type for any nested array + [$innerType] = $collectionValueType; + + // note that it will break for any other builtinType + $dimensions = '[]'; + while (\count($innerType->getCollectionValueTypes()) > 0 && Type::BUILTIN_TYPE_ARRAY === $innerType->getBuiltinType()) { + $dimensions .= '[]'; + [$innerType] = $innerType->getCollectionValueTypes(); + } + + if (null !== $innerType->getClassName()) { + // the builtinType is the inner one and the class is the class followed by []...[] + $builtinType = $innerType->getBuiltinType(); + $class = $innerType->getClassName().$dimensions; + } else { + // default fallback (keep it as array) + $builtinType = $type->getBuiltinType(); + $class = $type->getClassName(); + } + } else { + $builtinType = $type->getBuiltinType(); + $class = $type->getClassName(); + } + + $expectedTypes[Type::BUILTIN_TYPE_OBJECT === $builtinType && $class ? $class : $builtinType] = true; + + if (Type::BUILTIN_TYPE_OBJECT === $builtinType) { + /* START CUSTOM */ + $childContext = $context; + // $childContext = $this->createChildContext($context, $attribute, $format); + if ($isWritable) { + $this->preventArrayDehydratedValueForObjectThatIsWritable($data, $class, false, $fullPathForErrors, true); + } + /* END CUSTOM */ + + if ($this->normalizer->supportsDenormalization($data, $class, $format, $childContext)) { + return $this->normalizer->denormalize($data, $class, $format, $childContext); + } + } + + // JSON only has a Number type corresponding to both int and float PHP types. + // PHP's json_encode, JavaScript's JSON.stringify, Go's json.Marshal as well as most other JSON encoders convert + // floating-point numbers like 12.0 to 12 (the decimal part is dropped when possible). + // PHP's json_decode automatically converts Numbers without a decimal part to integers. + // To circumvent this behavior, integers are converted to floats when denormalizing JSON based formats and when + // a float is expected. + if (Type::BUILTIN_TYPE_FLOAT === $builtinType && \is_int($data) && null !== $format && str_contains($format, JsonEncoder::FORMAT)) { + return (float) $data; + } + + if ((Type::BUILTIN_TYPE_FALSE === $builtinType && false === $data) || (Type::BUILTIN_TYPE_TRUE === $builtinType && true === $data)) { + return $data; + } + + if (('is_'.$builtinType)($data)) { + return $data; + } + } catch (NotNormalizableValueException $e) { + if (!$isUnionType) { + throw $e; + } + } catch (ExtraAttributesException $e) { + if (!$isUnionType) { + throw $e; + } + + $extraAttributesException ??= $e; + } catch (MissingConstructorArgumentsException $e) { + if (!$isUnionType) { + throw $e; + } + + $missingConstructorArgumentException ??= $e; + } + /* END DUPLICATION */ } - if (null === $value || null === $type || $isBuiltIn) { - return $value; + /* START DUPLICATION */ + if ($extraAttributesException) { + throw $extraAttributesException; + } + + if ($missingConstructorArgumentException) { + throw $missingConstructorArgumentException; } - if ($isWritable) { - $this->preventArrayDehydratedValueForObjectThatIsWritable($value, $type, $isBuiltIn, $fullPathForErrors, true); + if ($context[AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT] ?? $this->defaultContext[AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT] ?? false) { + return $data; + } + + throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('The type of the "%s" attribute for class "%s" must be one of "%s" ("%s" given).', $fullPathForErrors, $currentClass, implode('", "', array_keys($expectedTypes)), get_debug_type($data)), $data, array_keys($expectedTypes), $context['deserialization_path'] ?? $fullPathForErrors); + /* END DUPLICATION */ + + if (\is_string($value) && \in_array($typeString, ['int', 'float', 'bool'], true)) { + return self::coerceScalarValue($value, $typeString, $allowsNull); + } + + if (null === $value || null === $typeString || $isBuiltIn) { + return $value; } try { return $this->normalizer->denormalize( $value, - $type, + $typeString, 'json', array_merge([self::LIVE_CONTEXT => true], $denormalizationContext) ); } catch (ExceptionInterface $exception) { $json = json_encode($value); - $message = sprintf( - 'The normalizer was used to hydrate/denormalize the "%s" property on your "%s" live component, but it failed: %s', - $fullPathForErrors, + $message = sprintf(<< %s + +Fix the error, or use the hydrateWith/dehydrateWith options to control the (de)hydration process.' +EOF, $componentClass, + $fullPathForErrors, $exception->getMessage() ); // unless the data is gigantic, include it in the error to help if (\strlen($json) < 1000) { - $message .= sprintf(' The data sent from the frontend was: %s', $json); + $message .= sprintf(<<hydrateMethod()) { if (!method_exists($component, $propMetadata->hydrateMethod())) { @@ -410,11 +607,13 @@ private function hydrateLiveProp(object $component, Metadata\LivePropMetadata $p return $component->{$propMetadata->hydrateMethod()}($dehydratedProp); } + if (null === $types) { + return $dehydratedProp; + } + return $this->hydrateValue( $dehydratedProp, - $propMetadata->getType(), - $propMetadata->allowsNull(), - $propMetadata->isBuiltIn(), + $types, $this->getDenormalizationContext($component, $propMetadata->getName()), $propMetadata->getName(), $propMetadata->isIdentityWritable(), @@ -455,30 +654,24 @@ private function hydrateAndSetWritablePaths(LivePropMetadata $propMetadata, stri // e.g. writablePaths: ['post.createdAt'] is not supported. if (0 === substr_count($writablePath, '.') && \is_object($propertyValue)) { $types = $this->propertyTypeExtractor->getTypes(\get_class($propertyValue), $writablePath); - $type = null === $types ? null : $types[0] ?? null; - $isBuiltIn = $type ? Type::BUILTIN_TYPE_OBJECT !== $type->getBuiltinType() : false; - $typeString = null; - if ($type) { - $typeString = $isBuiltIn ? $type->getBuiltinType() : $type->getClassName(); - } - try { - $writablePathData = $this->hydrateValue( - $writablePathData, - $typeString, - $type ? $type->isNullable() : true, - $isBuiltIn, - $denormalizationContext, - sprintf('%s.%s', $frontendPropName, $writablePath), - true, - $componentClass, - ); - } catch (HydrationException $exception) { - if ($throwErrors) { - throw $exception; + if (null !== $types) { + try { + $writablePathData = $this->hydrateValue( + $writablePathData, + $types, + $denormalizationContext, + sprintf('%s.%s', $frontendPropName, $writablePath), + true, + $componentClass, + ); + } catch (HydrationException $exception) { + if ($throwErrors) { + throw $exception; + } + // swallow problems hydrating user-sent data + continue; } - // swallow problems hydrating user-sent data - continue; } } @@ -520,7 +713,18 @@ private function getNormalizationContext(object $component, string $propertyName { $attributeMetadata = $this->getSerializationAttributeMetadata($component, $propertyName); // passing [] for groups - not sure if we need to be smarter - return $attributeMetadata ? $attributeMetadata->getNormalizationContextForGroups([]) : []; + + $context = $attributeMetadata ? $attributeMetadata->getNormalizationContextForGroups([]) : []; + + return array_merge([ + self::LIVE_CONTEXT => true, + // avoid circular references by setting the reference to null + // this covers the most common case of Doctrine relations + // and isn't perfect, but is a decent default + AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER => static function ($object, $format, $context) { + return null; + }, + ], $context); } private function getDenormalizationContext(object $component, string $propertyName): array diff --git a/src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php b/src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php index 685c7998e07..bb16ac4e30f 100644 --- a/src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php +++ b/src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php @@ -61,7 +61,6 @@ public static function createPropMetadatas(\ReflectionClass $class): array $property->getName(), $attribute->newInstance(), $type ? $type->getName() : null, - $type ? $type->allowsNull() : false, $type ? $type->isBuiltin() : false, ); } diff --git a/src/LiveComponent/src/Metadata/LivePropMetadata.php b/src/LiveComponent/src/Metadata/LivePropMetadata.php index 2953ab40a10..005d7259e4e 100644 --- a/src/LiveComponent/src/Metadata/LivePropMetadata.php +++ b/src/LiveComponent/src/Metadata/LivePropMetadata.php @@ -26,7 +26,6 @@ public function __construct( private string $name, private LiveProp $liveProp, private ?string $typeName, - private bool $allowsNull, private bool $isBuiltIn, ) { } @@ -41,11 +40,6 @@ public function getType(): ?string return $this->typeName; } - public function allowsNull(): bool - { - return $this->allowsNull; - } - public function isBuiltIn(): bool { return $this->isBuiltIn; diff --git a/src/LiveComponent/src/Normalizer/DoctrineObjectNormalizer.php b/src/LiveComponent/src/Normalizer/DoctrineObjectNormalizer.php index 76345af7ec7..478233010ef 100644 --- a/src/LiveComponent/src/Normalizer/DoctrineObjectNormalizer.php +++ b/src/LiveComponent/src/Normalizer/DoctrineObjectNormalizer.php @@ -15,7 +15,6 @@ use Doctrine\Persistence\ManagerRegistry; use Doctrine\Persistence\ObjectManager; use Psr\Container\ContainerInterface; -use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; use Symfony\Contracts\Service\ServiceSubscriberInterface; @@ -30,9 +29,6 @@ */ final class DoctrineObjectNormalizer implements NormalizerInterface, DenormalizerInterface, ServiceSubscriberInterface { - /** Flag to avoid recursion in the normalizer */ - private const DOCTRINE_OBJECT_ALREADY_NORMALIZED = 'doctrine_object_normalizer.normalized'; - /** * @param ManagerRegistry[] $managerRegistries */ @@ -89,45 +85,24 @@ public function supportsNormalization(mixed $data, string $format = null, array public function denormalize(mixed $data, string $type, string $format = null, array $context = []): ?object { - if (null === $data) { - return null; - } - // $data is the single identifier or array of identifiers if (\is_scalar($data) || (\is_array($data) && isset($data[0]))) { return $this->objectManagerFor($type)->find($type, $data); } - // $data is an associative array to denormalize the entity - // allow the object to be denormalized using the default denormalizer - // except that the denormalizer has problems with "nullable: false" columns - // https://github.com/symfony/symfony/issues/49149 - // so, we send the object through the denormalizer, but turn type-checks off - // NOTE: The hydration system will already have prevented a writable property - // from reaching this. - $context[AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT] = true; - $context[self::DOCTRINE_OBJECT_ALREADY_NORMALIZED] = true; - - return $this->getDenormalizer()->denormalize($data, $type, $format, $context); + throw new \LogicException('Invalid denormalization case'); } public function supportsDenormalization(mixed $data, string $type, string $format = null, array $context = []) { - if (true !== ($context[LiveComponentHydrator::LIVE_CONTEXT] ?? null) || !class_exists($type)) { - return false; - } - - // not an entity? - if (null === $this->objectManagerFor($type)) { - return false; - } - - // avoid recursion - if ($context[self::DOCTRINE_OBJECT_ALREADY_NORMALIZED] ?? false) { - return false; + if ( + (\is_scalar($data) || (\is_array($data) && isset($data[0]))) + && null !== $this->objectManagerFor($type) + ) { + return true; } - return true; + return false; } public static function getSubscribedServices(): array @@ -139,6 +114,10 @@ public static function getSubscribedServices(): array private function objectManagerFor(string $class): ?ObjectManager { + if (!class_exists($class)) { + return null; + } + // todo cache/warmup an array of classes that are "doctrine objects" foreach ($this->managerRegistries as $registry) { if ($om = $registry->getManagerForClass($class)) { diff --git a/src/LiveComponent/tests/Fixtures/Entity/TodoItemFixtureEntity.php b/src/LiveComponent/tests/Fixtures/Entity/TodoItemFixtureEntity.php new file mode 100644 index 00000000000..801643f37a4 --- /dev/null +++ b/src/LiveComponent/tests/Fixtures/Entity/TodoItemFixtureEntity.php @@ -0,0 +1,69 @@ + + * + * 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\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class TodoItemFixtureEntity +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + public $id; + + /** + * @ORM\Column(type="string") + */ + private ?string $name = null; + + /** + * @ORM\ManyToOne(targetEntity=TodoListFixtureEntity::class, inversedBy="todoItems") + */ + private TodoListFixtureEntity $todoList; + + /** + * @param string $name + */ + public function __construct(string $name = null) + { + $this->name = $name; + } + + public function getTodoList(): TodoListFixtureEntity + { + return $this->todoList; + } + + public function setTodoList(?TodoListFixtureEntity $todoList): self + { + $this->todoList = $todoList; + + return $this; + } + + public function getName(): ?string + { + return $this->name; + } + + public function setName(?string $name) + { + $this->name = $name; + } +} diff --git a/src/LiveComponent/tests/Fixtures/Entity/TodoListFixtureEntity.php b/src/LiveComponent/tests/Fixtures/Entity/TodoListFixtureEntity.php new file mode 100644 index 00000000000..25a59103fed --- /dev/null +++ b/src/LiveComponent/tests/Fixtures/Entity/TodoListFixtureEntity.php @@ -0,0 +1,72 @@ + + * + * 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\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class TodoListFixtureEntity +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + public $id; + + /** + * @ORM\Column(type="string") + */ + public string $listTitle = ''; + + /** + * @ORM\OneToMany(targetEntity=TodoItemFixtureEntity::class, mappedBy="todoList") + */ + private Collection $todoItems; + + public function __construct(string $listTitle = '') + { + $this->listTitle = $listTitle; + $this->todoItems = new ArrayCollection(); + } + + public function getTodoItems(): Collection + { + return $this->todoItems; + } + + public function addTodoItem(TodoItemFixtureEntity $todoItem): self + { + if (!$this->todoItems->contains($todoItem)) { + $this->todoItems[] = $todoItem; + $todoItem->setTodoList($this); + } + + return $this; + } + + public function removeTodoItem(TodoItemFixtureEntity $todoItem): self + { + if ($this->todoItems->removeElement($todoItem)) { + // set the owning side to null (unless already changed) + if ($todoItem->getTodoList() === $this) { + $todoItem->setTodoList(null); + } + } + + return $this; + } +} diff --git a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php index 492479ef898..87bec59769c 100644 --- a/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php +++ b/src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php @@ -30,6 +30,8 @@ use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\Entity1; use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\Entity2; use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\ProductFixtureEntity; +use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\TodoItemFixtureEntity; +use Symfony\UX\LiveComponent\Tests\Fixtures\Entity\TodoListFixtureEntity; use Symfony\UX\LiveComponent\Tests\Fixtures\Enum\EmptyStringEnum; use Symfony\UX\LiveComponent\Tests\Fixtures\Enum\IntEnum; use Symfony\UX\LiveComponent\Tests\Fixtures\Enum\StringEnum; @@ -380,6 +382,37 @@ public function provideDehydrationHydrationTests(): iterable ; }]; + yield 'Non-Persisted entity with embedded entities (de)hydrates correctly' => [function () { + $list = new TodoListFixtureEntity('Cool stuff for today'); + $item = new TodoItemFixtureEntity('Buy milk'); + $list->addTodoItem($item); + + return HydrationTest::create(new class() { + #[LiveProp] + public TodoListFixtureEntity $todoList; + }) + ->mountWith(['todoList' => $list]) + ->assertDehydratesTo([ + 'todoList' => [ + 'id' => null, + 'listTitle' => 'Cool stuff for today', + 'todoItems' => [ + [ + 'id' => null, + 'name' => 'Buy milk', + 'todoList' => null, + ], + ], + ], + ]) + ->assertObjectAfterHydration(function (object $object) { + $this->assertSame('Cool stuff for today', $object->todoList->listTitle); + $this->assertCount(1, $object->todoList->getTodoItems()); + $this->assertSame('Buy milk', $object->todoList->getTodoItems()[0]->getName()); + }) + ; + }]; + yield 'Index array: (de)hydrates correctly' => [function () { return HydrationTest::create(new class() { #[LiveProp()] @@ -659,6 +692,37 @@ public function provideDehydrationHydrationTests(): iterable ; }]; + yield 'Array with objects: (de)hydrates correctly' => [function () { + $prod1 = new ProductFixtureEntity(); + $prod1->name = 'item1'; + $prod2 = new ProductFixtureEntity(); + $prod2->name = 'item2'; + + return HydrationTest::create(new class() { + #[LiveProp()] + /** @var \Symfony\UX\LiveComponent\Tests\Fixtures\Entity\ProductFixtureEntity[] */ + public $products = []; + }) + ->mountWith(['products' => [$prod1, $prod2]]) + ->assertDehydratesTo([ + 'products' => [ + ['id' => null, 'name' => 'item1', 'price' => 0], + ['id' => null, 'name' => 'item2', 'price' => 0], + ], + ]) + ->assertObjectAfterHydration(function (object $object) { + $this->assertSame( + 'item1', + $object->products[0]->name + ); + $this->assertSame( + 'item2', + $object->products[1]->name + ); + }) + ; + }]; + yield 'Enum: null remains null' => [function () { return HydrationTest::create(new class() { #[LiveProp()] @@ -1007,6 +1071,41 @@ public function __construct() ); } + public function testCircularReferencesAreAvoided(): void + { + $component = new class() { + #[LiveProp()] + public TodoListFixtureEntity $todoList; + }; + + $list = new TodoListFixtureEntity('My Todo List'); + $item = new TodoItemFixtureEntity('Roll the trash can around for awhile'); + $list->addTodoItem($item); + $component->todoList = $list; + + $dehydratedProps = $this->hydrator()->dehydrate( + $component, + new ComponentAttributes([]), + $this->createLiveMetadata($component) + ); + + $actualProps = $dehydratedProps->getProps(); + unset($actualProps['@checksum']); + $this->assertSame([ + 'todoList' => [ + 'todoItems' => [ + [ + 'todoList' => null, + 'name' => 'Roll the trash can around for awhile', + 'id' => null, + ], + ], + 'id' => null, + 'listTitle' => 'My Todo List', + ], + ], $actualProps); + } + public function testPassingArrayToWritablePropForHydrationIsNotAllowed(): void { $component = new class() { @@ -1082,7 +1181,7 @@ public function testInvalidTypeHydration(callable $testFactory, int $minPhpVersi public function provideInvalidHydrationTests(): iterable { - yield 'invalid_types_string_to_number_becomes_zero' => [function () { + yield 'invalid_types_string_to_number_uses_coerced' => [function () { return HydrationTest::create(new class() { #[LiveProp(writable: true)] public int $count; @@ -1090,6 +1189,7 @@ public function provideInvalidHydrationTests(): iterable ->mountWith(['count' => 1]) ->userUpdatesProps(['count' => 'pretzels']) ->assertObjectAfterHydration(function (object $object) { + // pretzels is coerced to 0 $this->assertSame(0, $object->count); }); }]; @@ -1124,11 +1224,39 @@ public function provideInvalidHydrationTests(): iterable ->assertObjectAfterHydration(function (object $object) { // change rejected $this->assertSame('oranges', $object->product->name); - // string becomes 0 + // bananas is coerced to 0 $this->assertSame(0, $object->product->price); }); }]; + yield 'nullable_properties_that_cannot_set_null_are_rejected_but_no_error' => [function () { + $todoList = create(TodoListFixtureEntity::class, [ + 'listTitle' => 'My Todo List', + ])->object(); + \assert($todoList instanceof TodoListFixtureEntity); + // make an item with a null "name" + // but, the setName() method does not allow null + $todoItem = new TodoItemFixtureEntity(); + $todoList->addTodoItem($todoItem); + + return HydrationTest::create(new class() { + #[LiveProp()] + public TodoItemFixtureEntity $todoItem; + }) + ->mountWith(['todoItem' => $todoItem]) + ->assertDehydratesTo([ + 'todoItem' => [ + 'todoList' => $todoList->id, + 'name' => null, + 'id' => null, + ], + ]) + ->assertObjectAfterHydration(function (object $component) use ($todoList) { + $this->assertSame($todoList, $component->todoItem->getTodoList()); + $this->assertNull($component->todoItem->getName()); + }); + }]; + yield 'invalid_types_enum_with_an_invalid_value' => [function () { return HydrationTest::create(new class() { #[LiveProp(writable: true)] diff --git a/ux.symfony.com/src/Controller/LiveComponentDemoController.php b/ux.symfony.com/src/Controller/LiveComponentDemoController.php index 3ebf943b676..03070cb99ff 100644 --- a/ux.symfony.com/src/Controller/LiveComponentDemoController.php +++ b/ux.symfony.com/src/Controller/LiveComponentDemoController.php @@ -43,9 +43,9 @@ public function demoFormCollectionType(LiveDemoRepository $liveDemoRepository, R ]); } - return $this->renderForm('live_component_demo/form_collection_type.html.twig', [ + return $this->render('live_component_demo/form_collection_type.html.twig', [ 'form' => $form, - 'todo' => $todoList, + 'todoList' => $todoList, 'demo' => $liveDemoRepository->find('form-collection-type'), ]); } diff --git a/ux.symfony.com/src/Form/TodoItemForm.php b/ux.symfony.com/src/Form/TodoItemForm.php index cb2f4a78f2d..7074856a5b2 100644 --- a/ux.symfony.com/src/Form/TodoItemForm.php +++ b/ux.symfony.com/src/Form/TodoItemForm.php @@ -12,7 +12,11 @@ class TodoItemForm extends AbstractType public function buildForm(FormBuilderInterface $builder, array $options) { $builder - ->add('description') + ->add('description', null, [ + // added because setDescription() doesn't allow null + // it would be simpler to make the arg to that method nullable + 'empty_data' => '', + ]) ->add('priority') ; } diff --git a/ux.symfony.com/src/Form/TodoListForm.php b/ux.symfony.com/src/Form/TodoListForm.php index 7dce05caf4a..49776007afb 100644 --- a/ux.symfony.com/src/Form/TodoListForm.php +++ b/ux.symfony.com/src/Form/TodoListForm.php @@ -15,6 +15,9 @@ public function buildForm(FormBuilderInterface $builder, array $options) $builder ->add('name', null, [ 'label' => 'List name', + // added because setName() doesn't allow null + // it would be simpler to make the arg to that method nullable + 'empty_data' => '', ]) ->add('todoItems', LiveCollectionType::class, [ 'entry_type' => TodoItemForm::class, diff --git a/ux.symfony.com/templates/live_component_demo/form_collection_type.html.twig b/ux.symfony.com/templates/live_component_demo/form_collection_type.html.twig index ab8632ebe9c..267c0856271 100644 --- a/ux.symfony.com/templates/live_component_demo/form_collection_type.html.twig +++ b/ux.symfony.com/templates/live_component_demo/form_collection_type.html.twig @@ -19,7 +19,7 @@ {% block demo_content %} {{ component('todo_list_form', { form: form, - todoList: todo.id ? todo : null + todoList: todoList }) }} {% endblock %}