From a4f147af51a0a302b44a5ab34786da72a209db03 Mon Sep 17 00:00:00 2001 From: Dave Barnwell Date: Sun, 8 Mar 2026 09:27:01 +0000 Subject: [PATCH 1/2] Implement phase 3 ORM hardening --- README.md | 6 + src/Model/Model.php | 56 +++++- test-src/Model/CodedCategory.php | 16 ++ test-src/Model/MetadataRefreshCategory.php | 15 ++ test-src/Model/SchemaQualifiedCategory.php | 14 ++ test-src/Model/UntimedCategory.php | 14 ++ tests/Model/CategoryTest.php | 193 ++++++++++++++++++++- 7 files changed, 302 insertions(+), 12 deletions(-) create mode 100644 test-src/Model/CodedCategory.php create mode 100644 test-src/Model/MetadataRefreshCategory.php create mode 100644 test-src/Model/SchemaQualifiedCategory.php create mode 100644 test-src/Model/UntimedCategory.php diff --git a/README.md b/README.md index e699157..924a5b4 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,8 @@ The base model gives you the methods most applications reach for first: If your table includes `created_at` and `updated_at`, they are populated automatically on insert and update. +Timestamps are generated in UTC using the `Y-m-d H:i:s` format. SQLite stores those values as text, while MySQL/MariaDB and PostgreSQL accept them in timestamp-style columns. + ### Dynamic finders and counters Build expressive queries straight from method names: @@ -157,6 +159,8 @@ $statement = Freshsauce\Model\Model::execute( $rows = $statement->fetchAll(PDO::FETCH_ASSOC); ``` +If you change a table schema at runtime and need the model to see the new columns without reconnecting, call `YourModel::refreshTableMetadata()`. + ### Validation hooks Use instance-aware hooks when writes need application rules: @@ -226,6 +230,8 @@ Freshsauce\Model\Model::connectDb( SQLite is supported in the library and covered by the automated test suite. +Schema-qualified table names such as `reporting.categories` are supported for PostgreSQL models. + ## Built for real projects The repository includes: diff --git a/src/Model/Model.php b/src/Model/Model.php index 2ec38d6..c24614f 100644 --- a/src/Model/Model.php +++ b/src/Model/Model.php @@ -403,6 +403,16 @@ protected static function getFieldnames(): array return self::$_tableColumns[$class]; } + /** + * Refresh cached table metadata for the current model class. + * + * @return void + */ + public static function refreshTableMetadata(): void + { + unset(self::$_tableColumns[static::class]); + } + /** * Split a table name into schema and table, defaulting schema to public. * @@ -985,7 +995,7 @@ protected static function fetchWhereWithSuffix(string $SQLfragment = '', array $ * returns an array of objects of the sub-class which match the conditions * * @param string $SQLfragment conditions, sorting, grouping and limit to apply (to right of WHERE keywords) - * @param array $params optional params to be escaped and injected into the SQL query (standrd PDO syntax) + * @param array $params optional params to be escaped and injected into the SQL query (standard PDO syntax) * @param bool $limitOne if true the first match will be returned * * @return array|static|null @@ -999,7 +1009,7 @@ public static function fetchWhere(string $SQLfragment = '', array $params = [], * returns an array of objects of the sub-class which match the conditions * * @param string $SQLfragment conditions, sorting, grouping and limit to apply (to right of WHERE keywords) - * @param array $params optional params to be escaped and injected into the SQL query (standrd PDO syntax) + * @param array $params optional params to be escaped and injected into the SQL query (standard PDO syntax) * * @return array object[] of objects of calling class */ @@ -1014,7 +1024,7 @@ public static function fetchAllWhere(string $SQLfragment = '', array $params = [ * returns an object of the sub-class which matches the conditions * * @param string $SQLfragment conditions, sorting, grouping and limit to apply (to right of WHERE keywords) - * @param array $params optional params to be escaped and injected into the SQL query (standrd PDO syntax) + * @param array $params optional params to be escaped and injected into the SQL query (standard PDO syntax) * * @return static|null object of calling class */ @@ -1137,7 +1147,7 @@ public function delete() * Delete records based on an SQL conditions * * @param string $where SQL fragment of conditions - * @param array $params optional params to be escaped and injected into the SQL query (standrd PDO syntax) + * @param array $params optional params to be escaped and injected into the SQL query (standard PDO syntax) * * @return \PDOStatement */ @@ -1206,6 +1216,16 @@ protected function runUpdateValidation(): void $this->validateForUpdate(); } + /** + * Return a UTC timestamp string suitable for the built-in timestamp columns. + * + * @return string + */ + protected static function currentTimestamp(): string + { + return gmdate('Y-m-d H:i:s'); + } + /** * insert a row into the database table, and update the primary key field with the one generated on insert * @@ -1219,7 +1239,7 @@ protected function runUpdateValidation(): void public function insert(bool $autoTimestamp = true, bool $allowSetPrimaryKey = false): bool { $pk = static::$_primary_column_name; - $timeStr = gmdate('Y-m-d H:i:s'); + $timeStr = static::currentTimestamp(); if ($autoTimestamp && in_array('created_at', static::getFieldnames())) { $this->created_at = $timeStr; } @@ -1291,7 +1311,7 @@ public function insert(bool $autoTimestamp = true, bool $allowSetPrimaryKey = fa public function update(bool $autoTimestamp = true): bool { if ($autoTimestamp && in_array('updated_at', static::getFieldnames())) { - $this->updated_at = gmdate('Y-m-d H:i:s'); + $this->updated_at = static::currentTimestamp(); } $this->runUpdateValidation(); $set = $this->setString(); @@ -1305,10 +1325,11 @@ public function update(bool $autoTimestamp = true): bool $query, $set['params'] ); - if ($st->rowCount() == 1) { + if ($this->updateSucceeded($st)) { $this->clearDirtyFields(); + return true; } - return ($st->rowCount() == 1); + return false; } /** @@ -1417,6 +1438,25 @@ protected function hasPrimaryKeyValue(): bool return $this->$primaryKey !== null; } + /** + * Determine whether an update succeeded even when the driver reports zero changed rows. + * + * @param \PDOStatement $statement + * + * @return bool + */ + protected function updateSucceeded(\PDOStatement $statement): bool + { + if ($statement->rowCount() > 0) { + return true; + } + + return static::existsWhere( + static::_quote_identifier(static::$_primary_column_name) . ' = ?', + [$this->{static::$_primary_column_name}] + ); + } + /** * @param mixed $match * diff --git a/test-src/Model/CodedCategory.php b/test-src/Model/CodedCategory.php new file mode 100644 index 0000000..3f3212f --- /dev/null +++ b/test-src/Model/CodedCategory.php @@ -0,0 +1,16 @@ +resetSqliteSequenceIfPresent(); } } @@ -110,10 +173,12 @@ public function setUp(): void private function resetSqliteSequenceIfPresent(): void { try { - Freshsauce\Model\Model::execute( - 'DELETE FROM `' . self::SQLITE_SEQUENCE_TABLE . '` WHERE `name` = ?', - ['categories'] - ); + foreach (['categories', 'metadata_refresh_categories', 'untimed_categories'] as $tableName) { + Freshsauce\Model\Model::execute( + 'DELETE FROM `' . self::SQLITE_SEQUENCE_TABLE . '` WHERE `name` = ?', + [$tableName] + ); + } } catch (\PDOException $e) { if (strpos($e->getMessage(), 'no such table: ' . self::SQLITE_SEQUENCE_TABLE) === false) { throw $e; @@ -320,6 +385,126 @@ public function testRecordLifecycleHelpers(): void $this->assertSame(0, App\Model\Category::count()); } + public function testModelSubclassesCanShareTheInheritedConnection(): void + { + $category = $this->createCategory('Shared connection'); + + $reloaded = App\Model\LegacyValidatingCategory::getById((int) $category->id); + + $this->assertNotNull($reloaded); + $this->assertInstanceOf(App\Model\LegacyValidatingCategory::class, $reloaded); + $this->assertSame('Shared connection', $reloaded->name); + } + + public function testCustomPrimaryKeyLifecycleWorksAcrossDrivers(): void + { + $category = new App\Model\CodedCategory([ + 'code' => 42, + 'name' => 'Meaning', + ]); + + $this->assertTrue($category->insert(false, true)); + $this->assertSame(42, (int) $category->code); + + $reloaded = App\Model\CodedCategory::getById(42); + $this->assertNotNull($reloaded); + $this->assertSame('Meaning', $reloaded->name); + + $reloaded->name = 'Updated meaning'; + $this->assertTrue($reloaded->save()); + $this->assertSame('Updated meaning', App\Model\CodedCategory::getById(42)?->name); + } + + public function testMetadataRefreshAllowsNewColumnsWithoutReconnect(): void + { + $withoutRefresh = new App\Model\MetadataRefreshCategory([ + 'name' => 'Before refresh', + 'description' => 'ignored until refresh', + ]); + $this->assertFalse(isset($withoutRefresh->description)); + + if (self::$driverName === 'mysql') { + Freshsauce\Model\Model::execute( + 'ALTER TABLE `metadata_refresh_categories` ADD COLUMN `description` VARCHAR(120) NULL' + ); + } elseif (self::$driverName === 'pgsql') { + Freshsauce\Model\Model::execute( + 'ALTER TABLE "metadata_refresh_categories" ADD COLUMN "description" VARCHAR(120) NULL' + ); + } elseif (self::$driverName === 'sqlite') { + Freshsauce\Model\Model::execute( + 'ALTER TABLE `metadata_refresh_categories` ADD COLUMN `description` VARCHAR(120) NULL' + ); + } + + App\Model\MetadataRefreshCategory::refreshTableMetadata(); + + $withRefresh = new App\Model\MetadataRefreshCategory([ + 'name' => 'After refresh', + 'description' => 'captured after refresh', + ]); + $this->assertSame('captured after refresh', $withRefresh->description); + $this->assertTrue($withRefresh->save()); + $this->assertSame('captured after refresh', App\Model\MetadataRefreshCategory::getById((int) $withRefresh->id)?->description); + } + + public function testSchemaQualifiedPostgresTablesWork(): void + { + if (self::$driverName !== 'pgsql') { + $this->markTestSkipped('Schema-qualified table coverage is PostgreSQL-specific.'); + } + + $category = new App\Model\SchemaQualifiedCategory([ + 'name' => 'Namespaced category', + ]); + + $this->assertTrue($category->save()); + $reloaded = App\Model\SchemaQualifiedCategory::getById((int) $category->id); + + $this->assertNotNull($reloaded); + $this->assertSame('Namespaced category', $reloaded->name); + } + + public function testInsertAndUpdateCanOptOutOfAutomaticTimestamps(): void + { + $category = new App\Model\Category([ + 'name' => 'No automatic timestamps', + ]); + + $this->assertTrue($category->insert(false)); + $this->assertNull($category->created_at); + $this->assertNull($category->updated_at); + + $category->name = 'Still no automatic timestamps'; + $this->assertTrue($category->update(false)); + $this->assertNull($category->updated_at); + + $reloaded = App\Model\Category::getById((int) $category->id); + $this->assertNotNull($reloaded); + $this->assertNull($reloaded->created_at); + $this->assertNull($reloaded->updated_at); + } + + public function testModelsWithoutTimestampColumnsSaveNormally(): void + { + $category = new App\Model\UntimedCategory([ + 'name' => 'Untimed', + ]); + + $this->assertTrue($category->save()); + $this->assertNotNull($category->id); + $this->assertSame('Untimed', App\Model\UntimedCategory::getById((int) $category->id)?->name); + } + + public function testUpdateTreatsExistingRowAsSuccessWhenDriverReportsZeroChangedRows(): void + { + $category = $this->createCategory('No-op update'); + $category->name = 'No-op update'; + + $this->assertTrue($category->update(false)); + $this->assertFalse($category->isFieldDirty('name')); + } + public function testFocusedQueryHelpers(): void { $this->assertFalse(App\Model\Category::exists()); From d9778bcf6497e24d96faddd16f2dab82f794a0bd Mon Sep 17 00:00:00 2001 From: Dave Barnwell Date: Sun, 8 Mar 2026 09:33:45 +0000 Subject: [PATCH 2/2] Address phase 3 review comments --- src/Model/Model.php | 20 ++++++++++++++++---- tests/Model/CategoryTest.php | 3 +++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Model/Model.php b/src/Model/Model.php index c24614f..00583f8 100644 --- a/src/Model/Model.php +++ b/src/Model/Model.php @@ -1447,13 +1447,25 @@ protected function hasPrimaryKeyValue(): bool */ protected function updateSucceeded(\PDOStatement $statement): bool { - if ($statement->rowCount() > 0) { + $count = $statement->rowCount(); + + if ($count === 1) { return true; } - return static::existsWhere( - static::_quote_identifier(static::$_primary_column_name) . ' = ?', - [$this->{static::$_primary_column_name}] + if ($count === 0) { + return static::existsWhere( + static::_quote_identifier(static::$_primary_column_name) . ' = ?', + [$this->{static::$_primary_column_name}] + ); + } + + throw new ModelException( + sprintf( + 'Update affected %d rows for %s; expected at most one row.', + $count, + static::class + ) ); } diff --git a/tests/Model/CategoryTest.php b/tests/Model/CategoryTest.php index 78654f8..30f5cbd 100644 --- a/tests/Model/CategoryTest.php +++ b/tests/Model/CategoryTest.php @@ -75,14 +75,17 @@ public static function setUpBeforeClass(): void "updated_at" TIMESTAMP NULL, "created_at" TIMESTAMP NULL )', + 'DROP TABLE IF EXISTS "coded_categories"', 'CREATE TABLE "coded_categories" ( "code" INTEGER PRIMARY KEY, "name" VARCHAR(120) NULL )', + 'DROP TABLE IF EXISTS "metadata_refresh_categories"', 'CREATE TABLE "metadata_refresh_categories" ( "id" SERIAL PRIMARY KEY, "name" VARCHAR(120) NULL )', + 'DROP TABLE IF EXISTS "untimed_categories"', 'CREATE TABLE "untimed_categories" ( "id" SERIAL PRIMARY KEY, "name" VARCHAR(120) NULL