Skip to content

Commit 51936a1

Browse files
authored
fix: implement phase 1 ORM fixes (#23)
* Implement phase 1 ORM fixes * Update exception docblocks * Fix review follow-ups in model docs
1 parent 3879412 commit 51936a1

13 files changed

Lines changed: 660 additions & 45 deletions

README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,15 @@ class Category extends Freshsauce\Model\Model
149149

150150
Throw an exception from `validate()` to block invalid inserts or updates.
151151

152+
### Predictable exceptions
153+
154+
The library now throws model-specific exceptions for common failure modes instead of generic `Exception` objects.
155+
156+
- `Freshsauce\Model\Exception\ConnectionException` for missing database connections
157+
- `Freshsauce\Model\Exception\UnknownFieldException` for invalid model fields and dynamic finder columns
158+
- `Freshsauce\Model\Exception\InvalidDynamicMethodException` for unsupported dynamic static methods
159+
- `Freshsauce\Model\Exception\MissingDataException` for invalid access to uninitialized model data
160+
152161
## Database support
153162

154163
MySQL or MariaDB:
@@ -185,5 +194,6 @@ The repository includes:
185194

186195
## Learn more
187196

197+
- Want to see planned improvements? See [ROADMAP.md](ROADMAP.md).
188198
- Want fuller usage examples? See [EXAMPLE.md](EXAMPLE.md).
189199
- Want to contribute? See [CONTRIBUTING.md](CONTRIBUTING.md).

ROADMAP.md

Lines changed: 344 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,344 @@
1+
# Roadmap
2+
3+
This roadmap turns the current improvement analysis into a concrete execution plan for `freshsauce/model`.
4+
5+
The sequencing is intentional:
6+
7+
1. Fix correctness issues before expanding the API.
8+
2. Improve developer ergonomics without turning the library into a heavyweight ORM.
9+
3. Add optional features only where they preserve the package's lightweight position.
10+
11+
## Principles
12+
13+
- Keep PDO-first escape hatches intact.
14+
- Prefer additive changes with low migration cost.
15+
- Tighten behavior with tests before changing public APIs.
16+
- Avoid feature growth that pushes the library toward framework-scale complexity.
17+
18+
## Phase 1: Core correctness and safety
19+
20+
Goal: remove known edge-case bugs and make failure modes explicit.
21+
22+
Priority: high
23+
24+
### Milestone 1.1: Serialization works correctly
25+
26+
Problem:
27+
`Model::__sleep()` returns table field names, but model state lives in `$data` and `$dirty`. Serializing a model currently emits warnings and drops state.
28+
29+
Tasks:
30+
31+
- Replace `__sleep()` with `__serialize()` and `__unserialize()`.
32+
- Preserve hydrated values and dirty-state behavior after unserialization.
33+
- Decide whether deserialized models should retain dirty flags or reset to clean.
34+
- Add PHPUnit coverage for round-trip serialization of new and persisted models.
35+
36+
Acceptance criteria:
37+
38+
- `serialize($model)` produces no warnings.
39+
- `unserialize(serialize($model))` preserves field values.
40+
- Dirty-state behavior after round-trip is documented and tested.
41+
42+
### Milestone 1.2: Persisted-state detection is explicit
43+
44+
Problem:
45+
`save()` uses truthiness on the primary key. A value like `0` is treated as "new" and triggers insert instead of update.
46+
47+
Tasks:
48+
49+
- Introduce a dedicated persisted-state check based on `null` rather than truthiness.
50+
- Review insert/update/delete behavior for zero-like primary key values.
51+
- Add tests for integer `0`, string `'0'`, and non-default primary key names.
52+
53+
Acceptance criteria:
54+
55+
- `save()` updates when a record has a zero-like primary key value.
56+
- Insert behavior remains unchanged for `null` primary keys.
57+
58+
### Milestone 1.3: Dynamic finder failures are model-level errors
59+
60+
Problem:
61+
Unknown dynamic fields fall through to raw SQL execution and surface as PDO errors instead of clear model exceptions.
62+
63+
Tasks:
64+
65+
- Make `resolveFieldName()` fail fast when a field does not map to a real column.
66+
- Add a dedicated exception for unknown fields or invalid dynamic methods.
67+
- Add tests for invalid `findBy...`, `findOneBy...`, and `countBy...` calls.
68+
69+
Acceptance criteria:
70+
71+
- Invalid dynamic finders throw a predictable library exception before query execution.
72+
- Error messages identify the requested field and model class.
73+
74+
### Milestone 1.4: Empty-array query behavior is defined
75+
76+
Problem:
77+
Helpers that build `IN (...)` clauses do not define behavior for empty arrays.
78+
79+
Tasks:
80+
81+
- Define expected behavior for empty-array matches across:
82+
- `findBy...`
83+
- `findOneBy...`
84+
- `firstBy...`
85+
- `lastBy...`
86+
- `countBy...`
87+
- `fetchAllWhereMatchingSingleField()`
88+
- `fetchOneWhereMatchingSingleField()`
89+
- Implement that behavior without generating invalid SQL.
90+
- Add regression tests for each public entry point.
91+
92+
Acceptance criteria:
93+
94+
- Empty arrays never produce invalid SQL.
95+
- Collection methods return empty results.
96+
- Singular methods return `null`.
97+
- Count methods return `0`.
98+
99+
### Milestone 1.5: Replace generic exceptions with library exceptions
100+
101+
Problem:
102+
The model currently throws generic `\Exception` in many places, which makes calling code and tests less precise.
103+
104+
Tasks:
105+
106+
- Introduce a small exception hierarchy under `Freshsauce\Model\Exception\`.
107+
- Replace generic throws for:
108+
- missing connection
109+
- unknown field
110+
- invalid dynamic method
111+
- missing data access
112+
- identifier quoting setup failures
113+
- Keep exception names narrow and practical.
114+
115+
Acceptance criteria:
116+
117+
- Core failure modes throw specific exception classes.
118+
- Existing messages remain readable.
119+
- Public docs mention the main exception types users should expect.
120+
121+
## Phase 2: API ergonomics and typing
122+
123+
Goal: make the library easier to use correctly while keeping the current lightweight style.
124+
125+
Priority: medium
126+
127+
### Milestone 2.1: Validation becomes instance-aware
128+
129+
Problem:
130+
`validate()` is declared static, but it is invoked from instance writes. That makes record-aware validation awkward.
131+
132+
Tasks:
133+
134+
- Change validation to an instance hook, or introduce `validateForInsert()` and `validateForUpdate()` instance hooks.
135+
- Preserve backward compatibility where practical, or provide a clear migration path.
136+
- Add tests covering validation against current field values.
137+
138+
Acceptance criteria:
139+
140+
- Validation can inspect instance state directly.
141+
- Validation behavior for insert vs update is documented.
142+
143+
### Milestone 2.2: Strict field handling is available
144+
145+
Problem:
146+
Unknown fields can be assigned silently via `__set()`, but are ignored during persistence. That hides typos.
147+
148+
Tasks:
149+
150+
- Add an optional strict mode that rejects assignment to unknown fields.
151+
- Decide whether strict mode is global, per model, or opt-in at runtime.
152+
- Keep the current permissive mode available for legacy integrations.
153+
- Add tests for strict and permissive behavior.
154+
155+
Acceptance criteria:
156+
157+
- Strict mode raises a clear exception on unknown field assignment.
158+
- Default behavior remains stable unless the user opts in.
159+
160+
### Milestone 2.3: Add focused query helpers
161+
162+
Problem:
163+
Many common query patterns require manual SQL fragments, which works but is unnecessarily error-prone.
164+
165+
Tasks:
166+
167+
- Add a small set of helpers with clear value:
168+
- `exists()`
169+
- `existsWhere()`
170+
- `pluck(string $field, ... )`
171+
- `orderBy(string $field, string $direction)` via new fetch helpers, not a full query builder
172+
- `limit(int $n)` support in helper methods where it keeps the API simple
173+
- Avoid introducing a chainable query-builder unless a later need is proven.
174+
175+
Acceptance criteria:
176+
177+
- Common "check existence", "single column list", and "ordered fetch" cases need less handwritten SQL.
178+
- The API remains smaller than a full query-builder abstraction.
179+
180+
### Milestone 2.4: Tighten types and static analysis
181+
182+
Problem:
183+
The library runs on PHP 8.3+ but still carries loose typing in several public and protected methods.
184+
185+
Tasks:
186+
187+
- Add `declare(strict_types=1);` to source and tests.
188+
- Add explicit parameter and return types where missing.
189+
- Improve PHPDoc for dynamic methods and arrays.
190+
- Reduce `phpstan.neon` ignores where feasible.
191+
192+
Acceptance criteria:
193+
194+
- PHPStan remains green at the current level.
195+
- Public APIs are more explicit and easier to consume from IDEs.
196+
197+
### Milestone 2.5: Refresh documentation around modern usage
198+
199+
Problem:
200+
The docs still present deprecated snake_case dynamic methods in examples and do not explain newer behavior clearly enough.
201+
202+
Tasks:
203+
204+
- Update `README.md` and `EXAMPLE.md` to lead with camelCase dynamic methods only.
205+
- Add migration notes for deprecated snake_case methods.
206+
- Document strict mode, validation hooks, and exception behavior once shipped.
207+
- Add a short "when to use this library" section to reinforce scope boundaries.
208+
209+
Acceptance criteria:
210+
211+
- Public docs reflect the preferred API.
212+
- Deprecated behavior is documented as transitional, not primary.
213+
214+
## Phase 3: Quality, portability, and maintenance
215+
216+
Goal: make the library easier to maintain and safer across supported databases.
217+
218+
Priority: medium
219+
220+
### Milestone 3.1: Expand edge-case test coverage
221+
222+
Tasks:
223+
224+
- Add regression tests for every Phase 1 fix.
225+
- Add tests for multiple model subclasses sharing and isolating connections.
226+
- Add tests for schema-qualified PostgreSQL table names.
227+
- Add tests for custom primary key column names.
228+
- Add tests for timestamp opt-out behavior.
229+
230+
Acceptance criteria:
231+
232+
- New fixes are guarded by tests.
233+
- Cross-driver behavior is better documented in test form.
234+
235+
### Milestone 3.2: Review statement and metadata caching behavior
236+
237+
Problem:
238+
Statement caching is now keyed by connection, which is good, but metadata and caching behavior should stay predictable as the library grows.
239+
240+
Tasks:
241+
242+
- Audit cache invalidation rules for reconnection and subclass-specific connections.
243+
- Decide whether table column metadata should be refreshable without reconnecting.
244+
- Add tests around reconnect and metadata refresh behavior.
245+
246+
Acceptance criteria:
247+
248+
- Reconnection cannot leak stale prepared statements.
249+
- Metadata caching behavior is documented and tested.
250+
251+
### Milestone 3.3: Normalize exception and timestamp behavior across drivers
252+
253+
Tasks:
254+
255+
- Verify `rowCount()` assumptions for update/delete across supported drivers.
256+
- Review timestamp formatting consistency for MySQL, PostgreSQL, and SQLite.
257+
- Ensure identifier quoting and schema discovery stay correct for each supported driver.
258+
259+
Acceptance criteria:
260+
261+
- Driver-specific behavior is explicit where unavoidable.
262+
- Public docs do not imply unsupported guarantees.
263+
264+
## Phase 4: Optional feature expansion
265+
266+
Goal: add features that help real applications, but only if they fit the package's lightweight position.
267+
268+
Priority: lower
269+
270+
These should only start after Phases 1 and 2 are complete.
271+
272+
### Candidate 4.1: Transaction helpers
273+
274+
Possible scope:
275+
276+
- `transaction(callable $callback)`
277+
- pass through `beginTransaction()`, `commit()`, `rollBack()` wrappers
278+
279+
Why:
280+
This adds practical value without changing the core model shape.
281+
282+
### Candidate 4.2: Configurable timestamp columns
283+
284+
Possible scope:
285+
286+
- opt-in timestamp column names
287+
- disable automatic timestamps per model
288+
289+
Why:
290+
The current `created_at` / `updated_at` convention is convenient but rigid.
291+
292+
### Candidate 4.3: Attribute casting
293+
294+
Possible scope:
295+
296+
- integer
297+
- float
298+
- boolean
299+
- datetime
300+
- JSON array/object
301+
302+
Why:
303+
Casting improves ergonomics substantially without requiring relationships or a large query layer.
304+
305+
### Candidate 4.4: Composite keys or relationship support
306+
307+
Why this is last:
308+
This is where complexity rises sharply. It should only happen if the maintainer wants the library to move beyond lightweight active-record usage.
309+
310+
Recommendation:
311+
312+
- Do not start here by default.
313+
- Re-evaluate only after the earlier phases have shipped and real user demand is clear.
314+
315+
## Suggested issue order
316+
317+
If this work is split into GitHub issues, the most practical order is:
318+
319+
1. Replace `__sleep()` with proper serialization support.
320+
2. Fix zero-like primary key handling in `save()`.
321+
3. Make unknown dynamic fields fail fast.
322+
4. Define empty-array query behavior.
323+
5. Introduce library exception classes.
324+
6. Expand regression tests for the above.
325+
7. Convert validation to instance-aware hooks.
326+
8. Add strict field mode.
327+
9. Add focused query helpers.
328+
10. Tighten typing and refresh docs.
329+
330+
## Suggested release strategy
331+
332+
- Release 1: all Phase 1 fixes plus regression tests.
333+
- Release 2: validation, strict mode, typed API cleanup, and documentation refresh.
334+
- Release 3: selected optional helpers such as transactions, timestamp configuration, and casting.
335+
336+
## Out of scope unless demand changes
337+
338+
- Full relationship mapping
339+
- Eager/lazy loading systems
340+
- A chainable query builder comparable to framework ORMs
341+
- Migration tooling
342+
- Schema management
343+
344+
Those features would change the character of the package more than they would improve the current design.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Freshsauce\Model\Exception;
4+
5+
class ConfigurationException extends ModelException
6+
{
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Freshsauce\Model\Exception;
4+
5+
class ConnectionException extends ModelException
6+
{
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Freshsauce\Model\Exception;
4+
5+
class InvalidDynamicMethodException extends ModelException
6+
{
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Freshsauce\Model\Exception;
4+
5+
class MissingDataException extends ModelException
6+
{
7+
}

0 commit comments

Comments
 (0)