From d76087fdcbba8ae567d80b5aaf2c7572906fbfd6 Mon Sep 17 00:00:00 2001 From: Patrik Valenta Date: Thu, 16 Apr 2026 22:51:59 +0200 Subject: [PATCH] entity: strengthen the setRawValue() contract to avoid value validation --- docs/menu.md | 1 + docs/migrate_6.0.md | 13 +++ src/Entity/IProperty.php | 17 +++- src/Entity/ImmutableValuePropertyWrapper.php | 39 +++++++- .../PropertyWrapper/BackedEnumWrapper.php | 3 +- .../PropertyWrapper/DateTimeWrapper.php | 4 - src/Entity/PropertyWrapper/DateWrapper.php | 4 - .../integration/TestHelper/entityCreator.phpt | 45 +++++++++ .../Entity/ImmutableValuePropertyWrapper.phpt | 97 +++++++++++++++++++ 9 files changed, 204 insertions(+), 19 deletions(-) create mode 100644 docs/migrate_6.0.md create mode 100644 tests/cases/integration/TestHelper/entityCreator.phpt create mode 100644 tests/cases/unit/Entity/ImmutableValuePropertyWrapper.phpt diff --git a/docs/menu.md b/docs/menu.md index 324216318..950ae814d 100644 --- a/docs/menu.md +++ b/docs/menu.md @@ -21,3 +21,4 @@ - [UniqueConstraint](unique-constraint) - [Migration to 5.0](migrate_5.0) +- [Migration to 6.0](migrate_6.0) diff --git a/docs/migrate_6.0.md b/docs/migrate_6.0.md new file mode 100644 index 000000000..24f977e9e --- /dev/null +++ b/docs/migrate_6.0.md @@ -0,0 +1,13 @@ +## Migration Guide for 6.0 + +### BC Breaks + +- **`IProperty::setRawValue()` must not validate the value** - the method now only stores the passed raw value; it must neither convert it to the runtime representation nor validate it. Both the conversion and any validation are deferred to read time (`getInjectedValue()` / `getRawValue()`). + + This fixes premature initialization of non-nullable wrapped properties — reading such a property via `IEntity::getProperty()`, or creating an entity through `EntityCreator`, no longer triggers validation before the actual value is set. + + If you maintain a **custom property wrapper**: + - Any validation previously performed in `setRawValue()` must be moved to read time — `setRawValue()` may be called with a value that is not yet known to be valid (or that is about to be overwritten before it is ever read). + - `Nextras\Orm\Entity\ImmutableValuePropertyWrapper` already implements this: it keeps the raw value, converts it lazily via `convertFromRawValue()` on read, and validates nullability centrally. Your `convertFromRawValue()` is free to validate the (converted) value as before — it is simply invoked lazily now. + + As a consequence, an invalid raw value now surfaces its exception when the value is **read** rather than when it is **set** (i.e. during hydration / `setRawValue()`). diff --git a/src/Entity/IProperty.php b/src/Entity/IProperty.php index 7697706d4..f34018b85 100644 --- a/src/Entity/IProperty.php +++ b/src/Entity/IProperty.php @@ -26,19 +26,28 @@ public function convertToRawValue($value); /** * Sets a fetched raw value from a storage. - * Calling this method directly may BREAK things. - * Implementation must not require entity instance. + * + * Expectations: + * - Calling this method directly may BREAK things. + * - Implementation must not require entity instance. + * - Implementation must not validate the value (e.g. nullability of a non-nullable property); a raw value is set + * during property instance creation and the missing value reflected as null is passed. Any validation is + * therefore deferred to read time ({@see getRawValue()} / value read). + * * This method is not symmetric to {@see getRawValue()}. - * @param mixed $value + * * @internal + * @param mixed $value */ public function setRawValue($value): void; /** * Returns raw value. - * Raw value is a normalized value which is suitable for storing. + * + * Raw value is a normalized and validated value which is suitable for storing. * This method is not symmetric to {@see setRawValue()}. + * * @return mixed */ public function getRawValue(); diff --git a/src/Entity/ImmutableValuePropertyWrapper.php b/src/Entity/ImmutableValuePropertyWrapper.php index 4560b0bec..22b2a06d1 100644 --- a/src/Entity/ImmutableValuePropertyWrapper.php +++ b/src/Entity/ImmutableValuePropertyWrapper.php @@ -4,13 +4,20 @@ use Nextras\Orm\Entity\Reflection\PropertyMetadata; +use Nextras\Orm\Exception\NullValueException; abstract class ImmutableValuePropertyWrapper implements IPropertyContainer { - /** @var mixed */ + /** @var mixed converted runtime value; meaningful only when {@see $validated} is true */ protected $value; + /** @var mixed raw value pending conversion; meaningful only when {@see $validated} is false */ + private $rawValue; + + /** whether the currently held value has already been converted and validated */ + private bool $validated = true; + public function __construct( protected readonly PropertyMetadata $propertyMetadata, @@ -19,40 +26,62 @@ public function __construct( } + /** + * Sets a fetched raw value. + * + * The raw value is stored as-is; it is neither converted nor validated here. Both the conversion to the runtime + * representation and the nullability validation are deferred to read time ({@see getInjectedValue()} / + * {@see getRawValue()}), therefore a raw value may be set even before it is known whether it is valid. + */ public function setRawValue($value): void { - $this->value = $this->convertFromRawValue($value); + $this->rawValue = $value; + $this->validated = false; } public function getRawValue() { - return $this->convertToRawValue($this->value); + return $this->convertToRawValue($this->getInjectedValue()); } public function setInjectedValue($value): bool { $this->value = $value; + $this->rawValue = null; + $this->validated = true; return true; } public function hasInjectedValue(): bool { - return $this->value !== null; + return ($this->validated ? $this->value : $this->rawValue) !== null; } public function &getInjectedValue() { + if (!$this->validated) { + $this->value = $this->convertFromRawValue($this->rawValue); + $this->rawValue = null; + $this->validated = true; + } + if ($this->value === null && !$this->propertyMetadata->isNullable) { + throw new NullValueException($this->propertyMetadata); + } return $this->value; } /** * Converts passed value from raw value suitable for storing to runtime representation. - * Implementation must not require entity instance. + * + * - Implementation must not require entity instance. + * - The conversion is performed lazily on read; it must return null for a null input (nullability is validated + * centrally) but is allowed to throw for an otherwise invalid value. + * * @param mixed $value * @return mixed * @internal diff --git a/src/Entity/PropertyWrapper/BackedEnumWrapper.php b/src/Entity/PropertyWrapper/BackedEnumWrapper.php index 4be8c02f9..16fd34ecd 100644 --- a/src/Entity/PropertyWrapper/BackedEnumWrapper.php +++ b/src/Entity/PropertyWrapper/BackedEnumWrapper.php @@ -44,8 +44,7 @@ public function convertToRawValue(mixed $value): mixed public function convertFromRawValue(mixed $value): ?BackedEnum { if ($value === null) { - if ($this->propertyMetadata->isNullable) return null; - throw new NullValueException($this->propertyMetadata); + return null; } $type = array_key_first($this->propertyMetadata->types); diff --git a/src/Entity/PropertyWrapper/DateTimeWrapper.php b/src/Entity/PropertyWrapper/DateTimeWrapper.php index 2e9f59eb2..509c5a64a 100644 --- a/src/Entity/PropertyWrapper/DateTimeWrapper.php +++ b/src/Entity/PropertyWrapper/DateTimeWrapper.php @@ -49,10 +49,6 @@ public function convertToRawValue(mixed $value): mixed public function convertFromRawValue($value) { - if ($value === null && !$this->propertyMetadata->isNullable) { - throw new NullValueException($this->propertyMetadata); - } - // The string conversion from raw values is used when using {default} modifier in property definition. // This string value is considered to be a raw value. if (is_string($value)) { diff --git a/src/Entity/PropertyWrapper/DateWrapper.php b/src/Entity/PropertyWrapper/DateWrapper.php index f25a92250..03ed539bd 100644 --- a/src/Entity/PropertyWrapper/DateWrapper.php +++ b/src/Entity/PropertyWrapper/DateWrapper.php @@ -49,10 +49,6 @@ public function convertToRawValue(mixed $value): mixed public function convertFromRawValue($value) { - if ($value === null && !$this->propertyMetadata->isNullable) { - throw new NullValueException($this->propertyMetadata); - } - // The string conversion from raw values is used when using {default} modifier in property definition. // This string value is considered to be a raw value. if (is_string($value)) { diff --git a/tests/cases/integration/TestHelper/entityCreator.phpt b/tests/cases/integration/TestHelper/entityCreator.phpt new file mode 100644 index 000000000..b48c784e9 --- /dev/null +++ b/tests/cases/integration/TestHelper/entityCreator.phpt @@ -0,0 +1,45 @@ +container->getByType(EntityCreator::class); + + $repliedAt = new DateTimeImmutable('2020-01-01 18:00:00'); + $comment = $creator->create(Comment::class, [ + 'thread' => new Thread(), + 'repliedAt' => $repliedAt, + ]); + + Assert::type(Comment::class, $comment); + Assert::equal($repliedAt, $comment->repliedAt); + } +} + + +$test = new EntityCreatorTest(); +$test->run(); diff --git a/tests/cases/unit/Entity/ImmutableValuePropertyWrapper.phpt b/tests/cases/unit/Entity/ImmutableValuePropertyWrapper.phpt new file mode 100644 index 000000000..6ef367e61 --- /dev/null +++ b/tests/cases/unit/Entity/ImmutableValuePropertyWrapper.phpt @@ -0,0 +1,97 @@ +createMetadata(isNullable: false)); + + Assert::noError(function () use ($wrapper): void { + $wrapper->setRawValue(null); + }); + } + + + /** + * Nullability of a non-nullable property is validated lazily, on read. + */ + public function testReadValidatesNullOnNonNullable(): void + { + $wrapper = new DateTimeWrapper($this->createMetadata(isNullable: false)); + $wrapper->setRawValue(null); + + Assert::throws(function () use ($wrapper): void { + $wrapper->getRawValue(); + }, NullValueException::class); + + Assert::throws(function () use ($wrapper): void { + $wrapper->getInjectedValue(); + }, NullValueException::class); + } + + + public function testNullableAllowsNull(): void + { + $wrapper = new DateTimeWrapper($this->createMetadata(isNullable: true)); + $wrapper->setRawValue(null); + + Assert::null($wrapper->getRawValue()); + Assert::null($wrapper->getInjectedValue()); + } + + + /** + * The conversion of the raw value is deferred to read time; a malformed raw value therefore does not throw when + * set, only when it is actually read. + */ + public function testConversionIsDeferredToRead(): void + { + $wrapper = new DateTimeWrapper($this->createMetadata(isNullable: true)); + + Assert::noError(function () use ($wrapper): void { + $wrapper->setRawValue(''); + }); + + Assert::throws(function () use ($wrapper): void { + $wrapper->getInjectedValue(); + }, InvalidPropertyValueException::class); + } + + + private function createMetadata(bool $isNullable): PropertyMetadata + { + $metadata = Mockery::mock(PropertyMetadata::class); + $metadata->name = 'createdAt'; + $metadata->isNullable = $isNullable; + $metadata->types = [DateTimeImmutable::class => true]; + return $metadata; + } +} + + +$test = new ImmutableValuePropertyWrapperTest(); +$test->run();