-
-
Notifications
You must be signed in to change notification settings - Fork 36
Open
Labels
type:bugBugBug
Description
Description
The newValues() method fails to correctly detect changes for DateTimeType related types, e.g. DateTimeInterface respectively DateTimeImmutable.
Specifically:
DateTimeImmutable/DateTimeInterfacevalues are compared using strict comparison (!==), causing two instances representing the same moment to be always treated as changed (incorrectly), because two objects are never object-equal.
This affects Active Record dirty-checking and can cause unnecessary record updates and therefore subsequent problems.
Steps to Reproduce
- Load an Active Record model from the database with at least one DateTime column and make sure it gets mapped to
DateTimeImmutable. - ...either assign a new
DateTimeImmutableinstance with the same date/time value.
... or leave the value as is. - Call
newValues()or attempt to save the model.Expected:
The attribute should not be marked as dirty.
Actual:
The attribute is considered as changed (dirty) incorrectly.
Expected Behavior
DateTimeInterfacevalues should be compared using loose comparison (==) to compare actual date/time values rather than object identity.
Proposed Fix
/**
* Returns the property values that have been modified since they're loaded or saved most recently.
*
* The comparison of new DateTimeInterface objects with old values uses `==`.
* The comparison of all other values uses `===`.
*
* @param array|null $propertyNames The names of the properties whose values may be returned if they're changed
* recently. If `null`, {@see propertyNames()} will be used.
*
* @return array The changed property values (name-value pairs).
*
* @psalm-return array<string, mixed>
*/
#[Override]
public function newValues(array|null $propertyNames = null): array
{
$values = $this->propertyValues($propertyNames);
$oldValues = $this->oldValues();
if ($oldValues === []) {
return $values;
}
$result = array_diff_key($values, $oldValues);
foreach (array_diff_key($values, $result) as $name => $value) {
if ($value instanceof DateTimeInterface) {
if ($value != $oldValues[$name]) {
$result[$name] = $value;
}
} else {
if ($value !== $oldValues[$name]) {
$result[$name] = $value;
}
}
}
return $result;
}Notes
I think this behavior avoids false positives when using immutable date/time objects.
You may update \Yiisoft\ActiveRecord\ActiveRecordInterface::newValues documentation with the provided one and leave the actual implementation documentation empty (as it is currently).
Package version
1.0.0
PHP version
PHP 8.4.15 (cli)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
type:bugBugBug