Skip to content

Fix: EntityCreator::fill() triggers premature initialization of non-nullable wrapped properties#811

Open
patrikvalentaa wants to merge 1 commit intonextras:mainfrom
patrikvalentaa:main
Open

Fix: EntityCreator::fill() triggers premature initialization of non-nullable wrapped properties#811
patrikvalentaa wants to merge 1 commit intonextras:mainfrom
patrikvalentaa:main

Conversation

@patrikvalentaa
Copy link
Copy Markdown

Problem

When using EntityCreator::create() (or fill()) to create a test entity, properties wrapped by non-relationship wrappers (e.g. DateTimeWrapper) get prematurely initialized with null, which throws NullValueException for non-nullable properties — even when a valid value is being passed in $params.

Reproduction

Given an entity with a non-nullable DateTimeImmutable property:

/**
 * @property DateTimeImmutable $createdAt
 */
class MyEntity extends Entity {}

Calling:

$creator->create(MyEntity::class, [
    'createdAt' => new DateTimeImmutable(),
]);

throws:

Nextras\Orm\Exception\NullValueException: Property MyEntity::$createdAt is not nullable.

Root cause

In EntityCreator::fill(), for every property with a wrapper, $entity->getProperty($key) is called to detect whether the wrapper is an IRelationshipCollection:

if ($property->wrapper !== null) {
    $realProperty = $entity->getProperty($key);
    if ($realProperty instanceof IRelationshipCollection) {
        $realProperty->set($value);
        continue;
    }
}

$entity->setReadOnlyValue($key, $value);

getProperty() calls initProperty(), which calls setRawValue(null) on the wrapper since no value has been set yet. For wrappers like DateTimeWrapper, convertFromRawValue(null) throws NullValueException for non-nullable properties — before the actual value from $params ever reaches the entity.

This affects all non-nullable properties using a value wrapper (date/time, custom value wrappers, etc.) when used with EntityCreator.

Fix

Only call getProperty() when the property is a collection relationship (ONE_HAS_MANY or MANY_HAS_MANY) — those are the only relationships using IRelationshipCollection. The relationship type is available on $property->relationship->type from metadata, so we don't need to instantiate the wrapper to know whether to call set() on it.

if (
    $property->relationship !== null
    && in_array($property->relationship->type, [
        PropertyRelationshipMetadata::ONE_HAS_MANY,
        PropertyRelationshipMetadata::MANY_HAS_MANY,
    ], true)
) {
    $realProperty = $entity->getProperty($key);
    if ($realProperty instanceof IRelationshipCollection) {
        $realProperty->set($value);
        continue;
    }
}

$entity->setReadOnlyValue($key, $value);

For all other wrapped properties, setReadOnlyValue($key, $value) is called directly with the user-supplied value, which goes through normal initialization with the correct value rather than null.

Notes

  • Behavior for relationship collections is unchanged.
  • For non-collection wrapped properties (incl. DateTimeWrapper, EnumWrapper, custom ImmutableValuePropertyWrapper subclasses), the value supplied in $params is now used to initialize the property directly, instead of being preceded by an aborted null-init.
  • Detected on PHP 8.5 / nextras/orm v5.1, but the bug is version-agnostic.

…operties in EntityCreator

EntityCreator::fill() called $entity->getProperty($key) for every wrapped
property to detect IRelationshipCollection. This triggered initProperty()
with a null raw value before the user-supplied value from $params was
applied, causing NullValueException for non-nullable properties using
value wrappers (e.g. DateTimeWrapper on a non-nullable createdAt).

Restrict the getProperty() call to collection relationships
(ONE_HAS_MANY, MANY_HAS_MANY) — the only ones backed by
IRelationshipCollection — using metadata instead of instantiating the
wrapper. Non-collection wrapped properties are now initialized directly
via setReadOnlyValue() with the supplied value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant