From 4c96e04a87d3a589daffe2bb8143aaede70f3bc0 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Sat, 17 Oct 2026 14:53:02 +0600 Subject: [PATCH] Fix: Temporal diff() incorrectly compares cast values as strings --- .../Temporal/InteractsWithTemporal.php | 45 +++++++- tests/Model/TemporalModelTest.php | 100 ++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php index b552a6e5..0b7189de 100644 --- a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php +++ b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php @@ -129,7 +129,7 @@ public function diff(string $from, string $to): ?array foreach ($snapshotTo as $col => $newVal) { if (!array_key_exists($col, $snapshotFrom)) { $added[$col] = $newVal; - } elseif ((string) $snapshotFrom[$col] !== (string) $newVal) { + } elseif (self::snapshotValuesDiffer($snapshotFrom[$col], $newVal)) { $changes[$col] = ['from' => $snapshotFrom[$col], 'to' => $newVal]; } } @@ -259,6 +259,49 @@ private function snapshotAt(string $datetime): ?array return json_decode($row['snapshot'], true) ?? null; } + /** + * Determine whether two snapshot values are different + * + * @param mixed $from + * @param mixed $to + * @return bool + */ + private static function snapshotValuesDiffer(mixed $from, mixed $to): bool + { + if (is_array($from) || is_array($to)) { + if (!is_array($from) || !is_array($to) || count($from) !== count($to)) { + return true; + } + + foreach ($from as $key => $value) { + if (!array_key_exists($key, $to) || self::snapshotValuesDiffer($value, $to[$key])) { + return true; + } + } + + return false; + } + + if ($from === null || $to === null) { + return $from !== $to; + } + + if (is_object($from) || is_object($to)) { + return $from != $to; + } + + return self::snapshotScalar($from) !== self::snapshotScalar($to); + } + + /** + * @param bool|int|float|string $value + * @return string + */ + private static function snapshotScalar(mixed $value): string + { + return is_bool($value) ? ($value ? '1' : '0') : (string) $value; + } + /** * Assert that the model is temporal and has a primary key loaded. * diff --git a/tests/Model/TemporalModelTest.php b/tests/Model/TemporalModelTest.php index e604446f..0ab04909 100644 --- a/tests/Model/TemporalModelTest.php +++ b/tests/Model/TemporalModelTest.php @@ -505,6 +505,106 @@ private function registerTemporalHooks(string $class): void $register(); } + public function testDiffDetectsChangesInArrayValues(): void + { + $changes = $this->diffBetween(['tags' => ['a']], ['tags' => ['b']]); + + $this->assertSame(['tags'], array_keys($changes)); + $this->assertSame(['a'], $changes['tags']['from']); + $this->assertSame(['b'], $changes['tags']['to']); + } + + public function testDiffDetectsAddedAndRemovedArrayItemsAndNestedChanges(): void + { + $this->assertArrayHasKey('t', $this->diffBetween(['t' => ['a']], ['t' => ['a', 'b']])); + $this->assertArrayHasKey('t', $this->diffBetween(['t' => ['a', 'b']], ['t' => ['a']])); + $this->assertArrayHasKey('t', $this->diffBetween(['t' => ['k' => ['x' => 1]]], ['t' => ['k' => ['x' => 2]]])); + $this->assertArrayHasKey('t', $this->diffBetween(['t' => []], ['t' => ['a']])); + $this->assertArrayHasKey('t', $this->diffBetween(['t' => ['a']], ['t' => 'a'])); + } + + public function testDiffIgnoresEqualArraysWithoutWarnings(): void + { + $warnings = []; + set_error_handler(function (int $no, string $message) use (&$warnings) { + $warnings[] = $message; + + return true; + }); + + try { + $changes = $this->diffBetween( + ['t' => ['a', 'b'], 'o' => ['x' => 1, 'y' => ['z' => true]]], + ['t' => ['a', 'b'], 'o' => ['y' => ['z' => true], 'x' => 1]] + ); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $changes); + $this->assertSame([], $warnings); + } + + public function testDiffKeepsNullFalseAndEmptyStringApart(): void + { + $this->assertArrayHasKey('v', $this->diffBetween(['v' => null], ['v' => ''])); + $this->assertArrayHasKey('v', $this->diffBetween(['v' => false], ['v' => null])); + $this->assertArrayHasKey('v', $this->diffBetween(['v' => false], ['v' => ''])); + $this->assertArrayHasKey('v', $this->diffBetween(['v' => null], ['v' => 0])); + $this->assertArrayHasKey('v', $this->diffBetween(['v' => true], ['v' => false])); + $this->assertSame([], $this->diffBetween(['v' => null], ['v' => null])); + } + + public function testDiffTreatsDatabaseStyleScalarsAsEqual(): void + { + $this->assertSame([], $this->diffBetween(['n' => 1], ['n' => '1'])); + $this->assertSame([], $this->diffBetween(['n' => true], ['n' => 1])); + $this->assertSame([], $this->diffBetween(['n' => false], ['n' => '0'])); + $this->assertSame([], $this->diffBetween(['n' => 1.0], ['n' => 1])); + $this->assertArrayHasKey('n', $this->diffBetween(['n' => 1], ['n' => 2])); + } + + public function testDiffStillReportsAddedAndRemovedColumns(): void + { + $diff = $this->diffReport(['a' => 1], ['a' => 1, 'b' => ['x']]); + + $this->assertSame(['b' => ['x']], $diff['added']); + $this->assertSame([], $diff['removed']); + + $diff = $this->diffReport(['a' => 1, 'b' => 2], ['a' => 1]); + + $this->assertSame(['b' => 2], $diff['removed']); + } + + private function diffBetween(array $from, array $to): array + { + return $this->diffReport($from, $to)['changes']; + } + + /** + * Store two hand-written snapshots for one record and diff them. + */ + private function diffReport(array $from, array $to): array + { + $this->travelTo('2026-02-01 10:00:00.100000'); + $created = MockTemporalRecord::create(['title' => 'T', 'status' => 'a', 'body' => 'b']); + $record = MockTemporalRecord::find($created->id); + + $history = $record->historyTable(); + $this->pdo->exec("DELETE FROM {$history}"); + + foreach ([[$from, '2026-02-01 10:00:00.000000'], [$to, '2026-02-01 11:00:00.000000']] as [$snapshot, $at]) { + $this->pdo + ->prepare("INSERT INTO {$history} (record_id, action, valid_from, snapshot) VALUES (?, 'updated', ?, ?)") + ->execute([$record->id, $at, json_encode($snapshot)]); + } + + $report = $record->diff('2026-02-01 10:30:00', '2026-02-01 12:00:00'); + $this->pdo->exec('DELETE FROM temporal_records'); + + return $report; + } + private function travelTo(string $datetime): void { Carbon::setTestNow(Carbon::parse($datetime, 'UTC'));