From 8897cde1aa20a255568e76f7a80ae5b38e17a5f7 Mon Sep 17 00:00:00 2001 From: Abdul Majid <43958759+abdulmajidcse@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:54:48 +0600 Subject: [PATCH 1/4] (feat) Branchable temporal records with explainable history Add named branches to #[Temporal] models: a branch can be forked from a point-in-time snapshot of the main timeline and queried independently via Model::at($datetime, $branch) / $model->history($branch), without touching main history. Restoring from any branch still only ever appends a new version through the normal save()/snapshot path; history is never rewritten. Also fix diff() to run both snapshots through the model's casts and unexposable (hidden) attribute filtering instead of comparing raw stored values, so hidden fields no longer leak into diffs and cast values compare in their proper type. Branch support is fully opt-in via #[Temporal(branches: true)]; existing temporal models are unaffected. MigrateTemporalCommand and the per-driver DDL generators gained the matching branch column/index for MySQL, PostgreSQL and SQLite, following the same pattern already used for the actor column. --- phpstan-baseline.neon | 18 + .../Migrations/MigrateTemporalCommand.php | 38 +- .../Database/Temporal/Attributes/Temporal.php | 2 + .../Temporal/InteractsWithTemporal.php | 194 ++++++++--- .../Database/Temporal/TemporalBuilder.php | 76 +++- .../Database/Temporal/TemporalManager.php | 172 ++++++++-- tests/Console/MigrateTemporalCommandTest.php | 184 ++++++++++ tests/Model/TemporalBranchTest.php | 324 ++++++++++++++++++ tests/Support/Model/MockBranchableRecord.php | 21 ++ tests/Support/Model/MockMigratableRecord.php | 16 + 10 files changed, 965 insertions(+), 80 deletions(-) create mode 100644 tests/Model/TemporalBranchTest.php create mode 100644 tests/Support/Model/MockBranchableRecord.php create mode 100644 tests/Support/Model/MockMigratableRecord.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f0365c85..ab89bc04 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -258,6 +258,12 @@ parameters: count: 1 path: src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php + - + message: '#^Using nullsafe property access "\?\-\>branches" on left side of \?\? is unnecessary\. Use \-\> instead\.$#' + identifier: nullsafe.neverNull + count: 1 + path: src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php + - message: '#^Call to an undefined static method Phaseolies\\Support\\Facades\\Route\:\:cacheRoutes\(\)\.$#' identifier: staticMethod.notFound @@ -744,6 +750,12 @@ parameters: count: 1 path: src/Phaseolies/Database/Entity/Model.php + - + message: '#^Access to an undefined property Phaseolies\\Database\\Entity\\Model\:\:\$__branch\.$#' + identifier: property.notFound + count: 1 + path: src/Phaseolies/Database/Entity/Model.php + - message: '#^Access to an undefined property Phaseolies\\Database\\Entity\\Model\:\:\$__changed_cols\.$#' identifier: property.notFound @@ -924,6 +936,12 @@ parameters: count: 1 path: src/Phaseolies/Database/Temporal/TemporalManager.php + - + message: '#^Using nullsafe property access "\?\-\>branches" on left side of \?\? is unnecessary\. Use \-\> instead\.$#' + identifier: nullsafe.neverNull + count: 1 + path: src/Phaseolies/Database/Temporal/TemporalManager.php + - message: '#^Class App\\Http\\Exceptions\\BeforeExceptionHandler not found\.$#' identifier: class.notFound diff --git a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php index 376126a6..9e79aa44 100644 --- a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php +++ b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php @@ -96,10 +96,21 @@ public function handle(): int ); if (!$dryRun && $this->tableExists($pdo, $driver, $historyTable)) { - // Table exists — check if we need to add the actor column + // Table exists — check if we need to add the actor and/or branch columns + $addedColumns = []; + if ($attr?->trackActor && !$this->columnExists($pdo, $driver, $historyTable, 'actor')) { $pdo->exec($this->addActorColumnSql($driver, $historyTable)); - $row('✓', 'green', 'actor column added'); + $addedColumns[] = 'actor'; + } + + if ($attr?->branches && !$this->columnExists($pdo, $driver, $historyTable, 'branch')) { + $pdo->exec($this->addBranchColumnSql($driver, $historyTable)); + $addedColumns[] = 'branch'; + } + + if (!empty($addedColumns)) { + $row('✓', 'green', implode(', ', $addedColumns) . ' column' . (count($addedColumns) > 1 ? 's' : '') . ' added'); $created++; } else { $row('•', 'yellow', 'already exists'); @@ -109,9 +120,10 @@ public function handle(): int } $statements = TemporalManager::createHistoryTableSql( - historyTable: $historyTable, - driver: $driver, - trackActor: $attr?->trackActor ?? false, + historyTable: $historyTable, + driver: $driver, + trackActor: $attr?->trackActor ?? false, + trackBranches: $attr?->branches ?? false, ); if ($dryRun) { @@ -308,4 +320,20 @@ private function addActorColumnSql(string $driver, string $table): string default => "ALTER TABLE `{$table}` ADD COLUMN `actor` VARCHAR(255) NULL", }; } + + /** + * Generate the ALTER TABLE SQL to add the branch column. + * + * @param string $driver + * @param string $table + * @return string + */ + private function addBranchColumnSql(string $driver, string $table): string + { + return match ($driver) { + 'pgsql' => "ALTER TABLE \"{$table}\" ADD COLUMN branch VARCHAR(50) NOT NULL DEFAULT 'main'", + 'sqlite' => "ALTER TABLE \"{$table}\" ADD COLUMN branch TEXT NOT NULL DEFAULT 'main'", + default => "ALTER TABLE `{$table}` ADD COLUMN `branch` VARCHAR(50) NOT NULL DEFAULT 'main'", + }; + } } diff --git a/src/Phaseolies/Database/Temporal/Attributes/Temporal.php b/src/Phaseolies/Database/Temporal/Attributes/Temporal.php index 79eabc84..d412a3ce 100644 --- a/src/Phaseolies/Database/Temporal/Attributes/Temporal.php +++ b/src/Phaseolies/Database/Temporal/Attributes/Temporal.php @@ -10,9 +10,11 @@ final class Temporal /** * @param string $suffix * @param bool $trackActor + * @param bool $branches */ public function __construct( public readonly string $suffix = '_history', public readonly bool $trackActor = false, + public readonly bool $branches = false, ) {} } diff --git a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php index 2b278764..29eaa7bd 100644 --- a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php +++ b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php @@ -34,14 +34,15 @@ protected function registerTemporalHooks(): void } /** - * Return a TemporalBuilder scoped to the given point in time. + * Return a TemporalBuilder scoped to the given point in time (and branch). * * @param string $datetime + * @param string $branch * @return TemporalBuilder * * @throws \RuntimeException */ - public static function at(string $datetime): TemporalBuilder + public static function at(string $datetime, string $branch = 'main'): TemporalBuilder { if (!TemporalManager::isTemporal(static::class)) { throw new \RuntimeException( @@ -49,6 +50,8 @@ public static function at(string $datetime): TemporalBuilder ); } + TemporalManager::assertBranchSupported(static::class, $branch); + $model = new static(); return new TemporalBuilder( @@ -57,29 +60,82 @@ public static function at(string $datetime): TemporalBuilder modelClass: static::class, rowPerPage: $model->pageSize, datetime: $datetime, + branch: $branch, ); } /** - * Return the complete audit history for this model instance + * Fork a new branch from this record's main timeline as it existed at + * the given point in time. The branch is independently queryable via + * static::at($datetime, $name) and never alters the main history. + * + * @param string $name + * @param string $fromDatetime + * @return bool * + * @throws \RuntimeException + */ + public function branch(string $name, string $fromDatetime): bool + { + $this->assertTemporalAndLoaded(); + + if (!TemporalManager::isBranchable(static::class)) { + throw new \RuntimeException( + static::class . ' does not support branches. Add #[Temporal(branches: true)] to enable them.' + ); + } + + if ($name === '' || $name === 'main') { + throw new \InvalidArgumentException('Branch name must be non-empty and cannot be "main".'); + } + + $snapshot = $this->snapshotAt($fromDatetime, 'main'); + + if ($snapshot === null) { + return false; + } + + TemporalManager::snapshotBranch($this, $snapshot, $name); + + return true; + } + + /** + * Return the complete audit history for this model instance on the given branch. + * + * @param string|null $branch Pass null to include every branch. * @return Collection * @throws \RuntimeException */ - public function history(): Collection + public function history(?string $branch = 'main'): Collection { $this->assertTemporalAndLoaded(); $historyTable = $this->historyTable(); $pdo = $this->getConnection(); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); - $stmt = $pdo->prepare( - "SELECT history_id, action, valid_from, snapshot, changed_cols, actor - FROM {$historyTable} - WHERE record_id = ? - ORDER BY valid_from ASC" - ); - $stmt->execute([$this->getKey()]); + $hasActor = TemporalManager::hasActorColumn($pdo, $historyTable, $driver); + $hasBranch = TemporalManager::isBranchable(static::class) + && TemporalManager::hasBranchColumn($pdo, $historyTable, $driver); + + $columns = 'history_id, action, valid_from, snapshot, changed_cols' + . ($hasActor ? ', actor' : '') + . ($hasBranch ? ', branch' : ''); + + $sql = "SELECT {$columns} + FROM {$historyTable} + WHERE record_id = ?" + . ($hasBranch && $branch !== null ? ' AND branch = ?' : '') + . ' ORDER BY valid_from ASC'; + + $params = [$this->getKey()]; + if ($hasBranch && $branch !== null) { + $params[] = $branch; + } + + $stmt = $pdo->prepare($sql); + $stmt->execute($params); $rows = $stmt->fetchAll(PDO::FETCH_ASSOC); $models = []; @@ -96,7 +152,8 @@ public function history(): Collection $entry->__changed_cols = $row['changed_cols'] ? json_decode($row['changed_cols'], true) : null; - $entry->__actor = $row['actor'] ?? null; + $entry->__actor = $row['actor'] ?? null; + $entry->__branch = $row['branch'] ?? 'main'; $models[] = $entry; } @@ -105,37 +162,46 @@ public function history(): Collection } /** - * Compare the model's state at two points in time and return a structured diff. + * Compare the model's state at two points in time on the given branch and + * return a structured diff. Values are run through the model's casts and + * attributes marked unexposable (hidden) are excluded from the result. * * @param string $from * @param string $to + * @param string $branch * @return array|null + * + * @throws \RuntimeException */ - public function diff(string $from, string $to): ?array + public function diff(string $from, string $to, string $branch = 'main'): ?array { $this->assertTemporalAndLoaded(); + TemporalManager::assertBranchSupported(static::class, $branch); - $snapshotFrom = $this->snapshotAt($from); - $snapshotTo = $this->snapshotAt($to); + $snapshotFrom = $this->snapshotAt($from, $branch); + $snapshotTo = $this->snapshotAt($to, $branch); if ($snapshotFrom === null || $snapshotTo === null) { return null; } + $visibleFrom = $this->visibleSnapshot($snapshotFrom); + $visibleTo = $this->visibleSnapshot($snapshotTo); + $changes = []; $added = []; $removed = []; - foreach ($snapshotTo as $col => $newVal) { - if (!array_key_exists($col, $snapshotFrom)) { + foreach ($visibleTo as $col => $newVal) { + if (!array_key_exists($col, $visibleFrom)) { $added[$col] = $newVal; - } elseif ((string) $snapshotFrom[$col] !== (string) $newVal) { - $changes[$col] = ['from' => $snapshotFrom[$col], 'to' => $newVal]; + } elseif ((string) $visibleFrom[$col] !== (string) $newVal) { + $changes[$col] = ['from' => $visibleFrom[$col], 'to' => $newVal]; } } - foreach ($snapshotFrom as $col => $oldVal) { - if (!array_key_exists($col, $snapshotTo)) { + foreach ($visibleFrom as $col => $oldVal) { + if (!array_key_exists($col, $visibleTo)) { $removed[$col] = $oldVal; } } @@ -143,24 +209,52 @@ public function diff(string $from, string $to): ?array return [ 'from' => $from, 'to' => $to, + 'branch' => $branch, 'changes' => $changes, 'added' => $added, 'removed' => $removed, ]; } + /** + * Apply this model's casts and unexposable (hidden) attribute filtering + * to a raw stored snapshot, for display/diffing purposes only. + * + * @param array $snapshot + * @return array + */ + private function visibleSnapshot(array $snapshot): array + { + $probe = new static(); + $visible = []; + + foreach ($snapshot as $key => $value) { + if (in_array($key, $probe->unexposable, true)) { + continue; + } + + $visible[$key] = $probe->castForGet($key, $value); + } + + return $visible; + } + /** * Return a new (unsaved) model instance populated with the state of this - * record at the given point in time. + * record at the given point in time on the given branch. * * @param string $datetime + * @param string $branch * @return static|null + * + * @throws \RuntimeException */ - public function rewindTo(string $datetime): ?static + public function rewindTo(string $datetime, string $branch = 'main'): ?static { $this->assertTemporalAndLoaded(); + TemporalManager::assertBranchSupported(static::class, $branch); - $snapshot = $this->snapshotAt($datetime); + $snapshot = $this->snapshotAt($datetime, $branch); if ($snapshot === null) { return null; @@ -176,15 +270,21 @@ public function rewindTo(string $datetime): ?static } /** - * Restore this model to its state at the given datetime and immediately - * persist the change. + * Restore this model to its state at the given datetime on the given + * branch and immediately persist the change. This always creates a new + * version through the normal save()/snapshot hooks; it never rewrites + * existing history, and restoring from a branch other than "main" + * requires the model to be configured for branches. * * @param string $datetime + * @param string $branch * @return bool + * + * @throws \RuntimeException */ - public function restoreTo(string $datetime): bool + public function restoreTo(string $datetime, string $branch = 'main'): bool { - $rewound = $this->rewindTo($datetime); + $rewound = $this->rewindTo($datetime, $branch); if ($rewound === null) { return false; @@ -224,31 +324,43 @@ public function historyTable(): string } /** - * Fetch the raw snapshot array for this record at the given datetime. + * Fetch the raw snapshot array for this record at the given datetime on + * the given branch. * * @param string $datetime + * @param string $branch * @return array|null */ - private function snapshotAt(string $datetime): ?array + private function snapshotAt(string $datetime, string $branch = 'main'): ?array { $historyTable = $this->historyTable(); $pdo = $this->getConnection(); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + + $hasBranch = TemporalManager::isBranchable(static::class) + && TemporalManager::hasBranchColumn($pdo, $historyTable, $driver); // Normalize to end-of-day when only a date is given if (preg_match('/^\d{4}-\d{2}-\d{2}$/', trim($datetime))) { $datetime .= ' 23:59:59.999999'; } - $stmt = $pdo->prepare( - "SELECT snapshot - FROM {$historyTable} - WHERE record_id = ? - AND valid_from <= ? - AND action != 'deleted' - ORDER BY valid_from DESC - LIMIT 1" - ); - $stmt->execute([$this->getKey(), $datetime]); + $sql = "SELECT snapshot + FROM {$historyTable} + WHERE record_id = ? + AND valid_from <= ? + AND action != 'deleted'" + . ($hasBranch ? ' AND branch = ?' : '') + . ' ORDER BY valid_from DESC + LIMIT 1'; + + $params = [$this->getKey(), $datetime]; + if ($hasBranch) { + $params[] = $branch; + } + + $stmt = $pdo->prepare($sql); + $stmt->execute($params); $row = $stmt->fetch(PDO::FETCH_ASSOC); diff --git a/src/Phaseolies/Database/Temporal/TemporalBuilder.php b/src/Phaseolies/Database/Temporal/TemporalBuilder.php index 420160a7..1a389357 100644 --- a/src/Phaseolies/Database/Temporal/TemporalBuilder.php +++ b/src/Phaseolies/Database/Temporal/TemporalBuilder.php @@ -16,18 +16,33 @@ class TemporalBuilder extends Builder */ protected string $datetime; + /** + * The branch these history queries are scoped to. + * + * @var string + */ + protected string $branch; + /** * @param PDO $pdo * @param string $table * @param string $modelClass - * @param int $rowPerPage + * @param int $rowPerPage * @param string $datetime + * @param string $branch */ - public function __construct(PDO $pdo, string $table, string $modelClass, int $rowPerPage, string $datetime) - { + public function __construct( + PDO $pdo, + string $table, + string $modelClass, + int $rowPerPage, + string $datetime, + string $branch = 'main' + ) { parent::__construct($pdo, $table, $modelClass, $rowPerPage); $this->datetime = $this->normalizeDateTime($datetime); + $this->branch = $branch; } /** @@ -81,20 +96,26 @@ public function first(): ?Model public function find($id): ?Model { $historyTable = TemporalManager::historyTable($this->table, $this->modelClass); - $key = (new $this->modelClass())->getKeyName(); + $hasBranch = $this->hasBranchColumn($historyTable); $sql = " SELECT snapshot FROM {$historyTable} WHERE record_id = ? AND valid_from <= ? - AND action != 'deleted' + AND action != 'deleted'" + . ($hasBranch ? ' AND branch = ?' : '') . " ORDER BY valid_from DESC LIMIT 1 "; + $params = [$id, $this->datetime]; + if ($hasBranch) { + $params[] = $this->branch; + } + $stmt = $this->pdo->prepare($sql); - $stmt->execute([$id, $this->datetime]); + $stmt->execute($params); $row = $stmt->fetch(PDO::FETCH_ASSOC); if (!$row) { @@ -115,6 +136,10 @@ public function find($id): ?Model private function fetchTemporalRows(): array { $historyTable = TemporalManager::historyTable($this->table, $this->modelClass); + $hasBranch = $this->hasBranchColumn($historyTable); + + $innerBranch = $hasBranch ? ' AND h2.branch = ?' : ''; + $outerBranch = $hasBranch ? ' AND h1.branch = ?' : ''; $sql = " SELECT h1.snapshot, h1.record_id, h1.action @@ -123,17 +148,40 @@ private function fetchTemporalRows(): array SELECT MAX(h2.valid_from) FROM {$historyTable} h2 WHERE h2.record_id = h1.record_id - AND h2.valid_from <= ? + AND h2.valid_from <= ?{$innerBranch} ) - AND h1.action != 'deleted' + AND h1.action != 'deleted'{$outerBranch} "; + $params = [$this->datetime]; + if ($hasBranch) { + $params[] = $this->branch; + $params[] = $this->branch; + } + $stmt = $this->pdo->prepare($sql); - $stmt->execute([$this->datetime]); + $stmt->execute($params); return $stmt->fetchAll(PDO::FETCH_ASSOC); } + /** + * Whether the history table has a usable branch column for this model. + * + * @param string $historyTable + * @return bool + */ + private function hasBranchColumn(string $historyTable): bool + { + if (!TemporalManager::isBranchable($this->modelClass)) { + return false; + } + + $driver = $this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + + return TemporalManager::hasBranchColumn($this->pdo, $historyTable, $driver); + } + /** * Hydrate an array of raw history rows into model instances. * @@ -345,4 +393,14 @@ public function getDatetime(): string { return $this->datetime; } + + /** + * Expose the resolved branch for external use. + * + * @return string + */ + public function getBranch(): string + { + return $this->branch; + } } diff --git a/src/Phaseolies/Database/Temporal/TemporalManager.php b/src/Phaseolies/Database/Temporal/TemporalManager.php index 8a1304b3..ab749596 100644 --- a/src/Phaseolies/Database/Temporal/TemporalManager.php +++ b/src/Phaseolies/Database/Temporal/TemporalManager.php @@ -65,14 +65,44 @@ public static function historyTable(string $table, string $class): string return $table . $suffix; } + /** + * Determine whether the given model class supports named branches. + * + * @param string $class + * @return bool + */ + public static function isBranchable(string $class): bool + { + return self::getAttribute($class)?->branches ?? false; + } + + /** + * Guard a branch argument against classes that were not configured for branching. + * + * @param string $class + * @param string $branch + * @return void + * + * @throws \RuntimeException + */ + public static function assertBranchSupported(string $class, string $branch): void + { + if ($branch !== 'main' && !self::isBranchable($class)) { + throw new \RuntimeException( + $class . ' does not support branches. Add #[Temporal(branches: true)] to enable them.' + ); + } + } + /** * Snapshot the model's current state into the history table. * * @param Model $model * @param string $action + * @param string $branch * @return void */ - public static function snapshot(Model $model, string $action): void + public static function snapshot(Model $model, string $action, string $branch = 'main'): void { $pdo = $model->getConnection(); $historyTable = self::historyTable($model->getTable(), get_class($model)); @@ -111,44 +141,109 @@ public static function snapshot(Model $model, string $action): void $data['actor'] = self::resolveActor(); } + if ($attr?->branches && self::hasBranchColumn($pdo, $historyTable, $driver)) { + $data['branch'] = $branch; + } + self::insertHistory($pdo, $historyTable, $data); } /** - * Check whether the `actor` column exists in the given history table + * Snapshot an already-resolved attribute array as a new branch entry. + * + * Used to fork a branch from a point-in-time snapshot of the main + * timeline without disturbing the record's own history. + * + * @param Model $model + * @param array $snapshot + * @param string $branch + * @return void + */ + public static function snapshotBranch(Model $model, array $snapshot, string $branch): void + { + $pdo = $model->getConnection(); + $historyTable = self::historyTable($model->getTable(), get_class($model)); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + + $data = [ + 'record_id' => $model->getKey(), + 'action' => 'branched', + 'valid_from' => self::nowMicro($driver), + 'snapshot' => json_encode($snapshot, JSON_UNESCAPED_UNICODE), + 'changed_cols' => null, + 'branch' => $branch, + ]; + + $attr = self::getAttribute(get_class($model)); + if ($attr?->trackActor && self::hasActorColumn($pdo, $historyTable, $driver)) { + $data['actor'] = self::resolveActor(); + } + + self::insertHistory($pdo, $historyTable, $data); + } + + /** + * Check whether the `actor` column exists in the given history table. * * @param PDO $pdo * @param string $historyTable * @param string $driver * @return bool */ - private static function hasActorColumn(PDO $pdo, string $historyTable, string $driver): bool + public static function hasActorColumn(PDO $pdo, string $historyTable, string $driver): bool { - $cacheKey = $historyTable . '.actor'; + return self::hasColumn($pdo, $historyTable, 'actor', $driver); + } + + /** + * Check whether the `branch` column exists in the given history table. + * + * @param PDO $pdo + * @param string $historyTable + * @param string $driver + * @return bool + */ + public static function hasBranchColumn(PDO $pdo, string $historyTable, string $driver): bool + { + return self::hasColumn($pdo, $historyTable, 'branch', $driver); + } + + /** + * Check whether the given column exists in the given history table. + * + * @param PDO $pdo + * @param string $historyTable + * @param string $column + * @param string $driver + * @return bool + */ + private static function hasColumn(PDO $pdo, string $historyTable, string $column, string $driver): bool + { + $cacheKey = $historyTable . '.' . $column; if (!array_key_exists($cacheKey, self::$columnCache)) { try { self::$columnCache[$cacheKey] = match ($driver) { - 'pgsql' => (function () use ($pdo, $historyTable): bool { + 'pgsql' => (function () use ($pdo, $historyTable, $column): bool { $stmt = $pdo->prepare( "SELECT COUNT(*) FROM information_schema.columns - WHERE table_name = ? AND column_name = 'actor'" + WHERE table_name = ? AND column_name = ?" ); - $stmt->execute([$historyTable]); + $stmt->execute([$historyTable, $column]); return (bool) $stmt->fetchColumn(); })(), - 'sqlite' => (function () use ($pdo, $historyTable): bool { + 'sqlite' => (function () use ($pdo, $historyTable, $column): bool { $stmt = $pdo->query("PRAGMA table_info(\"{$historyTable}\")"); foreach ($stmt->fetchAll(PDO::FETCH_ASSOC) as $col) { - if ($col['name'] === 'actor') { + if ($col['name'] === $column) { return true; } } return false; })(), - default => (function () use ($pdo, $historyTable): bool { - $stmt = $pdo->prepare("SHOW COLUMNS FROM `{$historyTable}` LIKE 'actor'"); - $stmt->execute(); + default => (function () use ($pdo, $historyTable, $column): bool { + $stmt = $pdo->prepare("SHOW COLUMNS FROM `{$historyTable}` LIKE ?"); + $stmt->execute([$column]); return $stmt->fetch() !== false; })(), }; @@ -215,21 +310,29 @@ private static function resolveActor(): mixed * @param string $historyTable * @param string $driver * @param bool $trackActor + * @param bool $trackBranches * @return string[] */ - public static function createHistoryTableSql(string $historyTable, string $driver, bool $trackActor = false): array - { - $actorCol = $trackActor ? self::actorColumnSql($driver) : ''; + public static function createHistoryTableSql( + string $historyTable, + string $driver, + bool $trackActor = false, + bool $trackBranches = false + ): array { + $actorCol = $trackActor ? self::actorColumnSql($driver) : ''; + $branchCol = $trackBranches ? self::branchColumnSql($driver) : ''; return match ($driver) { - 'pgsql' => self::pgsqlDdl($historyTable, $actorCol), - 'sqlite' => self::sqliteDdl($historyTable, $actorCol), - default => self::mysqlDdl($historyTable, $actorCol), + 'pgsql' => self::pgsqlDdl($historyTable, $actorCol, $branchCol, $trackBranches), + 'sqlite' => self::sqliteDdl($historyTable, $actorCol, $branchCol, $trackBranches), + default => self::mysqlDdl($historyTable, $actorCol, $branchCol, $trackBranches), }; } - private static function mysqlDdl(string $table, string $actorCol): array + private static function mysqlDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { + $branchIndex = $trackBranches ? ",\n INDEX `idx_branch` (`record_id`, `branch`)" : ''; + return [ "CREATE TABLE IF NOT EXISTS `{$table}` ( `history_id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT, @@ -239,14 +342,15 @@ private static function mysqlDdl(string $table, string $actorCol): array `snapshot` JSON NOT NULL, `changed_cols` JSON NULL, {$actorCol} + {$branchCol} PRIMARY KEY (`history_id`), INDEX `idx_record_id` (`record_id`), - INDEX `idx_valid_from` (`valid_from`) + INDEX `idx_valid_from` (`valid_from`){$branchIndex} ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci" ]; } - private static function pgsqlDdl(string $table, string $actorCol): array + private static function pgsqlDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { $stmts = [ "CREATE TABLE IF NOT EXISTS \"{$table}\" ( @@ -256,16 +360,20 @@ private static function pgsqlDdl(string $table, string $actorCol): array valid_from TIMESTAMPTZ NOT NULL, snapshot JSONB NOT NULL, changed_cols JSONB NULL - {$actorCol} + {$actorCol}{$branchCol} )", "CREATE INDEX IF NOT EXISTS \"idx_{$table}_record_id\" ON \"{$table}\" (record_id)", "CREATE INDEX IF NOT EXISTS \"idx_{$table}_valid_from\" ON \"{$table}\" (valid_from)", ]; + if ($trackBranches) { + $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch)"; + } + return $stmts; } - private static function sqliteDdl(string $table, string $actorCol): array + private static function sqliteDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { $stmts = [ "CREATE TABLE IF NOT EXISTS \"{$table}\" ( @@ -275,12 +383,16 @@ private static function sqliteDdl(string $table, string $actorCol): array valid_from TEXT NOT NULL, snapshot TEXT NOT NULL, changed_cols TEXT NULL - {$actorCol} + {$actorCol}{$branchCol} )", "CREATE INDEX IF NOT EXISTS \"idx_{$table}_record_id\" ON \"{$table}\" (record_id)", "CREATE INDEX IF NOT EXISTS \"idx_{$table}_valid_from\" ON \"{$table}\" (valid_from)", ]; + if ($trackBranches) { + $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch)"; + } + return $stmts; } @@ -293,8 +405,17 @@ private static function actorColumnSql(string $driver): string }; } + private static function branchColumnSql(string $driver): string + { + return match ($driver) { + 'pgsql' => ",\n branch VARCHAR(50) NOT NULL DEFAULT 'main'", + 'sqlite' => ",\n branch TEXT NOT NULL DEFAULT 'main'", + default => "`branch` VARCHAR(50) NOT NULL DEFAULT 'main',", + }; + } + /** - * Reset the internal class attribute cache. Useful in tests. + * Reset the internal class attribute and column-existence caches. Useful in tests. * * @param string|null $class When null, clears all entries. * @return void @@ -305,6 +426,7 @@ public static function resetCache(?string $class = null): void unset(self::$cache[$class]); } else { self::$cache = []; + self::$columnCache = []; } } } diff --git a/tests/Console/MigrateTemporalCommandTest.php b/tests/Console/MigrateTemporalCommandTest.php index 9102e179..3d85ee74 100644 --- a/tests/Console/MigrateTemporalCommandTest.php +++ b/tests/Console/MigrateTemporalCommandTest.php @@ -12,13 +12,25 @@ function config($key = null, $default = null) namespace Tests\Unit\Console { require_once __DIR__ . '/../Support/Model/MockTemporalRecord.php'; +require_once __DIR__ . '/../Support/Model/MockMigratableRecord.php'; +require_once __DIR__ . '/Support/CommandTestEnvironment.php'; +use PDO; use PHPUnit\Framework\TestCase; use Phaseolies\Console\Commands\Migrations\MigrateTemporalCommand; +use Phaseolies\Database\Database; +use Phaseolies\Database\Temporal\TemporalManager; use Tests\Support\Model\MockTemporalRecord; +use Tests\Support\Model\MockMigratableRecord; +use Tests\Unit\Console\Support\InteractsWithFakeCommandIO; class MigrateTemporalCommandTest extends TestCase { + private ?PDO $pdo = null; + + /** @var string[] */ + private array $tempDirs = []; + protected function setUp(): void { parent::setUp(); @@ -26,6 +38,27 @@ protected function setUp(): void if (class_exists(\Tests\Unit\Console\Support\CommandTestEnvironment::class)) { \Tests\Unit\Console\Support\CommandTestEnvironment::$config['database.default'] = 'default_connection'; } + + TemporalManager::resetCache(); + + $this->pdo = new PDO('sqlite::memory:'); + $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $this->setStaticProperty(Database::class, 'connections', ['default' => $this->pdo]); + } + + protected function tearDown(): void + { + $this->setStaticProperty(Database::class, 'connections', []); + TemporalManager::resetCache(); + $this->pdo = null; + + foreach ($this->tempDirs as $dir) { + $this->removeDirectory($dir); + } + $this->tempDirs = []; + + parent::tearDown(); } public function testResolveConnectionForModelPrefersModelConnectionWhenNoOverride(): void @@ -62,5 +95,156 @@ public function testResolveConnectionForModelFallsBackToDefaultConfig(): void $this->assertSame('default_connection', $connection); } + + public function testHandleCreatesHistoryTableWithActorAndBranchColumnsForFreshTable(): void + { + $modelsPath = $this->writeDiscoverableModel(); + + $command = $this->makeCommand(['path' => $modelsPath]); + $exitCode = $command->handle(); + + $historyTable = (new MockMigratableRecord())->historyTable(); + + $this->assertSame(0, $exitCode); + $this->assertEmpty($command->capturedErrors); + $this->assertTrue($this->tableExists($historyTable)); + $this->assertTrue(TemporalManager::hasActorColumn($this->pdo, $historyTable, 'sqlite')); + $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $historyTable, 'sqlite')); + $this->assertStringContainsString('Created: 1', implode(' ', $command->capturedSuccesses)); + } + + public function testHandleAddsMissingActorAndBranchColumnsToAnExistingTable(): void + { + $modelsPath = $this->writeDiscoverableModel(); + $historyTable = (new MockMigratableRecord())->historyTable(); + + // Simulate a table that predates both the actor and branch columns. + foreach (TemporalManager::createHistoryTableSql($historyTable, 'sqlite', false, false) as $sql) { + $this->pdo->exec($sql); + } + TemporalManager::resetCache(); + + $this->assertFalse(TemporalManager::hasActorColumn($this->pdo, $historyTable, 'sqlite')); + $this->assertFalse(TemporalManager::hasBranchColumn($this->pdo, $historyTable, 'sqlite')); + + $command = $this->makeCommand(['path' => $modelsPath]); + $exitCode = $command->handle(); + + // The command's own column check is uncached, but our pre-check above + // cached a stale "false" in TemporalManager's process-lifetime cache. + TemporalManager::resetCache(); + + $this->assertSame(0, $exitCode); + $this->assertEmpty($command->capturedErrors); + $this->assertTrue(TemporalManager::hasActorColumn($this->pdo, $historyTable, 'sqlite')); + $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $historyTable, 'sqlite')); + + $status = implode(' ', array_map(fn ($line) => $line[0], $command->capturedLines)); + $this->assertStringContainsString('actor, branch columns added', $status); + } + + public function testHandleIsIdempotentOnceColumnsExist(): void + { + $modelsPath = $this->writeDiscoverableModel(); + $historyTable = (new MockMigratableRecord())->historyTable(); + + foreach (TemporalManager::createHistoryTableSql($historyTable, 'sqlite', true, true) as $sql) { + $this->pdo->exec($sql); + } + TemporalManager::resetCache(); + + $command = $this->makeCommand(['path' => $modelsPath]); + $exitCode = $command->handle(); + + $this->assertSame(0, $exitCode); + $status = implode(' ', array_map(fn ($line) => $line[0], $command->capturedLines)); + $this->assertStringContainsString('already exists', $status); + $this->assertStringNotContainsString('column', $status); + } + + public function testHandleDryRunPrintsBranchColumnSqlWithoutCreatingTheTable(): void + { + $modelsPath = $this->writeDiscoverableModel(); + $historyTable = (new MockMigratableRecord())->historyTable(); + + $command = $this->makeCommand(['path' => $modelsPath, 'show' => true]); + $exitCode = $command->handle(); + + $this->assertSame(0, $exitCode); + $this->assertFalse($this->tableExists($historyTable)); + + $output = implode(' ', array_map(fn ($line) => $line[0], $command->capturedLines)); + $this->assertStringContainsString("branch TEXT NOT NULL DEFAULT 'main'", $output); + $this->assertStringContainsString('actor TEXT NULL', $output); + } + + /** + * Write a copy of MockMigratableRecord into a temp directory so + * MigrateTemporalCommand's filesystem-based discovery can find it. The + * class itself is already loaded via the require_once at the top of + * this file; discovery only reads this file's text to resolve the FQCN. + * + * @return string + */ + private function writeDiscoverableModel(): string + { + $dir = sys_get_temp_dir() . '/doppar-migrate-temporal-' . bin2hex(random_bytes(5)); + mkdir($dir, 0755, true); + $this->tempDirs[] = $dir; + + copy( + __DIR__ . '/../Support/Model/MockMigratableRecord.php', + $dir . '/MockMigratableRecord.php' + ); + + return $dir; + } + + private function removeDirectory(string $dir): void + { + if (!is_dir($dir)) { + return; + } + + foreach (scandir($dir) ?: [] as $item) { + if ($item === '.' || $item === '..') { + continue; + } + + $path = $dir . '/' . $item; + is_dir($path) ? $this->removeDirectory($path) : unlink($path); + } + + rmdir($dir); + } + + /** + * @param array $options + */ + private function makeCommand(array $options): MigrateTemporalCommand + { + $command = new class extends MigrateTemporalCommand + { + use InteractsWithFakeCommandIO; + }; + $command->fakeOptions = $options; + + return $command; + } + + private function tableExists(string $table): bool + { + $stmt = $this->pdo->prepare("SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?"); + $stmt->execute([$table]); + + return (bool) $stmt->fetchColumn(); + } + + private function setStaticProperty(string $className, string $propertyName, mixed $value): void + { + $reflection = new \ReflectionClass($className); + $property = $reflection->getProperty($propertyName); + $property->setValue(null, $value); + } } } diff --git a/tests/Model/TemporalBranchTest.php b/tests/Model/TemporalBranchTest.php new file mode 100644 index 00000000..af1509b5 --- /dev/null +++ b/tests/Model/TemporalBranchTest.php @@ -0,0 +1,324 @@ +container = new MockContainer(); + Container::setInstance($this->container); + + $this->pdo = new PDO('sqlite::memory:'); + $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $this->createTables(); + $this->setupDatabaseConnections(); + + HookHandler::$hooks = []; + TemporalManager::resetCache(); + $this->registerTemporalHooks(MockBranchableRecord::class); + $this->registerTemporalHooks(MockTemporalRecord::class); + + Carbon::setTestNow(); + } + + protected function tearDown(): void + { + Carbon::setTestNow(); + HookHandler::$hooks = []; + TemporalManager::resetCache(); + $this->tearDownDatabaseConnections(); + Container::forgetInstance(); + $this->container = null; + $this->pdo = null; + } + + public function testBranchForksFromMainWithoutDisturbingMainHistory(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'cost basis is high', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '120'; + $record->save(); + + $this->travelTo('2026-02-01 12:00:00.000000'); + $this->assertTrue($record->branch('proposed-pricing', '2026-02-01 10:30:00')); + + $mainHistory = $record->history(); + $this->assertCount(2, $mainHistory); + $this->assertSame(['created', 'updated'], $mainHistory->map->__action->all()); + + $branchHistory = $record->history('proposed-pricing'); + $this->assertCount(1, $branchHistory); + $this->assertSame('branched', $branchHistory[0]->__action); + $this->assertSame(100, $branchHistory[0]->price); + + $onMain = MockBranchableRecord::at('2026-02-01 12:30:00')->find($record->id); + $onBranch = MockBranchableRecord::at('2026-02-01 12:30:00', 'proposed-pricing')->find($record->id); + + $this->assertSame(120, $onMain->price); + $this->assertSame(100, $onBranch->price); + } + + public function testBranchReturnsFalseWhenSourceSnapshotIsMissing(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->assertFalse($record->branch('too-early', '2026-01-01 00:00:00')); + } + + public function testBranchRejectsEmptyAndReservedNames(): void + { + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->expectException(\InvalidArgumentException::class); + $record->branch('main', '2026-02-01 10:00:00'); + } + + public function testBranchThrowsForModelsNotConfiguredForBranching(): void + { + $created = MockTemporalRecord::create([ + 'title' => 'Contract A', + 'status' => 'draft', + 'body' => 'Original body', + ]); + $record = MockTemporalRecord::find($created->id); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + $record->branch('attempt', '2026-01-01 00:00:00'); + } + + public function testAtWithNonMainBranchThrowsForModelsNotConfiguredForBranching(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + MockTemporalRecord::at('2026-01-01 00:00:00', 'some-branch'); + } + + public function testDiffRespectsCastsAndHidesUnexposableAttributes(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'cost basis is high', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '150'; + $record->internal_note = 'revised note'; + $record->save(); + + $diff = $record->diff('2026-02-01 10:30:00', '2026-02-01 11:30:00'); + + $this->assertIsInt($diff['changes']['price']['from']); + $this->assertIsInt($diff['changes']['price']['to']); + $this->assertSame(100, $diff['changes']['price']['from']); + $this->assertSame(150, $diff['changes']['price']['to']); + $this->assertArrayNotHasKey('internal_note', $diff['changes']); + $this->assertArrayNotHasKey('internal_note', $diff['added']); + $this->assertArrayNotHasKey('internal_note', $diff['removed']); + } + + public function testRestoreFromBranchCreatesANewMainVersionAndNeverRewritesHistory(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '120'; + $record->save(); + + $this->travelTo('2026-02-01 12:00:00.000000'); + $record->branch('rollback-candidate', '2026-02-01 10:30:00'); + + $this->travelTo('2026-02-01 13:00:00.000000'); + $this->assertTrue($record->restoreTo('2026-02-01 12:30:00', 'rollback-candidate')); + + $fresh = MockBranchableRecord::find($record->id); + $this->assertSame(100, $fresh->price); + + // Restoring persisted through the normal save()/snapshot path, so the + // main timeline only ever grew — nothing was rewritten. + $this->assertCount(3, $fresh->history()); + $this->assertSame(['created', 'updated', 'updated'], $fresh->history()->map->__action->all()); + } + + public function testRestoreToNonMainBranchThrowsForModelsNotConfiguredForBranching(): void + { + $created = MockTemporalRecord::create([ + 'title' => 'Contract A', + 'status' => 'draft', + 'body' => 'Original body', + ]); + $record = MockTemporalRecord::find($created->id); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + $record->restoreTo('2026-01-01 00:00:00', 'some-branch'); + } + + public function testMigrateTemporalAddsBranchColumnToExistingTable(): void + { + // A table name unique to this test, so the process-lifetime column + // cache can't carry a stale answer in from another test method. + $table = 'migration_check_history'; + $driver = 'sqlite'; + + $statements = TemporalManager::createHistoryTableSql($table, $driver, false, false); + foreach ($statements as $statement) { + $this->pdo->exec($statement); + } + + $this->assertFalse( + TemporalManager::hasBranchColumn($this->pdo, $table, $driver) + ); + + $this->pdo->exec("ALTER TABLE {$table} ADD COLUMN branch TEXT NOT NULL DEFAULT 'main'"); + TemporalManager::resetCache(); + + $this->assertTrue( + TemporalManager::hasBranchColumn($this->pdo, $table, $driver) + ); + + $this->pdo->exec("DROP TABLE {$table}"); + } + + public function testTemporalManagerDdlIncludesBranchColumnAndIndexWhenEnabled(): void + { + $mysql = TemporalManager::createHistoryTableSql('widgets_history', 'mysql', false, true); + $pgsql = TemporalManager::createHistoryTableSql('widgets_history', 'pgsql', false, true); + $sqlite = TemporalManager::createHistoryTableSql('widgets_history', 'sqlite', false, true); + + $this->assertStringContainsString("`branch` VARCHAR(50) NOT NULL DEFAULT 'main'", $mysql[0]); + $this->assertStringContainsString('idx_branch', $mysql[0]); + + $this->assertCount(4, $pgsql); + $this->assertStringContainsString("branch VARCHAR(50) NOT NULL DEFAULT 'main'", $pgsql[0]); + $this->assertStringContainsString('idx_widgets_history_branch', $pgsql[3]); + + $this->assertCount(4, $sqlite); + $this->assertStringContainsString("branch TEXT NOT NULL DEFAULT 'main'", $sqlite[0]); + $this->assertStringContainsString('idx_widgets_history_branch', $sqlite[3]); + } + + private function createTables(): void + { + $this->pdo->exec( + 'CREATE TABLE branchable_records ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + price TEXT NOT NULL, + internal_note TEXT NULL, + created_at TEXT NULL, + updated_at TEXT NULL + )' + ); + + $historyTable = (new MockBranchableRecord())->historyTable(); + $statements = TemporalManager::createHistoryTableSql($historyTable, 'sqlite', false, true); + foreach ($statements as $statement) { + $this->pdo->exec($statement); + } + + $this->pdo->exec( + 'CREATE TABLE temporal_records ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + status TEXT NOT NULL, + body TEXT NOT NULL, + created_at TEXT NULL, + updated_at TEXT NULL + )' + ); + + $temporalHistoryTable = (new MockTemporalRecord())->historyTable(); + $temporalStatements = TemporalManager::createHistoryTableSql($temporalHistoryTable, 'sqlite', true, false); + foreach ($temporalStatements as $statement) { + $this->pdo->exec($statement); + } + } + + private function setupDatabaseConnections(): void + { + $this->setStaticProperty(Database::class, 'connections', []); + $this->setStaticProperty(Database::class, 'transactions', []); + + $this->setStaticProperty(Database::class, 'connections', [ + 'default' => $this->pdo, + 'sqlite' => $this->pdo, + ]); + } + + private function tearDownDatabaseConnections(): void + { + $this->setStaticProperty(Database::class, 'connections', []); + $this->setStaticProperty(Database::class, 'transactions', []); + } + + private function setStaticProperty(string $className, string $propertyName, mixed $value): void + { + $reflection = new \ReflectionClass($className); + $property = $reflection->getProperty($propertyName); + $property->setValue(null, $value); + } + + private function registerTemporalHooks(string $class): void + { + $model = new $class(); + $register = \Closure::bind(function (): void { + $this->registerTemporalHooks(); + }, $model, $class); + + $register(); + } + + private function travelTo(string $datetime): void + { + Carbon::setTestNow(Carbon::parse($datetime, 'UTC')); + } +} diff --git a/tests/Support/Model/MockBranchableRecord.php b/tests/Support/Model/MockBranchableRecord.php new file mode 100644 index 00000000..13f968fc --- /dev/null +++ b/tests/Support/Model/MockBranchableRecord.php @@ -0,0 +1,21 @@ + Date: Fri, 16 Oct 2026 22:10:04 +0600 Subject: [PATCH 2/4] Address review: branch writes, restore guards, cross-driver tests - diff() compares cast values strictly instead of via (string), so array/json, bool and null changes are no longer dropped - Branchable models throw a "run migrate:temporal" RuntimeException when the history table lacks the branch column, instead of silently reading main-timeline rows - Add saveToBranch(), diffAgainstBranch() and restoreVersion(); both restore paths refuse deleted versions - migrate:temporal creates the (record_id, branch) index when it adds the column to an existing table - Validate branch names (1-50 chars, [A-Za-z0-9_-]) on every entry point; forking an existing branch name now throws - Use history_id as a tiebreak for same-microsecond versions - Port TemporalBranchTest to the Sqlite/Mysql/Pgsql driver layout - Drop phpstan baseline suppressions, don't cache missing columns, scope the PostgreSQL column check to the current schema --- phpstan-baseline.neon | 24 - .../Migrations/MigrateTemporalCommand.php | 43 +- src/Phaseolies/Database/Entity/Model.php | 4 + .../Temporal/InteractsWithTemporal.php | 157 +++++- .../Database/Temporal/TemporalBuilder.php | 13 +- .../Database/Temporal/TemporalManager.php | 59 ++- tests/Console/MigrateTemporalCommandTest.php | 10 +- .../TemporalBranch/TemporalBranchBehavior.php | 457 ++++++++++++++++++ .../TemporalBranchMysqlTest.php | 17 + .../TemporalBranchPgsqlTest.php | 17 + .../TemporalBranchSqliteTest.php | 13 + tests/Model/TemporalBranchTest.php | 324 ------------- tests/Support/Model/MockBranchableRecord.php | 10 +- 13 files changed, 766 insertions(+), 382 deletions(-) create mode 100644 tests/Model/TemporalBranch/TemporalBranchBehavior.php create mode 100644 tests/Model/TemporalBranch/TemporalBranchMysqlTest.php create mode 100644 tests/Model/TemporalBranch/TemporalBranchPgsqlTest.php create mode 100644 tests/Model/TemporalBranch/TemporalBranchSqliteTest.php delete mode 100644 tests/Model/TemporalBranchTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ab89bc04..78037316 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -258,12 +258,6 @@ parameters: count: 1 path: src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php - - - message: '#^Using nullsafe property access "\?\-\>branches" on left side of \?\? is unnecessary\. Use \-\> instead\.$#' - identifier: nullsafe.neverNull - count: 1 - path: src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php - - message: '#^Call to an undefined static method Phaseolies\\Support\\Facades\\Route\:\:cacheRoutes\(\)\.$#' identifier: staticMethod.notFound @@ -750,12 +744,6 @@ parameters: count: 1 path: src/Phaseolies/Database/Entity/Model.php - - - message: '#^Access to an undefined property Phaseolies\\Database\\Entity\\Model\:\:\$__branch\.$#' - identifier: property.notFound - count: 1 - path: src/Phaseolies/Database/Entity/Model.php - - message: '#^Access to an undefined property Phaseolies\\Database\\Entity\\Model\:\:\$__changed_cols\.$#' identifier: property.notFound @@ -810,12 +798,6 @@ parameters: count: 1 path: src/Phaseolies/Database/Entity/Model.php - - - message: '#^Unsafe usage of new static\(\)\.$#' - identifier: new.static - count: 11 - path: src/Phaseolies/Database/Entity/Model.php - - message: '#^Call to an undefined static method Phaseolies\\Support\\Facades\\URL\:\:current\(\)\.$#' identifier: staticMethod.notFound @@ -936,12 +918,6 @@ parameters: count: 1 path: src/Phaseolies/Database/Temporal/TemporalManager.php - - - message: '#^Using nullsafe property access "\?\-\>branches" on left side of \?\? is unnecessary\. Use \-\> instead\.$#' - identifier: nullsafe.neverNull - count: 1 - path: src/Phaseolies/Database/Temporal/TemporalManager.php - - message: '#^Class App\\Http\\Exceptions\\BeforeExceptionHandler not found\.$#' identifier: class.notFound diff --git a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php index 9e79aa44..276c7825 100644 --- a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php +++ b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php @@ -104,11 +104,17 @@ public function handle(): int $addedColumns[] = 'actor'; } - if ($attr?->branches && !$this->columnExists($pdo, $driver, $historyTable, 'branch')) { + if ($attr !== null && $attr->branches && !$this->columnExists($pdo, $driver, $historyTable, 'branch')) { $pdo->exec($this->addBranchColumnSql($driver, $historyTable)); $addedColumns[] = 'branch'; } + $branchIndex = $driver === 'mysql' ? 'idx_branch' : 'idx_' . $historyTable . '_branch'; + if ($attr !== null && $attr->branches && !$this->indexExists($pdo, $driver, $historyTable, $branchIndex)) { + $pdo->exec($this->createBranchIndexSql($driver, $historyTable)); + $addedColumns[] = 'branch index'; + } + if (!empty($addedColumns)) { $row('✓', 'green', implode(', ', $addedColumns) . ' column' . (count($addedColumns) > 1 ? 's' : '') . ' added'); $created++; @@ -123,7 +129,7 @@ public function handle(): int historyTable: $historyTable, driver: $driver, trackActor: $attr?->trackActor ?? false, - trackBranches: $attr?->branches ?? false, + trackBranches: $attr !== null && $attr->branches, ); if ($dryRun) { @@ -280,7 +286,7 @@ private function columnExists(PDO $pdo, string $driver, string $table, string $c if ($driver === 'pgsql') { $stmt = $pdo->prepare( "SELECT COUNT(*) FROM information_schema.columns - WHERE table_name = ? AND column_name = ?" + WHERE table_schema = current_schema() AND table_name = ? AND column_name = ?" ); $stmt->execute([$table, $column]); return (bool) $stmt->fetchColumn(); @@ -336,4 +342,35 @@ private function addBranchColumnSql(string $driver, string $table): string default => "ALTER TABLE `{$table}` ADD COLUMN `branch` VARCHAR(50) NOT NULL DEFAULT 'main'", }; } + + private function indexExists(PDO $pdo, string $driver, string $table, string $index): bool + { + try { + if ($driver === 'sqlite') { + foreach ($pdo->query("PRAGMA index_list(\"{$table}\")")->fetchAll(PDO::FETCH_ASSOC) as $row) { + if ($row['name'] === $index) return true; + } + return false; + } + if ($driver === 'pgsql') { + $stmt = $pdo->prepare('SELECT 1 FROM pg_indexes WHERE schemaname = current_schema() AND tablename = ? AND indexname = ?'); + $stmt->execute([$table, $index]); + return $stmt->fetchColumn() !== false; + } + $stmt = $pdo->prepare("SHOW INDEX FROM `{$table}` WHERE Key_name = ?"); + $stmt->execute([$index]); + return $stmt->fetch() !== false; + } catch (\Throwable) { + return false; + } + } + + private function createBranchIndexSql(string $driver, string $table): string + { + $name = $driver === 'mysql' ? 'idx_branch' : 'idx_' . $table . '_branch'; + return match ($driver) { + 'pgsql', 'sqlite' => "CREATE INDEX \"{$name}\" ON \"{$table}\" (record_id, branch)", + default => "CREATE INDEX `{$name}` ON `{$table}` (record_id, branch)", + }; + } } diff --git a/src/Phaseolies/Database/Entity/Model.php b/src/Phaseolies/Database/Entity/Model.php index 6b84390c..b6d3d4ad 100644 --- a/src/Phaseolies/Database/Entity/Model.php +++ b/src/Phaseolies/Database/Entity/Model.php @@ -14,6 +14,10 @@ use Phaseolies\Database\Contracts\Support\Jsonable; use Phaseolies\Database\Entity\Attributes\Hook; +/** + * @phpstan-consistent-constructor + * @property string $__branch Temporal history metadata. + */ abstract class Model implements Jsonable, \ArrayAccess, \JsonSerializable, \Stringable { use InteractsWithModelQueryProcessing, InteractsWithTemporal, InteractsWithCasting, InteractsWithWatches, InteractsWithComputedProperties, InteractsWithSoftDeletes; diff --git a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php index 29eaa7bd..361d8b19 100644 --- a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php +++ b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php @@ -66,8 +66,8 @@ public static function at(string $datetime, string $branch = 'main'): TemporalBu /** * Fork a new branch from this record's main timeline as it existed at - * the given point in time. The branch is independently queryable via - * static::at($datetime, $name) and never alters the main history. + * the given point in time. Its timeline starts when the fork is created, + * so earlier queries on that branch return no snapshot. * * @param string $name * @param string $fromDatetime @@ -85,7 +85,8 @@ public function branch(string $name, string $fromDatetime): bool ); } - if ($name === '' || $name === 'main') { + TemporalManager::validateBranchName($name); + if ($name === 'main') { throw new \InvalidArgumentException('Branch name must be non-empty and cannot be "main".'); } @@ -100,6 +101,37 @@ public function branch(string $name, string $fromDatetime): bool return true; } + /** Append this in-memory proposed state as a new version on an existing branch. */ + public function saveToBranch(string $branch): bool + { + $this->assertTemporalAndLoaded(); + TemporalManager::assertBranchSupported(static::class, $branch); + if ($branch === 'main') { + throw new \InvalidArgumentException('saveToBranch() requires a named branch.'); + } + TemporalManager::snapshotBranchVersion($this, $branch); + $this->setOriginalAttributes($this->getAttributes()); + return true; + } + + /** Compare the current main state with the latest state on a named branch. */ + public function diffAgainstBranch(string $branch): ?array + { + $this->assertTemporalAndLoaded(); + TemporalManager::assertBranchSupported(static::class, $branch); + if ($branch === 'main') { + throw new \InvalidArgumentException('A named branch is required.'); + } + $now = \Carbon\Carbon::now()->format('Y-m-d H:i:s.u'); + $main = $this->snapshotAt($now, 'main'); + $proposed = $this->snapshotAt($now, $branch); + if ($main === null || $proposed === null) { + return null; + } + $probe = new static(); + return $this->formatDiff($main, $proposed, 'main → ' . $branch, $now, $now, $probe); + } + /** * Return the complete audit history for this model instance on the given branch. * @@ -116,8 +148,13 @@ public function history(?string $branch = 'main'): Collection $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); $hasActor = TemporalManager::hasActorColumn($pdo, $historyTable, $driver); - $hasBranch = TemporalManager::isBranchable(static::class) - && TemporalManager::hasBranchColumn($pdo, $historyTable, $driver); + $hasBranch = TemporalManager::isBranchable(static::class); + if ($branch !== null) { + TemporalManager::assertBranchSupported(static::class, $branch); + } + if ($hasBranch) { + TemporalManager::assertBranchColumn($pdo, $historyTable, $driver); + } $columns = 'history_id, action, valid_from, snapshot, changed_cols' . ($hasActor ? ', actor' : '') @@ -127,7 +164,7 @@ public function history(?string $branch = 'main'): Collection FROM {$historyTable} WHERE record_id = ?" . ($hasBranch && $branch !== null ? ' AND branch = ?' : '') - . ' ORDER BY valid_from ASC'; + . ' ORDER BY valid_from ASC, history_id ASC'; $params = [$this->getKey()]; if ($hasBranch && $branch !== null) { @@ -185,8 +222,13 @@ public function diff(string $from, string $to, string $branch = 'main'): ?array return null; } - $visibleFrom = $this->visibleSnapshot($snapshotFrom); - $visibleTo = $this->visibleSnapshot($snapshotTo); + return $this->formatDiff($snapshotFrom, $snapshotTo, $branch, $from, $to, new static()); + } + + private function formatDiff(array $snapshotFrom, array $snapshotTo, string $branch, string $from, string $to, Model $probe): array + { + $visibleFrom = $this->visibleSnapshot($snapshotFrom, $probe); + $visibleTo = $this->visibleSnapshot($snapshotTo, $probe); $changes = []; $added = []; @@ -195,7 +237,7 @@ public function diff(string $from, string $to, string $branch = 'main'): ?array foreach ($visibleTo as $col => $newVal) { if (!array_key_exists($col, $visibleFrom)) { $added[$col] = $newVal; - } elseif ((string) $visibleFrom[$col] !== (string) $newVal) { + } elseif (!$this->sameCastValue($visibleFrom[$col], $newVal)) { $changes[$col] = ['from' => $visibleFrom[$col], 'to' => $newVal]; } } @@ -216,6 +258,14 @@ public function diff(string $from, string $to, string $branch = 'main'): ?array ]; } + private function sameCastValue(mixed $left, mixed $right): bool + { + if (is_object($left) && is_object($right)) { + return $left == $right; + } + return $left === $right; + } + /** * Apply this model's casts and unexposable (hidden) attribute filtering * to a raw stored snapshot, for display/diffing purposes only. @@ -223,9 +273,8 @@ public function diff(string $from, string $to, string $branch = 'main'): ?array * @param array $snapshot * @return array */ - private function visibleSnapshot(array $snapshot): array + private function visibleSnapshot(array $snapshot, Model $probe): array { - $probe = new static(); $visible = []; foreach ($snapshot as $key => $value) { @@ -284,6 +333,8 @@ public function rewindTo(string $datetime, string $branch = 'main'): ?static */ public function restoreTo(string $datetime, string $branch = 'main'): bool { + $this->assertTemporalAndLoaded(); + $this->assertNotDeletedAt($datetime, $branch); $rewound = $this->rewindTo($datetime, $branch); if ($rewound === null) { @@ -303,6 +354,62 @@ public function restoreTo(string $datetime, string $branch = 'main'): bool return true; } + private function assertNotDeletedAt(string $datetime, string $branch): void + { + TemporalManager::assertBranchSupported(static::class, $branch); + $datetime = $this->normalizeTemporalDateTime($datetime); + $table = $this->historyTable(); + $pdo = $this->getConnection(); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + $hasBranch = TemporalManager::isBranchable(static::class); + if ($hasBranch) { + TemporalManager::assertBranchColumn($pdo, $table, $driver); + } + $sql = "SELECT action FROM {$table} WHERE record_id = ? AND valid_from <= ?" . ($hasBranch ? ' AND branch = ?' : '') . ' ORDER BY valid_from DESC, history_id DESC LIMIT 1'; + $params = [$this->getKey(), $datetime]; + if ($hasBranch) { + $params[] = $branch; + } + $stmt = $pdo->prepare($sql); + $stmt->execute($params); + if ($stmt->fetchColumn() === 'deleted') { + throw new \RuntimeException('Cannot restore a deleted temporal version.'); + } + } + + /** Restore a specific history entry by id, refusing deleted snapshots. */ + public function restoreVersion(int $historyId): bool + { + $this->assertTemporalAndLoaded(); + $historyTable = $this->historyTable(); + $pdo = $this->getConnection(); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + $hasBranch = TemporalManager::isBranchable(static::class); + if ($hasBranch) { + TemporalManager::assertBranchColumn($pdo, $historyTable, $driver); + } + + $stmt = $pdo->prepare( + "SELECT snapshot, action" . ($hasBranch ? ', branch' : '') + . " FROM {$historyTable} WHERE history_id = ? AND record_id = ?" + ); + $stmt->execute([$historyId, $this->getKey()]); + $row = $stmt->fetch(PDO::FETCH_ASSOC); + if (!$row) { + return false; + } + if ($row['action'] === 'deleted') { + throw new \RuntimeException('A deleted temporal version cannot be restored.'); + } + $snapshot = json_decode($row['snapshot'], true) ?? []; + $restored = new static($snapshot); + $restored->setOriginalAttributes($this->getAttributes()); + if (!$restored->save()) { + return false; + } + return true; + } + /** * Whether this model class is marked #[Temporal]. * @@ -337,21 +444,20 @@ private function snapshotAt(string $datetime, string $branch = 'main'): ?array $pdo = $this->getConnection(); $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); - $hasBranch = TemporalManager::isBranchable(static::class) - && TemporalManager::hasBranchColumn($pdo, $historyTable, $driver); - - // Normalize to end-of-day when only a date is given - if (preg_match('/^\d{4}-\d{2}-\d{2}$/', trim($datetime))) { - $datetime .= ' 23:59:59.999999'; + $hasBranch = TemporalManager::isBranchable(static::class); + if ($hasBranch) { + TemporalManager::assertBranchColumn($pdo, $historyTable, $driver); } + $datetime = $this->normalizeTemporalDateTime($datetime); + $sql = "SELECT snapshot FROM {$historyTable} WHERE record_id = ? AND valid_from <= ? AND action != 'deleted'" . ($hasBranch ? ' AND branch = ?' : '') - . ' ORDER BY valid_from DESC + . ' ORDER BY valid_from DESC, history_id DESC LIMIT 1'; $params = [$this->getKey(), $datetime]; @@ -367,6 +473,21 @@ private function snapshotAt(string $datetime, string $branch = 'main'): ?array return $row ? (json_decode($row['snapshot'], true) ?? null) : null; } + private function normalizeTemporalDateTime(string $datetime): string + { + $datetime = trim($datetime); + if (preg_match('/^\d{4}-\d{2}-\d{2}$/', $datetime)) { + return $datetime . ' 23:59:59.999999'; + } + if (preg_match('/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}$/', $datetime)) { + return $datetime . ':59.999999'; + } + if (preg_match('/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/', $datetime)) { + return $datetime . '.999999'; + } + return $datetime; + } + /** * Assert that the model is temporal and has a primary key loaded. * diff --git a/src/Phaseolies/Database/Temporal/TemporalBuilder.php b/src/Phaseolies/Database/Temporal/TemporalBuilder.php index 1a389357..31799c00 100644 --- a/src/Phaseolies/Database/Temporal/TemporalBuilder.php +++ b/src/Phaseolies/Database/Temporal/TemporalBuilder.php @@ -105,7 +105,7 @@ public function find($id): ?Model AND valid_from <= ? AND action != 'deleted'" . ($hasBranch ? ' AND branch = ?' : '') . " - ORDER BY valid_from DESC + ORDER BY valid_from DESC, history_id DESC LIMIT 1 "; @@ -139,6 +139,7 @@ private function fetchTemporalRows(): array $hasBranch = $this->hasBranchColumn($historyTable); $innerBranch = $hasBranch ? ' AND h2.branch = ?' : ''; + $tieBranch = $hasBranch ? ' AND h3.branch = ?' : ''; $outerBranch = $hasBranch ? ' AND h1.branch = ?' : ''; $sql = " @@ -150,6 +151,12 @@ private function fetchTemporalRows(): array WHERE h2.record_id = h1.record_id AND h2.valid_from <= ?{$innerBranch} ) + AND h1.history_id = ( + SELECT MAX(h3.history_id) + FROM {$historyTable} h3 + WHERE h3.record_id = h1.record_id + AND h3.valid_from = h1.valid_from{$tieBranch} + ) AND h1.action != 'deleted'{$outerBranch} "; @@ -157,6 +164,7 @@ private function fetchTemporalRows(): array if ($hasBranch) { $params[] = $this->branch; $params[] = $this->branch; + $params[] = $this->branch; } $stmt = $this->pdo->prepare($sql); @@ -179,7 +187,8 @@ private function hasBranchColumn(string $historyTable): bool $driver = $this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME); - return TemporalManager::hasBranchColumn($this->pdo, $historyTable, $driver); + TemporalManager::assertBranchColumn($this->pdo, $historyTable, $driver); + return true; } /** diff --git a/src/Phaseolies/Database/Temporal/TemporalManager.php b/src/Phaseolies/Database/Temporal/TemporalManager.php index ab749596..56f79d11 100644 --- a/src/Phaseolies/Database/Temporal/TemporalManager.php +++ b/src/Phaseolies/Database/Temporal/TemporalManager.php @@ -73,7 +73,8 @@ public static function historyTable(string $table, string $class): string */ public static function isBranchable(string $class): bool { - return self::getAttribute($class)?->branches ?? false; + $attribute = self::getAttribute($class); + return $attribute !== null && $attribute->branches; } /** @@ -87,6 +88,8 @@ public static function isBranchable(string $class): bool */ public static function assertBranchSupported(string $class, string $branch): void { + self::validateBranchName($branch); + if ($branch !== 'main' && !self::isBranchable($class)) { throw new \RuntimeException( $class . ' does not support branches. Add #[Temporal(branches: true)] to enable them.' @@ -94,6 +97,22 @@ public static function assertBranchSupported(string $class, string $branch): voi } } + /** Validate branch names consistently with the persisted column. */ + public static function validateBranchName(string $branch): void + { + if (strlen($branch) > 50 || !preg_match('/^[A-Za-z0-9_-]+$/', $branch)) { + throw new \InvalidArgumentException('Branch names must be 1-50 characters using letters, numbers, _ or -.'); + } + } + + /** Ensure branch-enabled models have completed migrate:temporal. */ + public static function assertBranchColumn(PDO $pdo, string $historyTable, string $driver): void + { + if (!self::hasBranchColumn($pdo, $historyTable, $driver)) { + throw new \RuntimeException("Temporal branch column is missing from {$historyTable}; run migrate:temporal."); + } + } + /** * Snapshot the model's current state into the history table. * @@ -141,7 +160,8 @@ public static function snapshot(Model $model, string $action, string $branch = ' $data['actor'] = self::resolveActor(); } - if ($attr?->branches && self::hasBranchColumn($pdo, $historyTable, $driver)) { + if ($attr !== null && $attr->branches) { + self::assertBranchColumn($pdo, $historyTable, $driver); $data['branch'] = $branch; } @@ -164,6 +184,13 @@ public static function snapshotBranch(Model $model, array $snapshot, string $bra $pdo = $model->getConnection(); $historyTable = self::historyTable($model->getTable(), get_class($model)); $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + self::assertBranchColumn($pdo, $historyTable, $driver); + + $stmt = $pdo->prepare("SELECT 1 FROM {$historyTable} WHERE record_id = ? AND branch = ? LIMIT 1"); + $stmt->execute([$model->getKey(), $branch]); + if ($stmt->fetchColumn()) { + throw new \RuntimeException("Branch '{$branch}' already exists for this record."); + } $data = [ 'record_id' => $model->getKey(), @@ -182,6 +209,21 @@ public static function snapshotBranch(Model $model, array $snapshot, string $bra self::insertHistory($pdo, $historyTable, $data); } + /** Append an edited proposed state to an existing branch without saving the live record. */ + public static function snapshotBranchVersion(Model $model, string $branch): void + { + $pdo = $model->getConnection(); + $table = self::historyTable($model->getTable(), get_class($model)); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + self::assertBranchColumn($pdo, $table, $driver); + $exists = $pdo->prepare("SELECT 1 FROM {$table} WHERE record_id = ? AND branch = ? LIMIT 1"); + $exists->execute([$model->getKey(), $branch]); + if (!$exists->fetchColumn()) { + throw new \RuntimeException("Branch '{$branch}' does not exist for this record."); + } + self::snapshot($model, 'updated', $branch); + } + /** * Check whether the `actor` column exists in the given history table. * @@ -219,15 +261,15 @@ public static function hasBranchColumn(PDO $pdo, string $historyTable, string $d */ private static function hasColumn(PDO $pdo, string $historyTable, string $column, string $driver): bool { - $cacheKey = $historyTable . '.' . $column; + $cacheKey = spl_object_id($pdo) . '.' . $historyTable . '.' . $column; if (!array_key_exists($cacheKey, self::$columnCache)) { try { - self::$columnCache[$cacheKey] = match ($driver) { + $exists = match ($driver) { 'pgsql' => (function () use ($pdo, $historyTable, $column): bool { $stmt = $pdo->prepare( "SELECT COUNT(*) FROM information_schema.columns - WHERE table_name = ? AND column_name = ?" + WHERE table_schema = current_schema() AND table_name = ? AND column_name = ?" ); $stmt->execute([$historyTable, $column]); return (bool) $stmt->fetchColumn(); @@ -247,12 +289,15 @@ private static function hasColumn(PDO $pdo, string $historyTable, string $column return $stmt->fetch() !== false; })(), }; + if ($exists) { + self::$columnCache[$cacheKey] = true; + } } catch (\Throwable) { - self::$columnCache[$cacheKey] = false; + unset(self::$columnCache[$cacheKey]); } } - return self::$columnCache[$cacheKey]; + return self::$columnCache[$cacheKey] ?? false; } /** diff --git a/tests/Console/MigrateTemporalCommandTest.php b/tests/Console/MigrateTemporalCommandTest.php index 3d85ee74..2d7d5cc9 100644 --- a/tests/Console/MigrateTemporalCommandTest.php +++ b/tests/Console/MigrateTemporalCommandTest.php @@ -110,6 +110,8 @@ public function testHandleCreatesHistoryTableWithActorAndBranchColumnsForFreshTa $this->assertTrue($this->tableExists($historyTable)); $this->assertTrue(TemporalManager::hasActorColumn($this->pdo, $historyTable, 'sqlite')); $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $historyTable, 'sqlite')); + $indexes = $this->pdo->query("PRAGMA index_list(\"{$historyTable}\")")->fetchAll(PDO::FETCH_ASSOC); + $this->assertContains('idx_' . $historyTable . '_branch', array_column($indexes, 'name')); $this->assertStringContainsString('Created: 1', implode(' ', $command->capturedSuccesses)); } @@ -130,17 +132,19 @@ public function testHandleAddsMissingActorAndBranchColumnsToAnExistingTable(): v $command = $this->makeCommand(['path' => $modelsPath]); $exitCode = $command->handle(); - // The command's own column check is uncached, but our pre-check above - // cached a stale "false" in TemporalManager's process-lifetime cache. + // The migration path changes the schema after the pre-check; existence + // checks must observe the new columns immediately. TemporalManager::resetCache(); $this->assertSame(0, $exitCode); $this->assertEmpty($command->capturedErrors); $this->assertTrue(TemporalManager::hasActorColumn($this->pdo, $historyTable, 'sqlite')); $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $historyTable, 'sqlite')); + $indexes = $this->pdo->query("PRAGMA index_list(\"{$historyTable}\")")->fetchAll(PDO::FETCH_ASSOC); + $this->assertContains('idx_' . $historyTable . '_branch', array_column($indexes, 'name')); $status = implode(' ', array_map(fn ($line) => $line[0], $command->capturedLines)); - $this->assertStringContainsString('actor, branch columns added', $status); + $this->assertStringContainsString('actor, branch, branch index columns added', $status); } public function testHandleIsIdempotentOnceColumnsExist(): void diff --git a/tests/Model/TemporalBranch/TemporalBranchBehavior.php b/tests/Model/TemporalBranch/TemporalBranchBehavior.php new file mode 100644 index 00000000..3bffd657 --- /dev/null +++ b/tests/Model/TemporalBranch/TemporalBranchBehavior.php @@ -0,0 +1,457 @@ + [ + ['name' => 'id', 'type' => 'id'], + ['name' => 'title', 'type' => 'string'], + ['name' => 'price', 'type' => 'string'], + ['name' => 'internal_note', 'type' => 'text', 'nullable' => true], + ['name' => 'metadata', 'type' => 'text', 'nullable' => true], + ['name' => 'enabled', 'type' => 'boolean', 'nullable' => true], + ['name' => 'created_at', 'type' => 'datetime', 'nullable' => true], + ['name' => 'updated_at', 'type' => 'datetime', 'nullable' => true], + ], + 'temporal_records' => [ + ['name' => 'id', 'type' => 'id'], + ['name' => 'title', 'type' => 'string'], + ['name' => 'status', 'type' => 'string'], + ['name' => 'body', 'type' => 'text'], + ['name' => 'created_at', 'type' => 'datetime', 'nullable' => true], + ['name' => 'updated_at', 'type' => 'datetime', 'nullable' => true], + ], + ]; + } + + protected function seedData(): array + { + return []; + } + + /** + * History tables are rebuilt for every test: some tests replace them with + * an unmigrated (branch-less) shape, and the DDL under test is per-driver. + */ + protected function resetTables(): void + { + parent::resetTables(); + + $this->recreateHistoryTable((new MockBranchableRecord())->historyTable(), false, true); + $this->recreateHistoryTable((new MockTemporalRecord())->historyTable(), true, false); + } + + protected function setupDatabaseConnections(): void + { + parent::setupDatabaseConnections(); + + // Hooks register once per class on first construction, so boot the + // classes first, then clear the registry and register them exactly once. + $branchable = new MockBranchableRecord(); + $temporal = new MockTemporalRecord(); + HookHandler::$hooks = []; + TemporalManager::resetCache(); + \Closure::bind(fn() => $this->registerTemporalHooks(), $branchable, MockBranchableRecord::class)(); + \Closure::bind(fn() => $this->registerTemporalHooks(), $temporal, MockTemporalRecord::class)(); + + Carbon::setTestNow(); + } + + #[After] + protected function resetTemporalState(): void + { + Carbon::setTestNow(); + HookHandler::$hooks = []; + TemporalManager::resetCache(); + } + + public function testBranchForksFromMainWithoutDisturbingMainHistory(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'cost basis is high', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '120'; + $record->save(); + + $this->travelTo('2026-02-01 12:00:00.000000'); + $this->assertTrue($record->branch('proposed-pricing', '2026-02-01 10:30:00')); + + $mainHistory = $record->history(); + $this->assertCount(2, $mainHistory); + $this->assertSame(['created', 'updated'], $mainHistory->map->__action->all()); + + $branchHistory = $record->history('proposed-pricing'); + $this->assertCount(1, $branchHistory); + $this->assertSame('branched', $branchHistory[0]->__action); + $this->assertSame(100, $branchHistory[0]->price); + + $onMain = MockBranchableRecord::at('2026-02-01 12:30:00')->find($record->id); + $onBranch = MockBranchableRecord::at('2026-02-01 12:30:00', 'proposed-pricing')->find($record->id); + + $this->assertSame(120, $onMain->price); + $this->assertSame(100, $onBranch->price); + } + + public function testBranchReturnsFalseWhenSourceSnapshotIsMissing(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->assertFalse($record->branch('too-early', '2026-01-01 00:00:00')); + } + + public function testBranchRejectsEmptyAndReservedNames(): void + { + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->expectException(\InvalidArgumentException::class); + $record->branch('main', '2026-02-01 10:00:00'); + } + + public function testBranchNameValidationAndDuplicateForkGuard(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + + foreach ([str_repeat('a', 51), '', 'has space', "quote'd"] as $invalid) { + try { + $record->branch($invalid, '2026-02-01 10:00:00'); + $this->fail("Expected branch name [{$invalid}] to be rejected."); + } catch (\InvalidArgumentException) { + $this->addToAssertionCount(1); + } + } + + // Read paths validate the name too, before it reaches the database. + $this->expectInvalidBranchName(fn() => MockBranchableRecord::at('2026-02-01 10:00:00', str_repeat('a', 51))); + $this->expectInvalidBranchName(fn() => $record->history(str_repeat('a', 51))); + $this->expectInvalidBranchName(fn() => $record->diff('2026-02-01', '2026-02-02', 'bad name')); + + $this->assertTrue($record->branch(str_repeat('a', 50), '2026-02-01 10:00:00')); + $this->assertTrue($record->branch('proposal', '2026-02-01 10:00:00')); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('already exists'); + $record->branch('proposal', '2026-02-01 10:00:00'); + } + + public function testBranchThrowsForModelsNotConfiguredForBranching(): void + { + $created = MockTemporalRecord::create([ + 'title' => 'Contract A', + 'status' => 'draft', + 'body' => 'Original body', + ]); + $record = MockTemporalRecord::find($created->id); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + $record->branch('attempt', '2026-01-01 00:00:00'); + } + + public function testAtWithNonMainBranchThrowsForModelsNotConfiguredForBranching(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + MockTemporalRecord::at('2026-01-01 00:00:00', 'some-branch'); + } + + public function testBranchOperationsRequireTheMigratedBranchColumn(): void + { + $this->recreateHistoryTable((new MockBranchableRecord())->historyTable(), false, false); + $this->pdo->exec("INSERT INTO branchable_records (title, price) VALUES ('Widget', '100')"); + $record = MockBranchableRecord::find(1); + TemporalManager::resetCache(); + + $this->assertMigrationGuard(fn() => MockBranchableRecord::at('2026-02-01 10:00:00')->find(1)); + $this->assertMigrationGuard(fn() => MockBranchableRecord::at('2026-02-01 10:00:00', 'proposal')->get()); + $this->assertMigrationGuard(fn() => $record->branch('proposal', '2026-02-01 10:00:00')); + $this->assertMigrationGuard(fn() => $record->history()); + $this->assertMigrationGuard(fn() => $record->diff('2026-02-01', '2026-02-02')); + $this->assertMigrationGuard(function () use ($record): void { + $record->title = 'Renamed'; + $record->save(); + }); + } + + public function testDiffRespectsCastsAndHidesUnexposableAttributes(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'cost basis is high', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '150'; + $record->internal_note = 'revised note'; + $record->save(); + + $diff = $record->diff('2026-02-01 10:30:00', '2026-02-01 11:30:00'); + + $this->assertIsInt($diff['changes']['price']['from']); + $this->assertIsInt($diff['changes']['price']['to']); + $this->assertSame(100, $diff['changes']['price']['from']); + $this->assertSame(150, $diff['changes']['price']['to']); + $this->assertArrayNotHasKey('internal_note', $diff['changes']); + $this->assertArrayNotHasKey('internal_note', $diff['added']); + $this->assertArrayNotHasKey('internal_note', $diff['removed']); + } + + public function testDiffDetectsArrayAndBooleanCastChanges(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', 'price' => '100', 'internal_note' => null, + 'metadata' => ['color' => 'blue'], 'enabled' => false, + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->metadata = ['color' => 'red']; + $record->enabled = true; + $record->save(); + + $diff = $record->diff('2026-02-01 10:30:00', '2026-02-01 11:30:00'); + + $this->assertSame(['color' => 'blue'], $diff['changes']['metadata']['from']); + $this->assertSame(['color' => 'red'], $diff['changes']['metadata']['to']); + $this->assertFalse($diff['changes']['enabled']['from']); + $this->assertTrue($diff['changes']['enabled']['to']); + $this->assertArrayNotHasKey('title', $diff['changes']); + } + + public function testBranchCanBeUpdatedDiffedAndRestoredByHistoryId(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->branch('proposal', '2026-02-01 10:30:00'); + + $proposal = MockBranchableRecord::at('2026-02-01 11:30:00', 'proposal')->find($record->id); + $proposal->price = '140'; + $this->assertTrue($proposal->saveToBranch('proposal')); + + // Writing to the branch never touches the live row or main history. + $this->assertSame(100, MockBranchableRecord::find($record->id)->price); + $this->assertCount(1, $record->history()); + + $diff = $record->diffAgainstBranch('proposal'); + $this->assertSame(100, $diff['changes']['price']['from']); + $this->assertSame(140, $diff['changes']['price']['to']); + + $branchHistory = $record->history('proposal'); + $this->assertSame(['branched', 'updated'], $branchHistory->map->__action->all()); + + $this->travelTo('2026-02-01 12:00:00.000000'); + $this->assertTrue($record->restoreVersion($branchHistory[1]->__history_id)); + $this->assertSame(140, MockBranchableRecord::find($record->id)->price); + $this->assertSame(['created', 'updated'], $record->history()->map->__action->all()); + } + + public function testSaveToBranchRequiresAnExistingBranch(): void + { + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not exist'); + $record->saveToBranch('never-forked'); + } + + public function testRestoreFromBranchCreatesANewMainVersionAndNeverRewritesHistory(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', + 'price' => '100', + 'internal_note' => 'n/a', + ]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->price = '120'; + $record->save(); + + $this->travelTo('2026-02-01 12:00:00.000000'); + $record->branch('rollback-candidate', '2026-02-01 10:30:00'); + + $this->travelTo('2026-02-01 13:00:00.000000'); + $this->assertTrue($record->restoreTo('2026-02-01 12:30:00', 'rollback-candidate')); + + $fresh = MockBranchableRecord::find($record->id); + $this->assertSame(100, $fresh->price); + + // Restoring persisted through the normal save()/snapshot path, so the + // main timeline only ever grew — nothing was rewritten. + $this->assertCount(3, $fresh->history()); + $this->assertSame(['created', 'updated', 'updated'], $fresh->history()->map->__action->all()); + } + + public function testRestoreVersionRejectsDeletedHistoryEntries(): void + { + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + $record->delete(); + $deleted = $record->history()[1]; + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('deleted temporal version'); + $record->restoreVersion($deleted->__history_id); + } + + public function testRestoreToRefusesAPointWhereTheRecordWasDeleted(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->delete(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('deleted temporal version'); + $record->restoreTo('2026-02-01 11:30:00'); + } + + public function testRestoreToNonMainBranchThrowsForModelsNotConfiguredForBranching(): void + { + $created = MockTemporalRecord::create([ + 'title' => 'Contract A', + 'status' => 'draft', + 'body' => 'Original body', + ]); + $record = MockTemporalRecord::find($created->id); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('does not support branches'); + $record->restoreTo('2026-01-01 00:00:00', 'some-branch'); + } + + public function testSameMicrosecondVersionsResolveToTheLatestHistoryId(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); + $record = MockBranchableRecord::find($created->id); + + // Same frozen clock: both updates share one valid_from value. + $record->price = '110'; + $record->save(); + $record->price = '120'; + $record->save(); + + $this->assertSame(120, MockBranchableRecord::at('2026-02-01 10:00:00')->find($record->id)->price); + $this->assertSame(120, MockBranchableRecord::at('2026-02-01 10:00:00')->get()->first()->price); + $this->assertSame(120, $record->rewindTo('2026-02-01 10:00:00')->price); + } + + public function testHasBranchColumnSeesAColumnAddedAfterANegativeCheck(): void + { + $table = 'migration_check_history'; + $driver = static::driverName(); + + $this->recreateHistoryTable($table, false, false); + + $this->assertFalse(TemporalManager::hasBranchColumn($this->pdo, $table, $driver)); + + // No resetCache(): a missing column must not be cached, so long-running + // workers see the column as soon as migrate:temporal adds it. + $this->pdo->exec(sprintf( + "ALTER TABLE %s ADD COLUMN branch VARCHAR(50) NOT NULL DEFAULT 'main'", + $this->quoteIdentifier($table) + )); + + $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $table, $driver)); + + $this->dropTablesIfExist([$table]); + } + + public function testTemporalManagerDdlIncludesBranchColumnAndIndexWhenEnabled(): void + { + $mysql = TemporalManager::createHistoryTableSql('widgets_history', 'mysql', false, true); + $pgsql = TemporalManager::createHistoryTableSql('widgets_history', 'pgsql', false, true); + $sqlite = TemporalManager::createHistoryTableSql('widgets_history', 'sqlite', false, true); + + $this->assertStringContainsString("`branch` VARCHAR(50) NOT NULL DEFAULT 'main'", $mysql[0]); + $this->assertStringContainsString('idx_branch', $mysql[0]); + + $this->assertCount(4, $pgsql); + $this->assertStringContainsString("branch VARCHAR(50) NOT NULL DEFAULT 'main'", $pgsql[0]); + $this->assertStringContainsString('idx_widgets_history_branch', $pgsql[3]); + + $this->assertCount(4, $sqlite); + $this->assertStringContainsString("branch TEXT NOT NULL DEFAULT 'main'", $sqlite[0]); + $this->assertStringContainsString('idx_widgets_history_branch', $sqlite[3]); + } + + private function recreateHistoryTable(string $table, bool $trackActor, bool $trackBranches): void + { + $this->dropTablesIfExist([$table]); + + foreach (TemporalManager::createHistoryTableSql($table, static::driverName(), $trackActor, $trackBranches) as $sql) { + $this->pdo->exec($sql); + } + } + + private function travelTo(string $datetime): void + { + Carbon::setTestNow(Carbon::parse($datetime, 'UTC')); + } + + private function assertMigrationGuard(callable $operation): void + { + try { + $operation(); + $this->fail('Expected missing branch column to require migrate:temporal.'); + } catch (RuntimeException $exception) { + $this->assertStringContainsString('run migrate:temporal', $exception->getMessage()); + } + } + + private function expectInvalidBranchName(callable $operation): void + { + try { + $operation(); + $this->fail('Expected an invalid branch name to be rejected.'); + } catch (\InvalidArgumentException) { + $this->addToAssertionCount(1); + } + } +} diff --git a/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php b/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php new file mode 100644 index 00000000..2f3c014e --- /dev/null +++ b/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php @@ -0,0 +1,17 @@ +container = new MockContainer(); - Container::setInstance($this->container); - - $this->pdo = new PDO('sqlite::memory:'); - $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - - $this->createTables(); - $this->setupDatabaseConnections(); - - HookHandler::$hooks = []; - TemporalManager::resetCache(); - $this->registerTemporalHooks(MockBranchableRecord::class); - $this->registerTemporalHooks(MockTemporalRecord::class); - - Carbon::setTestNow(); - } - - protected function tearDown(): void - { - Carbon::setTestNow(); - HookHandler::$hooks = []; - TemporalManager::resetCache(); - $this->tearDownDatabaseConnections(); - Container::forgetInstance(); - $this->container = null; - $this->pdo = null; - } - - public function testBranchForksFromMainWithoutDisturbingMainHistory(): void - { - $this->travelTo('2026-02-01 10:00:00.000000'); - $created = MockBranchableRecord::create([ - 'title' => 'Widget', - 'price' => '100', - 'internal_note' => 'cost basis is high', - ]); - $record = MockBranchableRecord::find($created->id); - - $this->travelTo('2026-02-01 11:00:00.000000'); - $record->price = '120'; - $record->save(); - - $this->travelTo('2026-02-01 12:00:00.000000'); - $this->assertTrue($record->branch('proposed-pricing', '2026-02-01 10:30:00')); - - $mainHistory = $record->history(); - $this->assertCount(2, $mainHistory); - $this->assertSame(['created', 'updated'], $mainHistory->map->__action->all()); - - $branchHistory = $record->history('proposed-pricing'); - $this->assertCount(1, $branchHistory); - $this->assertSame('branched', $branchHistory[0]->__action); - $this->assertSame(100, $branchHistory[0]->price); - - $onMain = MockBranchableRecord::at('2026-02-01 12:30:00')->find($record->id); - $onBranch = MockBranchableRecord::at('2026-02-01 12:30:00', 'proposed-pricing')->find($record->id); - - $this->assertSame(120, $onMain->price); - $this->assertSame(100, $onBranch->price); - } - - public function testBranchReturnsFalseWhenSourceSnapshotIsMissing(): void - { - $this->travelTo('2026-02-01 10:00:00.000000'); - $created = MockBranchableRecord::create([ - 'title' => 'Widget', - 'price' => '100', - 'internal_note' => 'n/a', - ]); - $record = MockBranchableRecord::find($created->id); - - $this->assertFalse($record->branch('too-early', '2026-01-01 00:00:00')); - } - - public function testBranchRejectsEmptyAndReservedNames(): void - { - $created = MockBranchableRecord::create([ - 'title' => 'Widget', - 'price' => '100', - 'internal_note' => 'n/a', - ]); - $record = MockBranchableRecord::find($created->id); - - $this->expectException(\InvalidArgumentException::class); - $record->branch('main', '2026-02-01 10:00:00'); - } - - public function testBranchThrowsForModelsNotConfiguredForBranching(): void - { - $created = MockTemporalRecord::create([ - 'title' => 'Contract A', - 'status' => 'draft', - 'body' => 'Original body', - ]); - $record = MockTemporalRecord::find($created->id); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('does not support branches'); - $record->branch('attempt', '2026-01-01 00:00:00'); - } - - public function testAtWithNonMainBranchThrowsForModelsNotConfiguredForBranching(): void - { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('does not support branches'); - MockTemporalRecord::at('2026-01-01 00:00:00', 'some-branch'); - } - - public function testDiffRespectsCastsAndHidesUnexposableAttributes(): void - { - $this->travelTo('2026-02-01 10:00:00.000000'); - $created = MockBranchableRecord::create([ - 'title' => 'Widget', - 'price' => '100', - 'internal_note' => 'cost basis is high', - ]); - $record = MockBranchableRecord::find($created->id); - - $this->travelTo('2026-02-01 11:00:00.000000'); - $record->price = '150'; - $record->internal_note = 'revised note'; - $record->save(); - - $diff = $record->diff('2026-02-01 10:30:00', '2026-02-01 11:30:00'); - - $this->assertIsInt($diff['changes']['price']['from']); - $this->assertIsInt($diff['changes']['price']['to']); - $this->assertSame(100, $diff['changes']['price']['from']); - $this->assertSame(150, $diff['changes']['price']['to']); - $this->assertArrayNotHasKey('internal_note', $diff['changes']); - $this->assertArrayNotHasKey('internal_note', $diff['added']); - $this->assertArrayNotHasKey('internal_note', $diff['removed']); - } - - public function testRestoreFromBranchCreatesANewMainVersionAndNeverRewritesHistory(): void - { - $this->travelTo('2026-02-01 10:00:00.000000'); - $created = MockBranchableRecord::create([ - 'title' => 'Widget', - 'price' => '100', - 'internal_note' => 'n/a', - ]); - $record = MockBranchableRecord::find($created->id); - - $this->travelTo('2026-02-01 11:00:00.000000'); - $record->price = '120'; - $record->save(); - - $this->travelTo('2026-02-01 12:00:00.000000'); - $record->branch('rollback-candidate', '2026-02-01 10:30:00'); - - $this->travelTo('2026-02-01 13:00:00.000000'); - $this->assertTrue($record->restoreTo('2026-02-01 12:30:00', 'rollback-candidate')); - - $fresh = MockBranchableRecord::find($record->id); - $this->assertSame(100, $fresh->price); - - // Restoring persisted through the normal save()/snapshot path, so the - // main timeline only ever grew — nothing was rewritten. - $this->assertCount(3, $fresh->history()); - $this->assertSame(['created', 'updated', 'updated'], $fresh->history()->map->__action->all()); - } - - public function testRestoreToNonMainBranchThrowsForModelsNotConfiguredForBranching(): void - { - $created = MockTemporalRecord::create([ - 'title' => 'Contract A', - 'status' => 'draft', - 'body' => 'Original body', - ]); - $record = MockTemporalRecord::find($created->id); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('does not support branches'); - $record->restoreTo('2026-01-01 00:00:00', 'some-branch'); - } - - public function testMigrateTemporalAddsBranchColumnToExistingTable(): void - { - // A table name unique to this test, so the process-lifetime column - // cache can't carry a stale answer in from another test method. - $table = 'migration_check_history'; - $driver = 'sqlite'; - - $statements = TemporalManager::createHistoryTableSql($table, $driver, false, false); - foreach ($statements as $statement) { - $this->pdo->exec($statement); - } - - $this->assertFalse( - TemporalManager::hasBranchColumn($this->pdo, $table, $driver) - ); - - $this->pdo->exec("ALTER TABLE {$table} ADD COLUMN branch TEXT NOT NULL DEFAULT 'main'"); - TemporalManager::resetCache(); - - $this->assertTrue( - TemporalManager::hasBranchColumn($this->pdo, $table, $driver) - ); - - $this->pdo->exec("DROP TABLE {$table}"); - } - - public function testTemporalManagerDdlIncludesBranchColumnAndIndexWhenEnabled(): void - { - $mysql = TemporalManager::createHistoryTableSql('widgets_history', 'mysql', false, true); - $pgsql = TemporalManager::createHistoryTableSql('widgets_history', 'pgsql', false, true); - $sqlite = TemporalManager::createHistoryTableSql('widgets_history', 'sqlite', false, true); - - $this->assertStringContainsString("`branch` VARCHAR(50) NOT NULL DEFAULT 'main'", $mysql[0]); - $this->assertStringContainsString('idx_branch', $mysql[0]); - - $this->assertCount(4, $pgsql); - $this->assertStringContainsString("branch VARCHAR(50) NOT NULL DEFAULT 'main'", $pgsql[0]); - $this->assertStringContainsString('idx_widgets_history_branch', $pgsql[3]); - - $this->assertCount(4, $sqlite); - $this->assertStringContainsString("branch TEXT NOT NULL DEFAULT 'main'", $sqlite[0]); - $this->assertStringContainsString('idx_widgets_history_branch', $sqlite[3]); - } - - private function createTables(): void - { - $this->pdo->exec( - 'CREATE TABLE branchable_records ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - title TEXT NOT NULL, - price TEXT NOT NULL, - internal_note TEXT NULL, - created_at TEXT NULL, - updated_at TEXT NULL - )' - ); - - $historyTable = (new MockBranchableRecord())->historyTable(); - $statements = TemporalManager::createHistoryTableSql($historyTable, 'sqlite', false, true); - foreach ($statements as $statement) { - $this->pdo->exec($statement); - } - - $this->pdo->exec( - 'CREATE TABLE temporal_records ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - title TEXT NOT NULL, - status TEXT NOT NULL, - body TEXT NOT NULL, - created_at TEXT NULL, - updated_at TEXT NULL - )' - ); - - $temporalHistoryTable = (new MockTemporalRecord())->historyTable(); - $temporalStatements = TemporalManager::createHistoryTableSql($temporalHistoryTable, 'sqlite', true, false); - foreach ($temporalStatements as $statement) { - $this->pdo->exec($statement); - } - } - - private function setupDatabaseConnections(): void - { - $this->setStaticProperty(Database::class, 'connections', []); - $this->setStaticProperty(Database::class, 'transactions', []); - - $this->setStaticProperty(Database::class, 'connections', [ - 'default' => $this->pdo, - 'sqlite' => $this->pdo, - ]); - } - - private function tearDownDatabaseConnections(): void - { - $this->setStaticProperty(Database::class, 'connections', []); - $this->setStaticProperty(Database::class, 'transactions', []); - } - - private function setStaticProperty(string $className, string $propertyName, mixed $value): void - { - $reflection = new \ReflectionClass($className); - $property = $reflection->getProperty($propertyName); - $property->setValue(null, $value); - } - - private function registerTemporalHooks(string $class): void - { - $model = new $class(); - $register = \Closure::bind(function (): void { - $this->registerTemporalHooks(); - }, $model, $class); - - $register(); - } - - private function travelTo(string $datetime): void - { - Carbon::setTestNow(Carbon::parse($datetime, 'UTC')); - } -} diff --git a/tests/Support/Model/MockBranchableRecord.php b/tests/Support/Model/MockBranchableRecord.php index 13f968fc..404819ea 100644 --- a/tests/Support/Model/MockBranchableRecord.php +++ b/tests/Support/Model/MockBranchableRecord.php @@ -3,6 +3,8 @@ namespace Tests\Support\Model; use Phaseolies\Database\Entity\Casts\Attributes\ToInteger; +use Phaseolies\Database\Entity\Casts\Attributes\ToArray; +use Phaseolies\Database\Entity\Casts\Attributes\ToBoolean; use Phaseolies\Database\Entity\Model; use Phaseolies\Database\Temporal\Attributes\Temporal; @@ -13,9 +15,15 @@ class MockBranchableRecord extends Model protected $primaryKey = 'id'; protected $connection = 'default'; protected $timeStamps = true; - protected $creatable = ['title', 'price', 'internal_note']; + protected $creatable = ['title', 'price', 'internal_note', 'metadata', 'enabled']; protected $unexposable = ['internal_note']; #[ToInteger] protected string $price; + + #[ToArray] + protected array $metadata; + + #[ToBoolean] + protected bool $enabled; } From d30e7c4b8ab46e7e50c38cca137ff4b7550af5ec Mon Sep 17 00:00:00 2001 From: Abdul Majid <43958759+abdulmajidcse@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:06:58 +0600 Subject: [PATCH 3/4] Address review round 2: safe main writes, diff types, branch API polish - Record main writes on branchable models before migrate:temporal; only named-branch operations require the branch column - Compare uncast scalars by value in diff() so 5 and '5' are equal - Keep the model dirty after saveToBranch() - Give diffAgainstBranch() its own result shape and return null when the live record is deleted - Cache column checks per connection, including misses, and clear them in migrate:temporal and resetCache() - Accept lowercase branch names only - Index (record_id, branch, valid_from); report columns and indexes separately in migrate:temporal - Document the restoreVersion()/restoreTo() return convention --- .../Migrations/MigrateTemporalCommand.php | 19 +++- .../Temporal/InteractsWithTemporal.php | 94 +++++++++++++++---- .../Database/Temporal/TemporalManager.php | 39 ++++---- tests/Console/MigrateTemporalCommandTest.php | 2 +- .../TemporalBranch/TemporalBranchBehavior.php | 79 ++++++++++++++-- 5 files changed, 185 insertions(+), 48 deletions(-) diff --git a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php index 276c7825..a75bbb8d 100644 --- a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php +++ b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php @@ -98,6 +98,7 @@ public function handle(): int if (!$dryRun && $this->tableExists($pdo, $driver, $historyTable)) { // Table exists — check if we need to add the actor and/or branch columns $addedColumns = []; + $addedIndexes = []; if ($attr?->trackActor && !$this->columnExists($pdo, $driver, $historyTable, 'actor')) { $pdo->exec($this->addActorColumnSql($driver, $historyTable)); @@ -112,11 +113,19 @@ public function handle(): int $branchIndex = $driver === 'mysql' ? 'idx_branch' : 'idx_' . $historyTable . '_branch'; if ($attr !== null && $attr->branches && !$this->indexExists($pdo, $driver, $historyTable, $branchIndex)) { $pdo->exec($this->createBranchIndexSql($driver, $historyTable)); - $addedColumns[] = 'branch index'; + $addedIndexes[] = 'branch'; } - if (!empty($addedColumns)) { - $row('✓', 'green', implode(', ', $addedColumns) . ' column' . (count($addedColumns) > 1 ? 's' : '') . ' added'); + if (!empty($addedColumns) || !empty($addedIndexes)) { + TemporalManager::resetCache(); + $status = []; + if (!empty($addedColumns)) { + $status[] = implode(', ', $addedColumns) . ' column' . (count($addedColumns) > 1 ? 's' : '') . ' added'; + } + if (!empty($addedIndexes)) { + $status[] = implode(', ', $addedIndexes) . ' index added'; + } + $row('✓', 'green', implode('; ', $status)); $created++; } else { $row('•', 'yellow', 'already exists'); @@ -369,8 +378,8 @@ private function createBranchIndexSql(string $driver, string $table): string { $name = $driver === 'mysql' ? 'idx_branch' : 'idx_' . $table . '_branch'; return match ($driver) { - 'pgsql', 'sqlite' => "CREATE INDEX \"{$name}\" ON \"{$table}\" (record_id, branch)", - default => "CREATE INDEX `{$name}` ON `{$table}` (record_id, branch)", + 'pgsql', 'sqlite' => "CREATE INDEX \"{$name}\" ON \"{$table}\" (record_id, branch, valid_from)", + default => "CREATE INDEX `{$name}` ON `{$table}` (record_id, branch, valid_from)", }; } } diff --git a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php index 361d8b19..7c6568e9 100644 --- a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php +++ b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php @@ -110,7 +110,6 @@ public function saveToBranch(string $branch): bool throw new \InvalidArgumentException('saveToBranch() requires a named branch.'); } TemporalManager::snapshotBranchVersion($this, $branch); - $this->setOriginalAttributes($this->getAttributes()); return true; } @@ -123,13 +122,27 @@ public function diffAgainstBranch(string $branch): ?array throw new \InvalidArgumentException('A named branch is required.'); } $now = \Carbon\Carbon::now()->format('Y-m-d H:i:s.u'); + if ($this->latestActionAt($now, 'main') === 'deleted') { + return null; + } $main = $this->snapshotAt($now, 'main'); $proposed = $this->snapshotAt($now, $branch); if ($main === null || $proposed === null) { return null; } $probe = new static(); - return $this->formatDiff($main, $proposed, 'main → ' . $branch, $now, $now, $probe); + $diff = $this->formatDiff($main, $proposed, $branch, $now, $now, $probe); + $visibleMain = $this->visibleSnapshot($main, $probe); + $visibleBranch = $this->visibleSnapshot($proposed, $probe); + + return [ + 'at' => $now, + 'main' => $visibleMain, + 'branch' => ['name' => $branch, 'snapshot' => $visibleBranch], + 'changes' => $diff['changes'], + 'added' => $diff['added'], + 'removed' => $diff['removed'], + ]; } /** @@ -237,7 +250,7 @@ private function formatDiff(array $snapshotFrom, array $snapshotTo, string $bran foreach ($visibleTo as $col => $newVal) { if (!array_key_exists($col, $visibleFrom)) { $added[$col] = $newVal; - } elseif (!$this->sameCastValue($visibleFrom[$col], $newVal)) { + } elseif (!$this->sameCastValue($col, $visibleFrom[$col], $newVal, $probe)) { $changes[$col] = ['from' => $visibleFrom[$col], 'to' => $newVal]; } } @@ -258,12 +271,21 @@ private function formatDiff(array $snapshotFrom, array $snapshotTo, string $bran ]; } - private function sameCastValue(mixed $left, mixed $right): bool + private function sameCastValue(string $column, mixed $left, mixed $right, Model $probe): bool { if (is_object($left) && is_object($right)) { return $left == $right; } - return $left === $right; + + if ($probe->hasCast($column) || is_array($left) || is_array($right)) { + return $left === $right; + } + + if ($left === null || $right === null) { + return $left === $right; + } + + return (string) $left === (string) $right; } /** @@ -325,6 +347,11 @@ public function rewindTo(string $datetime, string $branch = 'main'): ?static * existing history, and restoring from a branch other than "main" * requires the model to be configured for branches. * + * Returns false when no snapshot exists at that point. On branchable + * models it throws when the latest entry at that point is a "deleted" + * snapshot; non-branchable models keep the pre-branch behaviour and + * restore the state captured in that snapshot. + * * @param string $datetime * @param string $branch * @return bool @@ -357,19 +384,16 @@ public function restoreTo(string $datetime, string $branch = 'main'): bool private function assertNotDeletedAt(string $datetime, string $branch): void { TemporalManager::assertBranchSupported(static::class, $branch); + if (!TemporalManager::isBranchable(static::class)) { + return; + } $datetime = $this->normalizeTemporalDateTime($datetime); $table = $this->historyTable(); $pdo = $this->getConnection(); $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); - $hasBranch = TemporalManager::isBranchable(static::class); - if ($hasBranch) { - TemporalManager::assertBranchColumn($pdo, $table, $driver); - } - $sql = "SELECT action FROM {$table} WHERE record_id = ? AND valid_from <= ?" . ($hasBranch ? ' AND branch = ?' : '') . ' ORDER BY valid_from DESC, history_id DESC LIMIT 1'; - $params = [$this->getKey(), $datetime]; - if ($hasBranch) { - $params[] = $branch; - } + TemporalManager::assertBranchColumn($pdo, $table, $driver); + $sql = "SELECT action FROM {$table} WHERE record_id = ? AND valid_from <= ? AND branch = ? ORDER BY valid_from DESC, history_id DESC LIMIT 1"; + $params = [$this->getKey(), $datetime, $branch]; $stmt = $pdo->prepare($sql); $stmt->execute($params); if ($stmt->fetchColumn() === 'deleted') { @@ -377,7 +401,20 @@ private function assertNotDeletedAt(string $datetime, string $branch): void } } - /** Restore a specific history entry by id, refusing deleted snapshots. */ + /** + * Restore this record to a specific history entry and persist it as a new + * version through the normal save()/snapshot hooks. + * + * Follows the same convention as restoreTo(): returns false when there is + * nothing to restore (the id is unknown or belongs to another record) and + * throws when the restore itself is refused (the entry is a "deleted" + * snapshot). + * + * @param int $historyId + * @return bool + * + * @throws \RuntimeException + */ public function restoreVersion(int $historyId): bool { $this->assertTemporalAndLoaded(); @@ -390,8 +427,7 @@ public function restoreVersion(int $historyId): bool } $stmt = $pdo->prepare( - "SELECT snapshot, action" . ($hasBranch ? ', branch' : '') - . " FROM {$historyTable} WHERE history_id = ? AND record_id = ?" + "SELECT snapshot, action FROM {$historyTable} WHERE history_id = ? AND record_id = ?" ); $stmt->execute([$historyId, $this->getKey()]); $row = $stmt->fetch(PDO::FETCH_ASSOC); @@ -473,6 +509,30 @@ private function snapshotAt(string $datetime, string $branch = 'main'): ?array return $row ? (json_decode($row['snapshot'], true) ?? null) : null; } + private function latestActionAt(string $datetime, string $branch): ?string + { + $historyTable = $this->historyTable(); + $pdo = $this->getConnection(); + $driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + $hasBranch = TemporalManager::isBranchable(static::class); + if ($hasBranch) { + TemporalManager::assertBranchColumn($pdo, $historyTable, $driver); + } + + $sql = "SELECT action FROM {$historyTable} WHERE record_id = ? AND valid_from <= ?" + . ($hasBranch ? ' AND branch = ?' : '') + . ' ORDER BY valid_from DESC, history_id DESC LIMIT 1'; + $params = [$this->getKey(), $this->normalizeTemporalDateTime($datetime)]; + if ($hasBranch) { + $params[] = $branch; + } + $stmt = $pdo->prepare($sql); + $stmt->execute($params); + $action = $stmt->fetchColumn(); + + return $action === false ? null : (string) $action; + } + private function normalizeTemporalDateTime(string $datetime): string { $datetime = trim($datetime); diff --git a/src/Phaseolies/Database/Temporal/TemporalManager.php b/src/Phaseolies/Database/Temporal/TemporalManager.php index 56f79d11..69f861e3 100644 --- a/src/Phaseolies/Database/Temporal/TemporalManager.php +++ b/src/Phaseolies/Database/Temporal/TemporalManager.php @@ -18,9 +18,9 @@ class TemporalManager /** * Per-process cache of history table column existence checks * - * @var array + * @var \WeakMap>|null */ - private static array $columnCache = []; + private static ?\WeakMap $columnCache = null; /** * Determine whether the given model class carries #[Temporal]. @@ -100,8 +100,8 @@ public static function assertBranchSupported(string $class, string $branch): voi /** Validate branch names consistently with the persisted column. */ public static function validateBranchName(string $branch): void { - if (strlen($branch) > 50 || !preg_match('/^[A-Za-z0-9_-]+$/', $branch)) { - throw new \InvalidArgumentException('Branch names must be 1-50 characters using letters, numbers, _ or -.'); + if (strlen($branch) > 50 || !preg_match('/^[a-z0-9_-]+$/', $branch)) { + throw new \InvalidArgumentException('Branch names must be 1-50 lowercase characters using letters, numbers, _ or -.'); } } @@ -161,8 +161,11 @@ public static function snapshot(Model $model, string $action, string $branch = ' } if ($attr !== null && $attr->branches) { - self::assertBranchColumn($pdo, $historyTable, $driver); - $data['branch'] = $branch; + if (self::hasBranchColumn($pdo, $historyTable, $driver)) { + $data['branch'] = $branch; + } elseif ($branch !== 'main') { + self::assertBranchColumn($pdo, $historyTable, $driver); + } } self::insertHistory($pdo, $historyTable, $data); @@ -261,9 +264,11 @@ public static function hasBranchColumn(PDO $pdo, string $historyTable, string $d */ private static function hasColumn(PDO $pdo, string $historyTable, string $column, string $driver): bool { - $cacheKey = spl_object_id($pdo) . '.' . $historyTable . '.' . $column; + self::$columnCache ??= new \WeakMap(); + $connectionCache = self::$columnCache[$pdo] ?? []; + $cacheKey = $historyTable . '.' . $column; - if (!array_key_exists($cacheKey, self::$columnCache)) { + if (!array_key_exists($cacheKey, $connectionCache)) { try { $exists = match ($driver) { 'pgsql' => (function () use ($pdo, $historyTable, $column): bool { @@ -289,15 +294,15 @@ private static function hasColumn(PDO $pdo, string $historyTable, string $column return $stmt->fetch() !== false; })(), }; - if ($exists) { - self::$columnCache[$cacheKey] = true; - } + $connectionCache[$cacheKey] = $exists; + self::$columnCache[$pdo] = $connectionCache; } catch (\Throwable) { - unset(self::$columnCache[$cacheKey]); + unset($connectionCache[$cacheKey]); + self::$columnCache[$pdo] = $connectionCache; } } - return self::$columnCache[$cacheKey] ?? false; + return $connectionCache[$cacheKey] ?? false; } /** @@ -376,7 +381,7 @@ public static function createHistoryTableSql( private static function mysqlDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { - $branchIndex = $trackBranches ? ",\n INDEX `idx_branch` (`record_id`, `branch`)" : ''; + $branchIndex = $trackBranches ? ",\n INDEX `idx_branch` (`record_id`, `branch`, `valid_from`)" : ''; return [ "CREATE TABLE IF NOT EXISTS `{$table}` ( @@ -412,7 +417,7 @@ private static function pgsqlDdl(string $table, string $actorCol, string $branch ]; if ($trackBranches) { - $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch)"; + $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch, valid_from)"; } return $stmts; @@ -435,7 +440,7 @@ private static function sqliteDdl(string $table, string $actorCol, string $branc ]; if ($trackBranches) { - $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch)"; + $stmts[] = "CREATE INDEX IF NOT EXISTS \"idx_{$table}_branch\" ON \"{$table}\" (record_id, branch, valid_from)"; } return $stmts; @@ -471,7 +476,7 @@ public static function resetCache(?string $class = null): void unset(self::$cache[$class]); } else { self::$cache = []; - self::$columnCache = []; + self::$columnCache = null; } } } diff --git a/tests/Console/MigrateTemporalCommandTest.php b/tests/Console/MigrateTemporalCommandTest.php index 2d7d5cc9..a09fbe62 100644 --- a/tests/Console/MigrateTemporalCommandTest.php +++ b/tests/Console/MigrateTemporalCommandTest.php @@ -144,7 +144,7 @@ public function testHandleAddsMissingActorAndBranchColumnsToAnExistingTable(): v $this->assertContains('idx_' . $historyTable . '_branch', array_column($indexes, 'name')); $status = implode(' ', array_map(fn ($line) => $line[0], $command->capturedLines)); - $this->assertStringContainsString('actor, branch, branch index columns added', $status); + $this->assertStringContainsString('actor, branch columns added; branch index added', $status); } public function testHandleIsIdempotentOnceColumnsExist(): void diff --git a/tests/Model/TemporalBranch/TemporalBranchBehavior.php b/tests/Model/TemporalBranch/TemporalBranchBehavior.php index 3bffd657..82e8fc02 100644 --- a/tests/Model/TemporalBranch/TemporalBranchBehavior.php +++ b/tests/Model/TemporalBranch/TemporalBranchBehavior.php @@ -146,7 +146,7 @@ public function testBranchNameValidationAndDuplicateForkGuard(): void $created = MockBranchableRecord::create(['title' => 'Widget', 'price' => '100', 'internal_note' => null]); $record = MockBranchableRecord::find($created->id); - foreach ([str_repeat('a', 51), '', 'has space', "quote'd"] as $invalid) { + foreach ([str_repeat('a', 51), '', 'has space', "quote'd", 'MAIN', 'Draft'] as $invalid) { try { $record->branch($invalid, '2026-02-01 10:00:00'); $this->fail("Expected branch name [{$invalid}] to be rejected."); @@ -201,10 +201,23 @@ public function testBranchOperationsRequireTheMigratedBranchColumn(): void $this->assertMigrationGuard(fn() => $record->branch('proposal', '2026-02-01 10:00:00')); $this->assertMigrationGuard(fn() => $record->history()); $this->assertMigrationGuard(fn() => $record->diff('2026-02-01', '2026-02-02')); - $this->assertMigrationGuard(function () use ($record): void { - $record->title = 'Renamed'; - $record->save(); - }); + } + + public function testMainWritesRemainSafeBeforeBranchColumnMigration(): void + { + $this->recreateHistoryTable((new MockBranchableRecord())->historyTable(), false, false); + TemporalManager::resetCache(); + + $record = MockBranchableRecord::create([ + 'title' => 'Widget', 'price' => '100', 'internal_note' => null, + ]); + $record = MockBranchableRecord::find($record->id); + $record->title = 'Renamed'; + $this->assertTrue($record->save()); + $this->assertTrue($record->delete()); + + $count = $this->pdo->query('SELECT COUNT(*) FROM branchable_records_history')->fetchColumn(); + $this->assertSame(3, (int) $count); } public function testDiffRespectsCastsAndHidesUnexposableAttributes(): void @@ -256,6 +269,24 @@ public function testDiffDetectsArrayAndBooleanCastChanges(): void $this->assertArrayNotHasKey('title', $diff['changes']); } + public function testDiffIgnoresEquivalentUncastScalarTypes(): void + { + $this->pdo->exec("INSERT INTO branchable_records (title, price) VALUES ('5', '100')"); + $record = MockBranchableRecord::find(1); + $table = $record->historyTable(); + $first = json_encode(['id' => 1, 'title' => 5, 'price' => '100']); + $second = json_encode(['id' => 1, 'title' => '5', 'price' => '100']); + $stmt = $this->pdo->prepare( + "INSERT INTO {$table} (record_id, action, valid_from, snapshot, branch) VALUES (?, 'updated', ?, ?, 'main')" + ); + $stmt->execute([1, '2026-02-01 10:00:00.000000', $first]); + $stmt->execute([1, '2026-02-01 11:00:00.000000', $second]); + + $diff = $record->diff('2026-02-01 10:30:00', '2026-02-01 11:30:00'); + + $this->assertArrayNotHasKey('title', $diff['changes']); + } + public function testBranchCanBeUpdatedDiffedAndRestoredByHistoryId(): void { $this->travelTo('2026-02-01 10:00:00.000000'); @@ -268,12 +299,16 @@ public function testBranchCanBeUpdatedDiffedAndRestoredByHistoryId(): void $proposal = MockBranchableRecord::at('2026-02-01 11:30:00', 'proposal')->find($record->id); $proposal->price = '140'; $this->assertTrue($proposal->saveToBranch('proposal')); + $this->assertArrayHasKey('price', $proposal->getDirtyAttributes()); // Writing to the branch never touches the live row or main history. $this->assertSame(100, MockBranchableRecord::find($record->id)->price); $this->assertCount(1, $record->history()); $diff = $record->diffAgainstBranch('proposal'); + $this->assertSame('proposal', $diff['branch']['name']); + $this->assertSame(100, $diff['main']['price']); + $this->assertSame(140, $diff['branch']['snapshot']['price']); $this->assertSame(100, $diff['changes']['price']['from']); $this->assertSame(140, $diff['changes']['price']['to']); @@ -365,6 +400,33 @@ public function testRestoreToNonMainBranchThrowsForModelsNotConfiguredForBranchi $record->restoreTo('2026-01-01 00:00:00', 'some-branch'); } + public function testRestoreToKeepsLegacyBehaviorForNonBranchableModels(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockTemporalRecord::create([ + 'title' => 'Contract A', 'status' => 'draft', 'body' => 'Original body', + ]); + $record = MockTemporalRecord::find($created->id); + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->delete(); + + $this->assertTrue($record->restoreTo('2026-02-01 11:30:00')); + } + + public function testDiffAgainstBranchReturnsNullAfterMainDeletion(): void + { + $this->travelTo('2026-02-01 10:00:00.000000'); + $created = MockBranchableRecord::create([ + 'title' => 'Widget', 'price' => '100', 'internal_note' => null, + ]); + $record = MockBranchableRecord::find($created->id); + $record->branch('proposal', '2026-02-01 10:00:00'); + $this->travelTo('2026-02-01 11:00:00.000000'); + $record->delete(); + + $this->assertNull($record->diffAgainstBranch('proposal')); + } + public function testSameMicrosecondVersionsResolveToTheLatestHistoryId(): void { $this->travelTo('2026-02-01 10:00:00.000000'); @@ -391,12 +453,11 @@ public function testHasBranchColumnSeesAColumnAddedAfterANegativeCheck(): void $this->assertFalse(TemporalManager::hasBranchColumn($this->pdo, $table, $driver)); - // No resetCache(): a missing column must not be cached, so long-running - // workers see the column as soon as migrate:temporal adds it. $this->pdo->exec(sprintf( "ALTER TABLE %s ADD COLUMN branch VARCHAR(50) NOT NULL DEFAULT 'main'", $this->quoteIdentifier($table) )); + TemporalManager::resetCache(); $this->assertTrue(TemporalManager::hasBranchColumn($this->pdo, $table, $driver)); @@ -410,15 +471,17 @@ public function testTemporalManagerDdlIncludesBranchColumnAndIndexWhenEnabled(): $sqlite = TemporalManager::createHistoryTableSql('widgets_history', 'sqlite', false, true); $this->assertStringContainsString("`branch` VARCHAR(50) NOT NULL DEFAULT 'main'", $mysql[0]); - $this->assertStringContainsString('idx_branch', $mysql[0]); + $this->assertStringContainsString('INDEX `idx_branch` (`record_id`, `branch`, `valid_from`)', $mysql[0]); $this->assertCount(4, $pgsql); $this->assertStringContainsString("branch VARCHAR(50) NOT NULL DEFAULT 'main'", $pgsql[0]); $this->assertStringContainsString('idx_widgets_history_branch', $pgsql[3]); + $this->assertStringContainsString('(record_id, branch, valid_from)', $pgsql[3]); $this->assertCount(4, $sqlite); $this->assertStringContainsString("branch TEXT NOT NULL DEFAULT 'main'", $sqlite[0]); $this->assertStringContainsString('idx_widgets_history_branch', $sqlite[3]); + $this->assertStringContainsString('(record_id, branch, valid_from)', $sqlite[3]); } private function recreateHistoryTable(string $table, bool $trackActor, bool $trackBranches): void From 8a0c5f4a5389429b403a656d8533cf29449a3b88 Mon Sep 17 00:00:00 2001 From: Abdul Majid <43958759+abdulmajidcse@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:05:00 +0600 Subject: [PATCH 4/4] Follow the PHPDoc convention for temporal branch functions --- .../Migrations/MigrateTemporalCommand.php | 16 +++++ .../Temporal/InteractsWithTemporal.php | 39 ++++++++++- .../Database/Temporal/TemporalManager.php | 64 ++++++++++++++++++- tests/Console/MigrateTemporalCommandTest.php | 28 ++++++++ .../TemporalBranch/TemporalBranchBehavior.php | 48 ++++++++++++++ .../TemporalBranchMysqlTest.php | 5 ++ .../TemporalBranchPgsqlTest.php | 5 ++ .../TemporalBranchSqliteTest.php | 5 ++ 8 files changed, 205 insertions(+), 5 deletions(-) diff --git a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php index a75bbb8d..c2325175 100644 --- a/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php +++ b/src/Phaseolies/Console/Commands/Migrations/MigrateTemporalCommand.php @@ -352,6 +352,15 @@ private function addBranchColumnSql(string $driver, string $table): string }; } + /** + * Check whether the given index exists on the given table. + * + * @param PDO $pdo + * @param string $driver + * @param string $table + * @param string $index + * @return bool + */ private function indexExists(PDO $pdo, string $driver, string $table, string $index): bool { try { @@ -374,6 +383,13 @@ private function indexExists(PDO $pdo, string $driver, string $table, string $in } } + /** + * Generate the CREATE INDEX SQL for the branch lookup index. + * + * @param string $driver + * @param string $table + * @return string + */ private function createBranchIndexSql(string $driver, string $table): string { $name = $driver === 'mysql' ? 'idx_branch' : 'idx_' . $table . '_branch'; diff --git a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php index 7bdeab02..cedf03bf 100644 --- a/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php +++ b/src/Phaseolies/Database/Temporal/InteractsWithTemporal.php @@ -101,7 +101,16 @@ public function branch(string $name, string $fromDatetime): bool return true; } - /** Append this in-memory proposed state as a new version on an existing branch. */ + /** + * Append this in-memory proposed state as a new version on an existing + * branch. The live record is not written and the model stays dirty. + * + * @param string $branch + * @return bool + * + * @throws \InvalidArgumentException + * @throws \RuntimeException + */ public function saveToBranch(string $branch): bool { $this->assertTemporalAndLoaded(); @@ -113,7 +122,15 @@ public function saveToBranch(string $branch): bool return true; } - /** Compare the current main state with the latest state on a named branch. */ + /** + * Compare the current main state with the latest state on a named branch. + * + * @param string $branch + * @return array|null + * + * @throws \InvalidArgumentException + * @throws \RuntimeException + */ public function diffAgainstBranch(string $branch): ?array { $this->assertTemporalAndLoaded(); @@ -235,6 +252,17 @@ public function diff(string $from, string $to, string $branch = 'main'): ?array return $this->formatDiff($snapshotFrom, $snapshotTo, $branch, $from, $to, new static()); } + /** + * Build the structured diff between two raw snapshots. + * + * @param array $snapshotFrom + * @param array $snapshotTo + * @param string $branch + * @param string $from + * @param string $to + * @param Model $probe + * @return array + */ private function formatDiff(array $snapshotFrom, array $snapshotTo, string $branch, string $from, string $to, Model $probe): array { $visibleFrom = $this->visibleSnapshot($snapshotFrom, $probe); @@ -273,6 +301,7 @@ private function formatDiff(array $snapshotFrom, array $snapshotTo, string $bran * to a raw stored snapshot, for display/diffing purposes only. * * @param array $snapshot + * @param Model $probe * @return array */ private function visibleSnapshot(array $snapshot, Model $probe): array @@ -513,6 +542,12 @@ private static function snapshotScalar(mixed $value): string return is_bool($value) ? ($value ? '1' : '0') : (string) $value; } + /** + * Expand a partial datetime to the end of the given day, minute or second. + * + * @param string $datetime + * @return string + */ private function normalizeTemporalDateTime(string $datetime): string { $datetime = trim($datetime); diff --git a/src/Phaseolies/Database/Temporal/TemporalManager.php b/src/Phaseolies/Database/Temporal/TemporalManager.php index 69f861e3..577b3fa0 100644 --- a/src/Phaseolies/Database/Temporal/TemporalManager.php +++ b/src/Phaseolies/Database/Temporal/TemporalManager.php @@ -97,7 +97,14 @@ public static function assertBranchSupported(string $class, string $branch): voi } } - /** Validate branch names consistently with the persisted column. */ + /** + * Validate branch names consistently with the persisted column. + * + * @param string $branch + * @return void + * + * @throws \InvalidArgumentException + */ public static function validateBranchName(string $branch): void { if (strlen($branch) > 50 || !preg_match('/^[a-z0-9_-]+$/', $branch)) { @@ -105,7 +112,16 @@ public static function validateBranchName(string $branch): void } } - /** Ensure branch-enabled models have completed migrate:temporal. */ + /** + * Ensure branch-enabled models have completed migrate:temporal. + * + * @param PDO $pdo + * @param string $historyTable + * @param string $driver + * @return void + * + * @throws \RuntimeException + */ public static function assertBranchColumn(PDO $pdo, string $historyTable, string $driver): void { if (!self::hasBranchColumn($pdo, $historyTable, $driver)) { @@ -212,7 +228,16 @@ public static function snapshotBranch(Model $model, array $snapshot, string $bra self::insertHistory($pdo, $historyTable, $data); } - /** Append an edited proposed state to an existing branch without saving the live record. */ + /** + * Append an edited proposed state to an existing branch without saving + * the live record. + * + * @param Model $model + * @param string $branch + * @return void + * + * @throws \RuntimeException + */ public static function snapshotBranchVersion(Model $model, string $branch): void { $pdo = $model->getConnection(); @@ -379,6 +404,15 @@ public static function createHistoryTableSql( }; } + /** + * Generate the MySQL history table DDL. + * + * @param string $table + * @param string $actorCol + * @param string $branchCol + * @param bool $trackBranches + * @return array + */ private static function mysqlDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { $branchIndex = $trackBranches ? ",\n INDEX `idx_branch` (`record_id`, `branch`, `valid_from`)" : ''; @@ -400,6 +434,15 @@ private static function mysqlDdl(string $table, string $actorCol, string $branch ]; } + /** + * Generate the PostgreSQL history table DDL. + * + * @param string $table + * @param string $actorCol + * @param string $branchCol + * @param bool $trackBranches + * @return array + */ private static function pgsqlDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { $stmts = [ @@ -423,6 +466,15 @@ private static function pgsqlDdl(string $table, string $actorCol, string $branch return $stmts; } + /** + * Generate the SQLite history table DDL. + * + * @param string $table + * @param string $actorCol + * @param string $branchCol + * @param bool $trackBranches + * @return array + */ private static function sqliteDdl(string $table, string $actorCol, string $branchCol, bool $trackBranches): array { $stmts = [ @@ -455,6 +507,12 @@ private static function actorColumnSql(string $driver): string }; } + /** + * Generate the branch column definition for the given driver. + * + * @param string $driver + * @return string + */ private static function branchColumnSql(string $driver): string { return match ($driver) { diff --git a/tests/Console/MigrateTemporalCommandTest.php b/tests/Console/MigrateTemporalCommandTest.php index a09fbe62..03e78c09 100644 --- a/tests/Console/MigrateTemporalCommandTest.php +++ b/tests/Console/MigrateTemporalCommandTest.php @@ -47,6 +47,11 @@ protected function setUp(): void $this->setStaticProperty(Database::class, 'connections', ['default' => $this->pdo]); } + /** + * Reset connections, temporal caches and temporary directories. + * + * @return void + */ protected function tearDown(): void { $this->setStaticProperty(Database::class, 'connections', []); @@ -204,6 +209,12 @@ private function writeDiscoverableModel(): string return $dir; } + /** + * Recursively delete a temporary directory. + * + * @param string $dir + * @return void + */ private function removeDirectory(string $dir): void { if (!is_dir($dir)) { @@ -223,7 +234,10 @@ private function removeDirectory(string $dir): void } /** + * Build a migrate:temporal command that captures its output. + * * @param array $options + * @return MigrateTemporalCommand */ private function makeCommand(array $options): MigrateTemporalCommand { @@ -236,6 +250,12 @@ private function makeCommand(array $options): MigrateTemporalCommand return $command; } + /** + * Check whether the given table exists in the SQLite test database. + * + * @param string $table + * @return bool + */ private function tableExists(string $table): bool { $stmt = $this->pdo->prepare("SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?"); @@ -244,6 +264,14 @@ private function tableExists(string $table): bool return (bool) $stmt->fetchColumn(); } + /** + * Overwrite a static property through reflection. + * + * @param string $className + * @param string $propertyName + * @param mixed $value + * @return void + */ private function setStaticProperty(string $className, string $propertyName, mixed $value): void { $reflection = new \ReflectionClass($className); diff --git a/tests/Model/TemporalBranch/TemporalBranchBehavior.php b/tests/Model/TemporalBranch/TemporalBranchBehavior.php index 11788e1f..b51b600e 100644 --- a/tests/Model/TemporalBranch/TemporalBranchBehavior.php +++ b/tests/Model/TemporalBranch/TemporalBranchBehavior.php @@ -16,6 +16,11 @@ abstract class TemporalBranchTest extends ModelQueryDriverTestCase { + /** + * Define the live tables used by the branch tests. + * + * @return array + */ protected function tableDefinitions(): array { return [ @@ -40,6 +45,11 @@ protected function tableDefinitions(): array ]; } + /** + * The branch tests create their own records. + * + * @return array + */ protected function seedData(): array { return []; @@ -48,6 +58,8 @@ protected function seedData(): array /** * History tables are rebuilt for every test: some tests replace them with * an unmigrated (branch-less) shape, and the DDL under test is per-driver. + * + * @return void */ protected function resetTables(): void { @@ -57,6 +69,11 @@ protected function resetTables(): void $this->recreateHistoryTable((new MockTemporalRecord())->historyTable(), true, false); } + /** + * Connect the database and register each mock model's temporal hooks once. + * + * @return void + */ protected function setupDatabaseConnections(): void { parent::setupDatabaseConnections(); @@ -74,6 +91,11 @@ protected function setupDatabaseConnections(): void } #[After] + /** + * Clear the frozen clock, registered hooks and temporal caches. + * + * @return void + */ protected function resetTemporalState(): void { Carbon::setTestNow(); @@ -485,6 +507,14 @@ public function testTemporalManagerDdlIncludesBranchColumnAndIndexWhenEnabled(): $this->assertStringContainsString('(record_id, branch, valid_from)', $sqlite[3]); } + /** + * Drop and recreate a history table with the given optional columns. + * + * @param string $table + * @param bool $trackActor + * @param bool $trackBranches + * @return void + */ private function recreateHistoryTable(string $table, bool $trackActor, bool $trackBranches): void { $this->dropTablesIfExist([$table]); @@ -494,11 +524,23 @@ private function recreateHistoryTable(string $table, bool $trackActor, bool $tra } } + /** + * Freeze the clock at the given UTC datetime. + * + * @param string $datetime + * @return void + */ private function travelTo(string $datetime): void { Carbon::setTestNow(Carbon::parse($datetime, 'UTC')); } + /** + * Assert that the operation asks for migrate:temporal to be run. + * + * @param callable $operation + * @return void + */ private function assertMigrationGuard(callable $operation): void { try { @@ -509,6 +551,12 @@ private function assertMigrationGuard(callable $operation): void } } + /** + * Assert that the operation rejects the branch name. + * + * @param callable $operation + * @return void + */ private function expectInvalidBranchName(callable $operation): void { try { diff --git a/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php b/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php index 2f3c014e..aee34149 100644 --- a/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php +++ b/tests/Model/TemporalBranch/TemporalBranchMysqlTest.php @@ -10,6 +10,11 @@ #[Group('mysql')] final class TemporalBranchMysqlTest extends TemporalBranchTest { + /** + * Run the branch tests against MySQL. + * + * @return string + */ protected static function driverName(): string { return 'mysql'; diff --git a/tests/Model/TemporalBranch/TemporalBranchPgsqlTest.php b/tests/Model/TemporalBranch/TemporalBranchPgsqlTest.php index 95553974..c68f31ea 100644 --- a/tests/Model/TemporalBranch/TemporalBranchPgsqlTest.php +++ b/tests/Model/TemporalBranch/TemporalBranchPgsqlTest.php @@ -10,6 +10,11 @@ #[Group('pgsql')] final class TemporalBranchPgsqlTest extends TemporalBranchTest { + /** + * Run the branch tests against PostgreSQL. + * + * @return string + */ protected static function driverName(): string { return 'pgsql'; diff --git a/tests/Model/TemporalBranch/TemporalBranchSqliteTest.php b/tests/Model/TemporalBranch/TemporalBranchSqliteTest.php index 42ba1eff..077f20d9 100644 --- a/tests/Model/TemporalBranch/TemporalBranchSqliteTest.php +++ b/tests/Model/TemporalBranch/TemporalBranchSqliteTest.php @@ -6,6 +6,11 @@ final class TemporalBranchSqliteTest extends TemporalBranchTest { + /** + * Run the branch tests against SQLite. + * + * @return string + */ protected static function driverName(): string { return 'sqlite';