From 67d1a77e1e3d46f20907eb4bb23ec6c1e913d2bd Mon Sep 17 00:00:00 2001 From: Dave Barnwell Date: Sun, 8 Mar 2026 08:34:55 +0000 Subject: [PATCH 1/3] Implement phase 1 ORM fixes --- README.md | 10 + ROADMAP.md | 344 ++++++++++++++++++ .../Exception/ConfigurationException.php | 7 + src/Model/Exception/ConnectionException.php | 7 + .../InvalidDynamicMethodException.php | 7 + src/Model/Exception/MissingDataException.php | 7 + src/Model/Exception/ModelException.php | 7 + src/Model/Exception/UnknownFieldException.php | 7 + src/Model/Model.php | 123 ++++++- test-src/Model/SqliteCodeCategory.php | 16 + test-src/Model/SqliteStringCodeCategory.php | 16 + tests/Model/CategoryTest.php | 47 ++- tests/Model/SqliteModelTest.php | 50 +++ 13 files changed, 624 insertions(+), 24 deletions(-) create mode 100644 ROADMAP.md create mode 100644 src/Model/Exception/ConfigurationException.php create mode 100644 src/Model/Exception/ConnectionException.php create mode 100644 src/Model/Exception/InvalidDynamicMethodException.php create mode 100644 src/Model/Exception/MissingDataException.php create mode 100644 src/Model/Exception/ModelException.php create mode 100644 src/Model/Exception/UnknownFieldException.php create mode 100644 test-src/Model/SqliteCodeCategory.php create mode 100644 test-src/Model/SqliteStringCodeCategory.php diff --git a/README.md b/README.md index 91539eb..be96a41 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,15 @@ class Category extends Freshsauce\Model\Model Throw an exception from `validate()` to block invalid inserts or updates. +### Predictable exceptions + +The library now throws model-specific exceptions for common failure modes instead of generic `Exception` objects. + +- `Freshsauce\Model\Exception\ConnectionException` for missing database connections +- `Freshsauce\Model\Exception\UnknownFieldException` for invalid model fields and dynamic finder columns +- `Freshsauce\Model\Exception\InvalidDynamicMethodException` for unsupported dynamic static methods +- `Freshsauce\Model\Exception\MissingDataException` for invalid access to uninitialized model data + ## Database support MySQL or MariaDB: @@ -185,5 +194,6 @@ The repository includes: ## Learn more +- Want to see planned improvements? See [ROADMAP.md](ROADMAP.md). - Want fuller usage examples? See [EXAMPLE.md](EXAMPLE.md). - Want to contribute? See [CONTRIBUTING.md](CONTRIBUTING.md). diff --git a/ROADMAP.md b/ROADMAP.md new file mode 100644 index 0000000..040acd1 --- /dev/null +++ b/ROADMAP.md @@ -0,0 +1,344 @@ +# Roadmap + +This roadmap turns the current improvement analysis into a concrete execution plan for `freshsauce/model`. + +The sequencing is intentional: + +1. Fix correctness issues before expanding the API. +2. Improve developer ergonomics without turning the library into a heavyweight ORM. +3. Add optional features only where they preserve the package's lightweight position. + +## Principles + +- Keep PDO-first escape hatches intact. +- Prefer additive changes with low migration cost. +- Tighten behavior with tests before changing public APIs. +- Avoid feature growth that pushes the library toward framework-scale complexity. + +## Phase 1: Core correctness and safety + +Goal: remove known edge-case bugs and make failure modes explicit. + +Priority: high + +### Milestone 1.1: Serialization works correctly + +Problem: +`Model::__sleep()` returns table field names, but model state lives in `$data` and `$dirty`. Serializing a model currently emits warnings and drops state. + +Tasks: + +- Replace `__sleep()` with `__serialize()` and `__unserialize()`. +- Preserve hydrated values and dirty-state behavior after unserialization. +- Decide whether deserialized models should retain dirty flags or reset to clean. +- Add PHPUnit coverage for round-trip serialization of new and persisted models. + +Acceptance criteria: + +- `serialize($model)` produces no warnings. +- `unserialize(serialize($model))` preserves field values. +- Dirty-state behavior after round-trip is documented and tested. + +### Milestone 1.2: Persisted-state detection is explicit + +Problem: +`save()` uses truthiness on the primary key. A value like `0` is treated as "new" and triggers insert instead of update. + +Tasks: + +- Introduce a dedicated persisted-state check based on `null` rather than truthiness. +- Review insert/update/delete behavior for zero-like primary key values. +- Add tests for integer `0`, string `'0'`, and non-default primary key names. + +Acceptance criteria: + +- `save()` updates when a record has a zero-like primary key value. +- Insert behavior remains unchanged for `null` primary keys. + +### Milestone 1.3: Dynamic finder failures are model-level errors + +Problem: +Unknown dynamic fields fall through to raw SQL execution and surface as PDO errors instead of clear model exceptions. + +Tasks: + +- Make `resolveFieldName()` fail fast when a field does not map to a real column. +- Add a dedicated exception for unknown fields or invalid dynamic methods. +- Add tests for invalid `findBy...`, `findOneBy...`, and `countBy...` calls. + +Acceptance criteria: + +- Invalid dynamic finders throw a predictable library exception before query execution. +- Error messages identify the requested field and model class. + +### Milestone 1.4: Empty-array query behavior is defined + +Problem: +Helpers that build `IN (...)` clauses do not define behavior for empty arrays. + +Tasks: + +- Define expected behavior for empty-array matches across: + - `findBy...` + - `findOneBy...` + - `firstBy...` + - `lastBy...` + - `countBy...` + - `fetchAllWhereMatchingSingleField()` + - `fetchOneWhereMatchingSingleField()` +- Implement that behavior without generating invalid SQL. +- Add regression tests for each public entry point. + +Acceptance criteria: + +- Empty arrays never produce invalid SQL. +- Collection methods return empty results. +- Singular methods return `null`. +- Count methods return `0`. + +### Milestone 1.5: Replace generic exceptions with library exceptions + +Problem: +The model currently throws generic `\Exception` in many places, which makes calling code and tests less precise. + +Tasks: + +- Introduce a small exception hierarchy under `Freshsauce\Model\Exception\`. +- Replace generic throws for: + - missing connection + - unknown field + - invalid dynamic method + - missing data access + - identifier quoting setup failures +- Keep exception names narrow and practical. + +Acceptance criteria: + +- Core failure modes throw specific exception classes. +- Existing messages remain readable. +- Public docs mention the main exception types users should expect. + +## Phase 2: API ergonomics and typing + +Goal: make the library easier to use correctly while keeping the current lightweight style. + +Priority: medium + +### Milestone 2.1: Validation becomes instance-aware + +Problem: +`validate()` is declared static, but it is invoked from instance writes. That makes record-aware validation awkward. + +Tasks: + +- Change validation to an instance hook, or introduce `validateForInsert()` and `validateForUpdate()` instance hooks. +- Preserve backward compatibility where practical, or provide a clear migration path. +- Add tests covering validation against current field values. + +Acceptance criteria: + +- Validation can inspect instance state directly. +- Validation behavior for insert vs update is documented. + +### Milestone 2.2: Strict field handling is available + +Problem: +Unknown fields can be assigned silently via `__set()`, but are ignored during persistence. That hides typos. + +Tasks: + +- Add an optional strict mode that rejects assignment to unknown fields. +- Decide whether strict mode is global, per model, or opt-in at runtime. +- Keep the current permissive mode available for legacy integrations. +- Add tests for strict and permissive behavior. + +Acceptance criteria: + +- Strict mode raises a clear exception on unknown field assignment. +- Default behavior remains stable unless the user opts in. + +### Milestone 2.3: Add focused query helpers + +Problem: +Many common query patterns require manual SQL fragments, which works but is unnecessarily error-prone. + +Tasks: + +- Add a small set of helpers with clear value: + - `exists()` + - `existsWhere()` + - `pluck(string $field, ... )` + - `orderBy(string $field, string $direction)` via new fetch helpers, not a full query builder + - `limit(int $n)` support in helper methods where it keeps the API simple +- Avoid introducing a chainable query-builder unless a later need is proven. + +Acceptance criteria: + +- Common "check existence", "single column list", and "ordered fetch" cases need less handwritten SQL. +- The API remains smaller than a full query-builder abstraction. + +### Milestone 2.4: Tighten types and static analysis + +Problem: +The library runs on PHP 8.3+ but still carries loose typing in several public and protected methods. + +Tasks: + +- Add `declare(strict_types=1);` to source and tests. +- Add explicit parameter and return types where missing. +- Improve PHPDoc for dynamic methods and arrays. +- Reduce `phpstan.neon` ignores where feasible. + +Acceptance criteria: + +- PHPStan remains green at the current level. +- Public APIs are more explicit and easier to consume from IDEs. + +### Milestone 2.5: Refresh documentation around modern usage + +Problem: +The docs still present deprecated snake_case dynamic methods in examples and do not explain newer behavior clearly enough. + +Tasks: + +- Update `README.md` and `EXAMPLE.md` to lead with camelCase dynamic methods only. +- Add migration notes for deprecated snake_case methods. +- Document strict mode, validation hooks, and exception behavior once shipped. +- Add a short "when to use this library" section to reinforce scope boundaries. + +Acceptance criteria: + +- Public docs reflect the preferred API. +- Deprecated behavior is documented as transitional, not primary. + +## Phase 3: Quality, portability, and maintenance + +Goal: make the library easier to maintain and safer across supported databases. + +Priority: medium + +### Milestone 3.1: Expand edge-case test coverage + +Tasks: + +- Add regression tests for every Phase 1 fix. +- Add tests for multiple model subclasses sharing and isolating connections. +- Add tests for schema-qualified PostgreSQL table names. +- Add tests for custom primary key column names. +- Add tests for timestamp opt-out behavior. + +Acceptance criteria: + +- New fixes are guarded by tests. +- Cross-driver behavior is better documented in test form. + +### Milestone 3.2: Review statement and metadata caching behavior + +Problem: +Statement caching is now keyed by connection, which is good, but metadata and caching behavior should stay predictable as the library grows. + +Tasks: + +- Audit cache invalidation rules for reconnection and subclass-specific connections. +- Decide whether table column metadata should be refreshable without reconnecting. +- Add tests around reconnect and metadata refresh behavior. + +Acceptance criteria: + +- Reconnection cannot leak stale prepared statements. +- Metadata caching behavior is documented and tested. + +### Milestone 3.3: Normalize exception and timestamp behavior across drivers + +Tasks: + +- Verify `rowCount()` assumptions for update/delete across supported drivers. +- Review timestamp formatting consistency for MySQL, PostgreSQL, and SQLite. +- Ensure identifier quoting and schema discovery stay correct for each supported driver. + +Acceptance criteria: + +- Driver-specific behavior is explicit where unavoidable. +- Public docs do not imply unsupported guarantees. + +## Phase 4: Optional feature expansion + +Goal: add features that help real applications, but only if they fit the package's lightweight position. + +Priority: lower + +These should only start after Phases 1 and 2 are complete. + +### Candidate 4.1: Transaction helpers + +Possible scope: + +- `transaction(callable $callback)` +- pass through `beginTransaction()`, `commit()`, `rollBack()` wrappers + +Why: +This adds practical value without changing the core model shape. + +### Candidate 4.2: Configurable timestamp columns + +Possible scope: + +- opt-in timestamp column names +- disable automatic timestamps per model + +Why: +The current `created_at` / `updated_at` convention is convenient but rigid. + +### Candidate 4.3: Attribute casting + +Possible scope: + +- integer +- float +- boolean +- datetime +- JSON array/object + +Why: +Casting improves ergonomics substantially without requiring relationships or a large query layer. + +### Candidate 4.4: Composite keys or relationship support + +Why this is last: +This is where complexity rises sharply. It should only happen if the maintainer wants the library to move beyond lightweight active-record usage. + +Recommendation: + +- Do not start here by default. +- Re-evaluate only after the earlier phases have shipped and real user demand is clear. + +## Suggested issue order + +If this work is split into GitHub issues, the most practical order is: + +1. Replace `__sleep()` with proper serialization support. +2. Fix zero-like primary key handling in `save()`. +3. Make unknown dynamic fields fail fast. +4. Define empty-array query behavior. +5. Introduce library exception classes. +6. Expand regression tests for the above. +7. Convert validation to instance-aware hooks. +8. Add strict field mode. +9. Add focused query helpers. +10. Tighten typing and refresh docs. + +## Suggested release strategy + +- Release 1: all Phase 1 fixes plus regression tests. +- Release 2: validation, strict mode, typed API cleanup, and documentation refresh. +- Release 3: selected optional helpers such as transactions, timestamp configuration, and casting. + +## Out of scope unless demand changes + +- Full relationship mapping +- Eager/lazy loading systems +- A chainable query builder comparable to framework ORMs +- Migration tooling +- Schema management + +Those features would change the character of the package more than they would improve the current design. diff --git a/src/Model/Exception/ConfigurationException.php b/src/Model/Exception/ConfigurationException.php new file mode 100644 index 0000000..8c2478f --- /dev/null +++ b/src/Model/Exception/ConfigurationException.php @@ -0,0 +1,7 @@ +hasData()) { - throw new \Exception('No data'); + throw new MissingDataException('No data'); } return true; @@ -175,18 +181,19 @@ public function clearDirtyFields(): void */ public function __get($name) { - if (!$this->hasData()) { - throw new \Exception("data property=$name has not been initialised", 1); + $data = $this->data; + if (!$data instanceof \stdClass) { + throw new MissingDataException("data property=$name has not been initialised", 1); } - if (property_exists($this->data, $name)) { - return $this->data->$name; + if (property_exists($data, $name)) { + return $data->$name; } $trace = debug_backtrace(); $file = $trace[0]['file'] ?? 'unknown'; $line = $trace[0]['line'] ?? 0; - throw new \Exception( + throw new UnknownFieldException( 'Undefined property via __get(): ' . $name . ' in ' . $file . ' on line ' . $line, @@ -204,7 +211,8 @@ public function __get($name) */ public function __isset($name) { - if ($this->hasData() && property_exists($this->data, $name)) { + $data = $this->data; + if ($data instanceof \stdClass && property_exists($data, $name)) { return true; } @@ -285,11 +293,11 @@ protected static function getDriverName(): string { $db = static::$_db; if (!$db) { - throw new \Exception('No database connection setup'); + throw new ConnectionException('No database connection setup'); } $driver = $db->getAttribute(\PDO::ATTR_DRIVER_NAME); if (!is_string($driver)) { - throw new \Exception('Unable to determine database driver'); + throw new ConfigurationException('Unable to determine database driver'); } return $driver; } @@ -344,7 +352,7 @@ protected static function _quote_identifier_part(string $part): string static::_setup_identifier_quote_character(); $quote = static::$_identifier_quote_character; if ($quote === null) { - throw new \Exception('Identifier quote character not set'); + throw new ConfigurationException('Identifier quote character not set'); } $escaped = str_replace($quote, $quote . $quote, $part); return $quote . $escaped . $quote; @@ -436,7 +444,29 @@ public function clear(): void */ public function __sleep() { - return static::getFieldnames(); + return array('data', 'dirty'); + } + + /** + * @return array{data: \stdClass, dirty: \stdClass} + */ + public function __serialize(): array + { + return array( + 'data' => $this->normaliseSerializedState(isset($this->data) ? $this->data : null), + 'dirty' => $this->normaliseSerializedState(isset($this->dirty) ? $this->dirty : null), + ); + } + + /** + * @param array{data?: mixed, dirty?: mixed} $data + * + * @return void + */ + public function __unserialize(array $data): void + { + $this->data = $this->normaliseSerializedState($data['data'] ?? null); + $this->dirty = $this->normaliseSerializedState($data['dirty'] ?? null); } /** @@ -520,7 +550,7 @@ public static function __callStatic($name, $arguments) } return static::dispatchDynamicStaticMethod($dynamicMethod['operation'], $dynamicMethod['fieldname'], $match); } - throw new \Exception(__CLASS__ . ' not such static method[' . $name . ']'); + throw new InvalidDynamicMethodException(__CLASS__ . ' not such static method[' . $name . ']'); } /** @@ -669,7 +699,7 @@ protected static function resolveFieldName($fieldname) return $field; } } - return $fieldname; + throw new UnknownFieldException('Unknown field [' . $fieldname . '] for model ' . static::class); } /** @@ -710,6 +740,9 @@ protected static function snakeToStudly(string $fieldname): string */ protected static function countByField($fieldname, $match) { + if (static::isEmptyMatchList($match)) { + return 0; + } if (is_array($match)) { return static::countAllWhere(static::_quote_identifier($fieldname) . ' IN (' . static::createInClausePlaceholders($match) . ')', $match); } @@ -727,6 +760,9 @@ protected static function countByField($fieldname, $match) */ public static function fetchOneWhereMatchingSingleField($fieldname, $match, $order) { + if (static::isEmptyMatchList($match)) { + return null; + } if (is_array($match)) { return static::fetchOneWhere(static::_quote_identifier($fieldname) . ' IN (' . static::createInClausePlaceholders($match) . ') ORDER BY ' . static::_quote_identifier($fieldname) . ' ' . $order, $match); } else { @@ -745,6 +781,9 @@ public static function fetchOneWhereMatchingSingleField($fieldname, $match, $ord */ public static function fetchAllWhereMatchingSingleField($fieldname, $match) { + if (static::isEmptyMatchList($match)) { + return array(); + } if (is_array($match)) { return static::fetchAllWhere(static::_quote_identifier($fieldname) . ' IN (' . static::createInClausePlaceholders($match) . ')', $match); } else { @@ -761,6 +800,9 @@ public static function fetchAllWhereMatchingSingleField($fieldname, $match) */ public static function createInClausePlaceholders($params) { + if (count($params) === 0) { + return 'NULL'; + } return implode(',', array_fill(0, count($params), '?')); } @@ -987,7 +1029,7 @@ public function insert($autoTimestamp = true, $allowSetPrimaryKey = false) if ($allowSetPrimaryKey !== true) { $db = static::$_db; if (!$db) { - throw new \Exception('No database connection setup'); + throw new ConnectionException('No database connection setup'); } $this->{static::$_primary_column_name} = $db->lastInsertId(); } @@ -1054,7 +1096,7 @@ protected static function _prepare($query) { $db = static::$_db; if (!$db) { - throw new \Exception('No database connection setup'); + throw new ConnectionException('No database connection setup'); } $connectionId = spl_object_id($db); if (!isset(static::$_stmt[$connectionId])) { @@ -1074,13 +1116,54 @@ protected static function _prepare($query) */ public function save() { - if ($this->{static::$_primary_column_name}) { + if ($this->hasPrimaryKeyValue()) { return $this->update(); } else { return $this->insert(); } } + /** + * @param mixed $state + * + * @return \stdClass + */ + protected function normaliseSerializedState($state): \stdClass + { + if ($state instanceof \stdClass) { + return $state; + } + if (is_array($state)) { + return (object) $state; + } + + return new \stdClass(); + } + + /** + * @return bool + */ + protected function hasPrimaryKeyValue(): bool + { + $primaryKey = static::$_primary_column_name; + $data = $this->data; + if (!$data instanceof \stdClass || !property_exists($data, $primaryKey)) { + return false; + } + + return $this->$primaryKey !== null; + } + + /** + * @param mixed $match + * + * @return bool + */ + protected static function isEmptyMatchList($match): bool + { + return is_array($match) && $match === array(); + } + /** * Create an SQL fragment to be used after the SET keyword in an SQL UPDATE * escaping parameters as necessary. @@ -1109,12 +1192,12 @@ protected function setString($ignorePrimary = true) * @var array $values placeholder list for INSERT */ $values = []; - $hasData = $this->hasData(); + $data = $this->data; foreach (static::getFieldnames() as $field) { if ($ignorePrimary && $field == static::$_primary_column_name) { continue; } - if (!$hasData || !property_exists($this->data, $field) || !$this->isFieldDirty($field)) { + if (!$data instanceof \stdClass || !property_exists($data, $field) || !$this->isFieldDirty($field)) { continue; } $columns[] = static::_quote_identifier($field); diff --git a/test-src/Model/SqliteCodeCategory.php b/test-src/Model/SqliteCodeCategory.php new file mode 100644 index 0000000..b419f0b --- /dev/null +++ b/test-src/Model/SqliteCodeCategory.php @@ -0,0 +1,16 @@ +toArray(); $this->assertSame('History', $data['name']); - $this->assertSame(['id', 'name', 'updated_at', 'created_at'], $category->__sleep()); + $this->assertSame(['data', 'dirty'], $category->__sleep()); $category->clear(); $this->assertNull($category->name); $this->assertFalse($category->isFieldDirty('name')); } + public function testSerializationRoundTripPreservesValuesAndDirtyState(): void + { + $category = new App\Model\Category([ + 'name' => 'Serialized category', + ]); + $category->clearDirtyFields(); + $category->name = 'Serialized category updated'; + + $restored = unserialize(serialize($category)); + + $this->assertInstanceOf(App\Model\Category::class, $restored); + $this->assertSame('Serialized category updated', $restored->name); + $this->assertTrue($restored->isFieldDirty('name')); + } + public function testMagicGetThrowsForMissingDataAndUnknownField(): void { $reflection = new ReflectionClass(App\Model\Category::class); @@ -254,13 +272,13 @@ public function testMagicGetThrowsForMissingDataAndUnknownField(): void try { $categoryWithoutConstructor->dataPresent(); $this->fail('Expected missing data exception.'); - } catch (Exception $exception) { + } catch (MissingDataException $exception) { $this->assertSame('No data', $exception->getMessage()); } $category = new App\Model\Category(); - $this->expectException(Exception::class); + $this->expectException(UnknownFieldException::class); $this->expectExceptionMessage('Undefined property via __get(): unknown_field'); $category->__get('unknown_field'); } @@ -401,12 +419,33 @@ public function testUtilityHelpers(): void public function testUnknownDynamicMethodThrows(): void { - $this->expectException(Exception::class); + $this->expectException(InvalidDynamicMethodException::class); $this->expectExceptionMessage('Freshsauce\Model\Model not such static method[doesNotExist]'); App\Model\Category::__callStatic('doesNotExist', ['value']); } + public function testDynamicFindersRejectUnknownFieldsBeforeRunningSql(): void + { + $this->expectException(UnknownFieldException::class); + $this->expectExceptionMessage('Unknown field [DoesNotExist] for model App\Model\Category'); + + App\Model\Category::__callStatic('findByDoesNotExist', ['value']); + } + + public function testDynamicFindersHandleEmptyMatchArrays(): void + { + $this->createCategory('Empty array control'); + + $this->assertSame([], App\Model\Category::findByName([])); + $this->assertNull(App\Model\Category::findOneByName([])); + $this->assertNull(App\Model\Category::firstByName([])); + $this->assertNull(App\Model\Category::lastByName([])); + $this->assertSame(0, App\Model\Category::countByName([])); + $this->assertSame([], App\Model\Category::fetchAllWhereMatchingSingleField('name', [])); + $this->assertNull(App\Model\Category::fetchOneWhereMatchingSingleField('name', [], 'ASC')); + } + private function captureUserDeprecation(string $expectedMessage, callable $callback): mixed { $result = null; diff --git a/tests/Model/SqliteModelTest.php b/tests/Model/SqliteModelTest.php index 0da5148..00a815e 100644 --- a/tests/Model/SqliteModelTest.php +++ b/tests/Model/SqliteModelTest.php @@ -20,11 +20,27 @@ public static function setUpBeforeClass(): void created_at TEXT NULL )' ); + App\Model\SqliteCodeCategory::connectDb('sqlite::memory:', '', ''); + App\Model\SqliteCodeCategory::execute( + 'CREATE TABLE code_categories ( + code INTEGER PRIMARY KEY, + name TEXT NULL + )' + ); + App\Model\SqliteStringCodeCategory::connectDb('sqlite::memory:', '', ''); + App\Model\SqliteStringCodeCategory::execute( + 'CREATE TABLE string_code_categories ( + code TEXT PRIMARY KEY, + name TEXT NULL + )' + ); } protected function setUp(): void { App\Model\SqliteCategory::execute('DELETE FROM `categories`'); + App\Model\SqliteCodeCategory::execute('DELETE FROM `code_categories`'); + App\Model\SqliteStringCodeCategory::execute('DELETE FROM `string_code_categories`'); $this->resetSqliteSequenceIfPresent(); } @@ -90,4 +106,38 @@ public function testPreparedStatementsStayBoundToTheirOwnConnection(): void $this->assertSame('from-a', $fromA->name); $this->assertSame('from-b', $fromB->name); } + + public function testSaveUpdatesWhenCustomIntegerPrimaryKeyIsZero(): void + { + App\Model\SqliteCodeCategory::execute( + 'INSERT INTO code_categories (code, name) VALUES (?, ?)', + [0, 'before'] + ); + + $category = new App\Model\SqliteCodeCategory([ + 'code' => 0, + 'name' => 'after', + ]); + + $this->assertTrue($category->save()); + $this->assertSame(1, App\Model\SqliteCodeCategory::count()); + $this->assertSame('after', App\Model\SqliteCodeCategory::fetchOneWhere('code = ?', [0])?->name); + } + + public function testSaveUpdatesWhenCustomStringPrimaryKeyIsZeroLike(): void + { + App\Model\SqliteStringCodeCategory::execute( + 'INSERT INTO string_code_categories (code, name) VALUES (?, ?)', + ['0', 'before'] + ); + + $category = new App\Model\SqliteStringCodeCategory([ + 'code' => '0', + 'name' => 'after', + ]); + + $this->assertTrue($category->save()); + $this->assertSame(1, App\Model\SqliteStringCodeCategory::count()); + $this->assertSame('after', App\Model\SqliteStringCodeCategory::fetchOneWhere('code = ?', ['0'])?->name); + } } From 9408c13acfb4ccccdd58411708cdeff02ce7b362 Mon Sep 17 00:00:00 2001 From: Dave Barnwell Date: Sun, 8 Mar 2026 08:44:41 +0000 Subject: [PATCH 2/3] Update exception docblocks --- src/Model/Model.php | 55 +++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/src/Model/Model.php b/src/Model/Model.php index 36ab6ab..b1800ea 100644 --- a/src/Model/Model.php +++ b/src/Model/Model.php @@ -6,6 +6,7 @@ use Freshsauce\Model\Exception\ConnectionException; use Freshsauce\Model\Exception\InvalidDynamicMethodException; use Freshsauce\Model\Exception\MissingDataException; +use Freshsauce\Model\Exception\ModelException; use Freshsauce\Model\Exception\UnknownFieldException; /** @@ -111,7 +112,7 @@ public function hasData() * Returns true if data present else throws an Exception * * @return bool - * @throws \Exception + * @throws MissingDataException */ public function dataPresent() { @@ -177,7 +178,8 @@ public function clearDirtyFields(): void * @param string $name * * @return mixed - * @throws \Exception + * @throws MissingDataException + * @throws UnknownFieldException */ public function __get($name) { @@ -230,7 +232,8 @@ public function __isset($name) * @param string $password * @param array $driverOptions * - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public static function connectDb(string $dsn, string $username, string $password, array $driverOptions = array()): void { @@ -250,7 +253,7 @@ public static function connectDb(string $dsn, string $username, string $password * (table names, column names etc). * * @return void - * @throws \Exception + * @throws ModelException */ public static function _setup_identifier_quote_character(): void { @@ -264,7 +267,7 @@ public static function _setup_identifier_quote_character(): void * names, column names etc) by looking at the driver being used by PDO. * * @return string - * @throws \Exception + * @throws ModelException */ protected static function _detect_identifier_quote_character(): string { @@ -287,7 +290,8 @@ protected static function _detect_identifier_quote_character(): string * return the driver name for the current database connection * * @return string - * @throws \Exception + * @throws ConnectionException + * @throws ConfigurationException */ protected static function getDriverName(): string { @@ -306,7 +310,8 @@ protected static function getDriverName(): string * Public accessor for the current PDO driver name. * * @return string - * @throws \Exception + * @throws ConnectionException + * @throws ConfigurationException */ public static function driverName(): string { @@ -342,7 +347,7 @@ protected static function _quote_identifier($identifier): string * @param string $part * * @return string - * @throws \Exception + * @throws ModelException */ protected static function _quote_identifier_part(string $part): string { @@ -362,7 +367,8 @@ protected static function _quote_identifier_part(string $part): string * Get and cache on the first call the column names associated with the current table * * @return array of column names for the current table - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ protected static function getFieldnames(): array { @@ -411,7 +417,8 @@ protected static function splitTableName(string $tableName): array * @param array $data * * @return void - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public function hydrate(array $data): void { @@ -428,7 +435,8 @@ public function hydrate(array $data): void * set all members to null that are associated with table columns * * @return void - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public function clear(): void { @@ -440,7 +448,8 @@ public function clear(): void /** * @return array - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public function __sleep() { @@ -471,7 +480,8 @@ public function __unserialize(array $data): void /** * @return array - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public function toArray() { @@ -538,7 +548,9 @@ public static function find($id) * @param array $arguments * * @return mixed int|object[]|object - * @throws \Exception + * @throws InvalidDynamicMethodException + * @throws \PDOException + * @throws ModelException */ public static function __callStatic($name, $arguments) { @@ -615,7 +627,9 @@ protected static function parseDynamicStaticMethod(string $name): ?array * @param mixed $match * * @return mixed - * @throws \Exception + * @throws InvalidDynamicMethodException + * @throws \PDOException + * @throws ModelException */ protected static function dispatchDynamicStaticMethod(string $operation, string $fieldname, $match) { @@ -677,7 +691,9 @@ protected static function snakeCaseDynamicMethodToCamelCase(string $name): strin * @param string $fieldname * * @return string - * @throws \Exception + * @throws UnknownFieldException + * @throws \PDOException + * @throws ModelException */ protected static function resolveFieldName($fieldname) { @@ -971,7 +987,8 @@ public static function validate() * @param boolean $allowSetPrimaryKey if true include primary key field in insert (ie. you want to set it yourself) * * @return boolean indicating success - * @throws \Exception + * @throws \PDOException + * @throws ModelException */ public function insert($autoTimestamp = true, $allowSetPrimaryKey = false) { @@ -1225,7 +1242,7 @@ protected function setString($ignorePrimary = true) * Determine whether the driver supports LIMIT on UPDATE. * * @return bool - * @throws \Exception + * @throws ConnectionException */ protected static function supportsUpdateLimit() { @@ -1237,7 +1254,7 @@ protected static function supportsUpdateLimit() * Determine whether the driver supports LIMIT on DELETE. * * @return bool - * @throws \Exception + * @throws ConnectionException */ protected static function supportsDeleteLimit() { From 6fcfb3190e713f45970feacb72b1b9de2d951b49 Mon Sep 17 00:00:00 2001 From: Dave Barnwell Date: Sun, 8 Mar 2026 08:52:06 +0000 Subject: [PATCH 3/3] Fix review follow-ups in model docs --- src/Model/Model.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Model/Model.php b/src/Model/Model.php index b1800ea..3ddbad0 100644 --- a/src/Model/Model.php +++ b/src/Model/Model.php @@ -109,7 +109,7 @@ public function hasData() /** - * Returns true if data present else throws an Exception + * Returns true if data is present else throws MissingDataException * * @return bool * @throws MissingDataException @@ -448,8 +448,6 @@ public function clear(): void /** * @return array - * @throws \PDOException - * @throws ModelException */ public function __sleep() { @@ -641,7 +639,7 @@ protected static function dispatchDynamicStaticMethod(string $operation, string 'first' => static::fetchOneWhereMatchingSingleField($resolvedFieldname, $match, 'ASC'), 'last' => static::fetchOneWhereMatchingSingleField($resolvedFieldname, $match, 'DESC'), 'count' => static::countByField($resolvedFieldname, $match), - default => throw new \Exception(static::class . ' not such static method operation[' . $operation . ']'), + default => throw new InvalidDynamicMethodException(static::class . ' not such static method operation[' . $operation . ']'), }; }