yiisoft / yiisoft/active-record
`AbstractActiveRecord::newValues()` incorrectly marks DateTimeInterface attribute as changed
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 119
- Forks
- 38
- Avg merge
- 1h 11m
- Merged PRs (30d)
- 1
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)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with newValues() in src/AbstractActiveRecord.php and compare its current DateTimeInterface handling with the issue's reproduction. Check the corresponding newValues documentation in Yiisoft\ActiveRecord\ActiveRecordInterface. Done means equivalent DateTimeImmutable values are not reported as dirty while other values retain strict comparison behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100