Skip to content

Bug new values incorrect detection #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
darealfive wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Bug new values incorrect detection #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68ded1b
Add `DATE` column `registered_at` of to the table `customer`. Ensures…
darealfive Dec 26, 2025
f3280c8
Fix `AbstractActiveRecord::newValues` detect DateTimeInterface change…
darealfive Dec 28, 2025
812be22
Apply PHP CS Fixer and Rector changes (CI)
darealfive Dec 28, 2025
0533678
Update tests/data/sqlite.sql
darealfive Jan 10, 2026
788da1c
Update tests/Stubs/ActiveRecord/Customer.php
darealfive Jan 10, 2026
bebc575
Update tests/Stubs/ActiveRecord/CustomerClosureField.php
darealfive Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/AbstractActiveRecord.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Yiisoft\ActiveRecord;

use Closure;
use DateTimeInterface;
use InvalidArgumentException;
use LogicException;
use ReflectionClass;
Expand Down Expand Up @@ -116,21 +117,25 @@

public function newValues(?array $propertyNames = null): array
{
$values = $this->propertyValues($propertyNames);

if ($this->oldValues === null) {
return $values;
$currentValues = $this->propertyValues($propertyNames);
if (($oldValues = $this->oldValues()) === []) {
return $currentValues;
}

$result = array_diff_key($values, $this->oldValues);
$newValues = array_diff_key($currentValues, $oldValues);

foreach (array_diff_key($values, $result) as $name => $value) {
if ($value !== $this->oldValues[$name]) {
$result[$name] = $value;
foreach (array_diff_key($currentValues, $newValues) as $name => $newValue) {
if ($newValue instanceof DateTimeInterface) {
if ($oldValues[$name] === null
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the new value is a DateTimeInterface, the code checks if the old value is null but doesn't verify that the old value also implements DateTimeInterface before calling format() on it at line 130. If the old value is a non-null value that doesn't implement DateTimeInterface (e.g., a string), this will cause a fatal error. Consider adding an additional check: || !($oldValues[$name] instanceof DateTimeInterface) to the condition at line 129.

Suggested change
if ($oldValues[$name] === null
if ($oldValues[$name] === null
|| !($oldValues[$name] instanceof DateTimeInterface)

Copilot uses AI. Check for mistakes.
|| $newValue->format('Y-m-d\TH:i:s.uP') != $oldValues[$name]->format('Y-m-d\TH:i:s.uP')) {

Check failure on line 130 in src/AbstractActiveRecord.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.1-ubuntu-latest

MixedMethodCall 

src/AbstractActiveRecord.php:130:83: MixedMethodCall: Cannot determine the type of $oldValues[$name] when calling method format (see https://psalm.dev/015)

Check failure on line 130 in src/AbstractActiveRecord.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.3-ubuntu-latest

MixedMethodCall 

src/AbstractActiveRecord.php:130:83: MixedMethodCall: Cannot determine the type of $oldValues[$name] when calling method format (see https://psalm.dev/015)

Check failure on line 130 in src/AbstractActiveRecord.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.2-ubuntu-latest

MixedMethodCall 

src/AbstractActiveRecord.php:130:83: MixedMethodCall: Cannot determine the type of $oldValues[$name] when calling method format (see https://psalm.dev/015)

Check failure on line 130 in src/AbstractActiveRecord.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.4-ubuntu-latest

MixedMethodCall 

src/AbstractActiveRecord.php:130:83: MixedMethodCall: Cannot determine the type of $oldValues[$name] when calling method format (see https://psalm.dev/015)
$newValues[$name] = $newValue;
}
} elseif ($newValue !== $oldValues[$name]) {
$newValues[$name] = $newValue;
}
}

return $result;
return $newValues;
}

public function oldValues(): array
Expand Down
3 changes: 2 additions & 1 deletion tests/ActiveQueryFindTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Yiisoft\ActiveRecord\Tests;

use DateTimeImmutable;
use Yiisoft\ActiveRecord\ActiveQueryInterface;
use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Customer;
use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\CustomerQuery;
Expand All @@ -12,7 +13,6 @@
use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Type;
use InvalidArgumentException;
use Yiisoft\Db\Exception\InvalidConfigException;
use Yiisoft\Db\QueryBuilder\Condition\In;

use function ksort;

Expand Down Expand Up @@ -177,6 +177,7 @@ public function testFindAsArray(): void
'address' => 'address2',
'status' => 1,
'bool_status' => true,
'registered_at' => new DateTimeImmutable('2022-02-02 02:02:02.222222 Europe/Kyiv'),
'profile_id' => null,
], $customer);

Expand Down
45 changes: 42 additions & 3 deletions tests/ActiveQueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
namespace Yiisoft\ActiveRecord\Tests;

use Closure;
use DateInterval;
use DateTimeImmutable;
use DateTimeInterface;
use InvalidArgumentException;
use LogicException;
use PHPUnit\Framework\Attributes\DataProvider;
use Yiisoft\ActiveRecord\ActiveQuery;
use Yiisoft\ActiveRecord\ActiveQueryInterface;
use Yiisoft\ActiveRecord\Internal\ArArrayHelper;
use Yiisoft\ActiveRecord\JoinWith;
use Yiisoft\ActiveRecord\OptimisticLockException;
use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\BitValues;
use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Category;
Expand All @@ -31,7 +33,6 @@
use Yiisoft\ActiveRecord\Tests\Support\DbHelper;
use Yiisoft\ActiveRecord\UnknownPropertyException;
use Yiisoft\Db\Command\AbstractCommand;
use Yiisoft\Db\Exception\Exception;
use Yiisoft\Db\Exception\InvalidCallException;
use Yiisoft\Db\Exception\InvalidConfigException;
use Yiisoft\Db\Expression\Expression;
Expand Down Expand Up @@ -2094,6 +2095,7 @@ public function testPropertyValues(): void
'address' => 'address1',
'status' => 1,
'bool_status' => true,
'registered_at' => new DateTimeImmutable('2011-01-01 01:01:01.111111 Europe/Berlin'),
'profile_id' => 1,
];

Expand All @@ -2117,7 +2119,7 @@ public function testPropertyValuesExcept(): void
{
$customer = Customer::query();

$values = $customer->findByPk(1)->propertyValues(null, ['status', 'bool_status', 'profile_id']);
$values = $customer->findByPk(1)->propertyValues(null, ['status', 'bool_status', 'registered_at', 'profile_id']);

$this->assertEquals(
['id' => 1, 'email' => 'user1@example.com', 'name' => 'user1', 'address' => 'address1'],
Expand Down Expand Up @@ -2148,6 +2150,7 @@ public function testGetOldValues(): void
'address' => 'address1',
'status' => 1,
'bool_status' => true,
'registered_at' => new DateTimeImmutable('2011-01-01 01:01:01.111111 Europe/Berlin'),
'profile_id' => 1,
];

Expand Down Expand Up @@ -2392,6 +2395,41 @@ public function testUnlinkAllAndConditionDelete(): void
$this->assertCount(0, $customer->getExpensiveOrders());
}

public function testNewValuesStaysEmptyOnSameMomentInTime()
{
$this->reloadFixtureAfterTest();

$customerQuery = Customer::query();
$customer = $customerQuery->findByPk(2);
$this->assertEmpty($customer->newValues());

/** @var DateTimeInterface $customerRegisteredAt */
$customerRegisteredAt = $customer->get('registered_at');
$equalRegisteredAt = $customerRegisteredAt->add(new DateInterval('PT0S'));
$this->assertEquals($equalRegisteredAt->format(($format = 'Y-m-d\TH:i:s.uP')), $customerRegisteredAt->format($format));

$customer->set('registered_at', $equalRegisteredAt);
$this->assertEmpty($customer->newValues());
}

public function testNewValuesStaysNotEmptyOnDifferentMomentInTime()
{
$this->reloadFixtureAfterTest();

$customerQuery = Customer::query();
$customer = $customerQuery->findByPk(2);
$this->assertEmpty($customer->newValues());

/** @var DateTimeInterface $customerRegisteredAt */
$customerRegisteredAt = $customer->get('registered_at');
$differentRegisteredAt = $customerRegisteredAt->add(new DateInterval('PT1S'));
$this->assertNotEquals($differentRegisteredAt->format(($format = 'Y-m-d\TH:i:s.uP')), $customerRegisteredAt->format($format));

$customer->set('registered_at', $differentRegisteredAt);
$this->assertSame($customer->oldValues()['registered_at'], $customerRegisteredAt);
$this->assertSame($customer->newValues()['registered_at'], $differentRegisteredAt);
}

public function testUpdate(): void
{
$this->reloadFixtureAfterTest();
Expand All @@ -2400,6 +2438,7 @@ public function testUpdate(): void
$customer = $customerQuery->findByPk(2);
$this->assertInstanceOf(Customer::class, $customer);
$this->assertEquals('user2', $customer->get('name'));
$this->assertEquals(new DateTimeImmutable('2022-02-02 02:02:02.222222 Europe/Kyiv'), $customer->get('registered_at'));
$this->assertFalse($customer->isNew());
$this->assertEmpty($customer->newValues());

Expand Down
54 changes: 54 additions & 0 deletions tests/ActiveRecordTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Yiisoft\ActiveRecord\Tests;

use ArgumentCountError;
use DateTimeImmutable;
use DivisionByZeroError;
use InvalidArgumentException;
use LogicException;
Expand Down Expand Up @@ -896,6 +897,56 @@ public function testPrimaryKeyOldValuesWithoutPrimaryKey(): void
$orderItem->primaryKeyOldValues();
}

public static function providerForNewValues(): array
{
/*
* Defines the same moment in time but in different timezones.
*/
$registeredAt_1 = new DateTimeImmutable('2011-01-01T01:01:01.111111+01:00');
$registeredAt_1_1 = new DateTimeImmutable('2011-01-01T00:01:01.111111+00:00');

return [
'new record 1.0' => [
'id' => null,
'propertyValues' => ['name' => 'user1'],
'expect' => ['name' => 'user1'],
],
'new record 1.1' => [
'id' => null,
'propertyValues' => ['name' => 'user1', 'registered_at' => $registeredAt_1],
'expect' => ['name' => 'user1', 'registered_at' => $registeredAt_1],
],
/*
* This record already has the "$registeredAt_1" date set.
*/
'old record 1.0' => [
'id' => 1,
'propertyValues' => ['name' => 'user1'],
'expect' => [],
],
'old record 1.1' => [
'id' => 1,
'propertyValues' => ['name' => 'user1.1', 'registered_at' => $registeredAt_1],
// We set the same value, so we do not expect any change here.
'expect' => ['name' => 'user1.1'],
],
'old record 1.2' => [
'id' => 1,
'propertyValues' => ['name' => 'user1.2', 'registered_at' => $registeredAt_1_1],
// We set the same moment in time but with different timezone. It will be detected as a new value.
'expect' => ['name' => 'user1.2', 'registered_at' => $registeredAt_1_1],
],
];
}

#[DataProvider('providerForNewValues')]
public function testNewValues(?int $id, array $propertyValues, array $expect): void
{
$customer = $id === null ? new Customer() : Customer::findByPk($id);
array_walk($propertyValues, static fn($value, string $key) => $customer->set($key, $value));
$this->assertSame($expect, $customer->newValues(array_keys($propertyValues)));
}

public function testGetDirtyValuesOnNewRecord(): void
{
$this->reloadFixtureAfterTest();
Expand All @@ -908,6 +959,7 @@ public function testGetDirtyValuesOnNewRecord(): void
'address' => null,
'status' => 0,
'bool_status' => false,
'registered_at' => null,
'profile_id' => null,
],
$customer->newValues(),
Expand All @@ -916,6 +968,7 @@ public function testGetDirtyValuesOnNewRecord(): void
$customer->set('name', 'Adam');
$customer->set('email', 'adam@example.com');
$customer->set('address', null);
$customer->set('registered_at', ($registeredAt = new DateTimeImmutable('2026-01-01 12:12:12.123456 Europe/Berlin')));

$this->assertSame([], $customer->newValues([]));

Expand All @@ -926,6 +979,7 @@ public function testGetDirtyValuesOnNewRecord(): void
'address' => null,
'status' => 0,
'bool_status' => false,
'registered_at' => $registeredAt,
'profile_id' => null,
],
$customer->newValues(),
Expand Down
3 changes: 3 additions & 0 deletions tests/ArrayableTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public function testFields(): void
'address' => 'address',
'status' => 'status',
'bool_status' => 'bool_status',
'registered_at' => 'registered_at',
'profile_id' => 'profile_id',
'item' => 'item',
'items' => 'items',
Expand All @@ -46,6 +47,7 @@ public function testToArray(): void
'address' => 'address1',
'status' => 1,
'bool_status' => true,
'registered_at' => '2011-01-01T01:01:01.111111+01:00',
'profile_id' => 1,
],
$customer->toArray(),
Expand All @@ -65,6 +67,7 @@ public function testToArrayWithClosure(): void
'address' => 'address1',
'status' => 'active',
'bool_status' => true,
'registered_at' => '2011-01-01T01:01:01.111111+01:00',
'profile_id' => 1,
],
$customer->toArray(),
Expand Down
19 changes: 19 additions & 0 deletions tests/Stubs/ActiveRecord/Customer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord;

use DateTimeImmutable;
use Yiisoft\ActiveRecord\ActiveQuery;
use Yiisoft\ActiveRecord\ActiveQueryInterface;
use Yiisoft\ActiveRecord\ActiveRecordInterface;
Expand All @@ -29,13 +30,21 @@ class Customer extends ArrayableActiveRecord
protected ?string $address = null;
protected ?int $status = 0;
protected bool|int|null $bool_status = false;
protected ?DateTimeImmutable $registered_at = null;
protected ?int $profile_id = null;

public function tableName(): string
{
return 'customer';
}

public function fields(): array
{
return array_merge(parent::fields(), [
'registered_at' => static fn(self $customer) => $customer->registered_at?->format('Y-m-d\TH:i:s.uP'),
]);
}

public function relationQuery(string $name): ActiveQueryInterface
{
return match ($name) {
Expand Down Expand Up @@ -88,6 +97,11 @@ public function getBoolStatus(): ?bool
return $this->bool_status;
}

public function getRegisteredAt(): ?DateTimeImmutable
{
return $this->registered_at;
}

public function getProfileId(): ?int
{
return $this->profile_id;
Expand Down Expand Up @@ -127,6 +141,11 @@ public function setBoolStatus(?bool $bool_status): void
$this->bool_status = $bool_status;
}

public function setRegisteredAt(?DateTimeImmutable $registered_at): void
{
$this->registered_at = $registered_at;
}

public function setProfileId(?int $profile_id): void
{
$this->set('profile_id', $profile_id);
Expand Down
14 changes: 9 additions & 5 deletions tests/Stubs/ActiveRecord/CustomerClosureField.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord;

use DateTimeImmutable;
use Yiisoft\ActiveRecord\Tests\Stubs\ArrayableActiveRecord;

/**
Expand All @@ -17,6 +18,7 @@ final class CustomerClosureField extends ArrayableActiveRecord
protected ?string $address = null;
protected ?int $status = 0;
protected bool|string|null $bool_status = false;
protected ?DateTimeImmutable $registered_at = null;
protected ?int $profile_id = null;

public function tableName(): string
Expand All @@ -26,10 +28,12 @@ public function tableName(): string

public function fields(): array
{
$fields = parent::fields();

$fields['status'] = static fn(self $customer) => $customer->status === 1 ? 'active' : 'inactive';

return $fields;
return array_merge(
parent::fields(),
[
'status' => static fn(self $customer) => $customer->status === 1 ? 'active' : 'inactive',
'registered_at' => static fn(self $customer) => $customer->registered_at?->format('Y-m-d\TH:i:s.uP'),
],
);
}
}
7 changes: 4 additions & 3 deletions tests/data/sqlite.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ CREATE TABLE "customer" (
address text,
status INTEGER DEFAULT 0,
bool_status bool DEFAULT FALSE,
registered_at DATETIME DEFAULT NULL,
profile_id INTEGER,
PRIMARY KEY (id)
);
Expand Down Expand Up @@ -221,9 +222,9 @@ INSERT INTO "animal" ("type") VALUES ('Yiisoft\ActiveRecord\Tests\Stubs\ActiveRe
INSERT INTO "profile" (description) VALUES ('profile customer 1');
INSERT INTO "profile" (description) VALUES ('profile customer 3');

INSERT INTO "customer" (email, name, address, status, bool_status, profile_id) VALUES ('user1@example.com', 'user1', 'address1', 1, 1, 1);
INSERT INTO "customer" (email, name, address, status, bool_status) VALUES ('user2@example.com', 'user2', 'address2', 1, 1);
INSERT INTO "customer" (email, name, address, status, bool_status, profile_id) VALUES ('user3@example.com', 'user3', 'address3', 2, 0, 2);
INSERT INTO "customer" (email, name, address, status, bool_status, registered_at, profile_id) VALUES ('user1@example.com', 'user1', 'address1', 1, 1, '2011-01-01 01:01:01.111111+01:00',1);
INSERT INTO "customer" (email, name, address, status, bool_status, registered_at) VALUES ('user2@example.com', 'user2', 'address2', 1, 1,'2022-02-02 02:02:02.222222+02:00');
INSERT INTO "customer" (email, name, address, status, bool_status, registered_at, profile_id) VALUES ('user3@example.com', 'user3', 'address3', 2, 0,'2023-03-03 03:03:03.333333+03:00', 2);

INSERT INTO "category" (name) VALUES ('Books');
INSERT INTO "category" (name) VALUES ('Movies');
Expand Down
Loading