mirror of
https://codeberg.org/PostERG/xamxam.git
synced 2026-05-06 19:19:19 +02:00
docs: ORM assessment — verdict: keep raw PDO, no ORM needed
This commit is contained in:
6
TODO.md
6
TODO.md
@@ -35,6 +35,12 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## Analysis / Reports
|
||||||
|
|
||||||
|
- [x] ORM assessment written → `docs/ORM_ASSESSMENT.md`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## NEW FEATURES
|
## NEW FEATURES
|
||||||
|
|
||||||
### 1 — License page (public)
|
### 1 — License page (public)
|
||||||
|
|||||||
105
docs/ORM_ASSESSMENT.md
Normal file
105
docs/ORM_ASSESSMENT.md
Normal file
@@ -0,0 +1,105 @@
|
|||||||
|
# ORM Assessment — posterg
|
||||||
|
|
||||||
|
> **Date:** 2026-03-28
|
||||||
|
> **Scope:** Full codebase review — `src/Database.php`, all `public/` PHP files, `storage/schema.sql`, migrations, tests.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Verdict: No ORM needed
|
||||||
|
|
||||||
|
An ORM would add complexity and dependency weight without solving any real pain points in this project. The recommendation is to **keep raw PDO** and instead make one targeted refactor to the `edit.php` file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## What the project actually is
|
||||||
|
|
||||||
|
- **PHP 8.4 / SQLite3**, single-file database (`posterg.db`)
|
||||||
|
- ~1 130-line `Database.php` service class wrapping a single `PDO` instance
|
||||||
|
- ~10 public-facing PHP files + ~15 admin PHP files
|
||||||
|
- One developer, non-commercial, low-to-medium traffic school project
|
||||||
|
- No Composer, no framework, no autoloader — intentionally minimal
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Where SQL lives today
|
||||||
|
|
||||||
|
| Location | SQL type | How parameterised |
|
||||||
|
|---|---|---|
|
||||||
|
| `src/Database.php` | All major queries (reads, writes, views, search) | Named/positional PDO bindings throughout |
|
||||||
|
| `public/admin/actions/formulaire.php` | INSERT thesis, link authors/jury (create path) | PDO positional params |
|
||||||
|
| `public/admin/actions/publish.php` | Bulk/single `UPDATE is_published` | PDO positional params |
|
||||||
|
| `public/admin/edit.php` | Large `UPDATE theses SET …`, DELETE+INSERT junction tables | Raw PDO via `$pdo = $db->getPDO()` — **partially bypasses the service class** |
|
||||||
|
| `public/admin/import.php` | Multi-row INSERT during CSV import | Raw PDO via `$pdo = $db->getPDO()` |
|
||||||
|
|
||||||
|
The pattern is largely consistent: `Database.php` is the canonical data-access layer. Two admin files (`edit.php`, `import.php`) poke through it via `getPDO()` for operations the service class doesn't expose as methods. That's the main rough edge, not something that calls for an ORM.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Arguments for an ORM (examined and rejected)
|
||||||
|
|
||||||
|
### "The schema is relational with many junctions"
|
||||||
|
|
||||||
|
The schema is normalised (14 tables, 6 junction tables, 2 views), but every relationship is **read by the database views** (`v_theses_full`, `v_theses_public`) which already aggregate `GROUP_CONCAT` columns. The PHP code barely touches junction tables directly — it calls `setThesisTags()`, `setThesisJury()`, `setThesisLanguages()` etc., which are already encapsulated service methods.
|
||||||
|
An ORM would re-implement these aggregations less efficiently and lose the view layer's performance benefits.
|
||||||
|
|
||||||
|
### "There is duplicated SQL in edit.php / formulaire.php"
|
||||||
|
|
||||||
|
True, but this is a **missing method on `Database.php`**, not an ORM problem. The `updateThesis()` method simply doesn't exist yet; `edit.php` compensates by calling `$db->getPDO()` and writing the UPDATE inline. The fix is a 30-line method addition, not adopting a 50 MB ORM.
|
||||||
|
|
||||||
|
### "Migrations are hand-written SQL files"
|
||||||
|
|
||||||
|
There are 6 migrations in `storage/migrations/`, all trivial (`ALTER TABLE ADD COLUMN`, `CREATE INDEX`, `INSERT OR IGNORE`). This is an appropriate level of complexity for a SQLite project with one schema target (no multi-tenant, no multi-DB sharding). An ORM's migration runner would add overhead without benefit.
|
||||||
|
|
||||||
|
### "Type safety — PHP arrays aren't typed"
|
||||||
|
|
||||||
|
The codebase already returns typed arrays from PDO with `FETCH_ASSOC` and documents return shapes in docblocks (`@return array{license_id:int|null,…}`). PHP 8.4 property types on a couple of DTOs could improve IDE ergonomics if desired, but that's independent of ORM adoption.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Arguments against an ORM (decisive)
|
||||||
|
|
||||||
|
### 1. The search query cannot be replicated by an ORM query builder
|
||||||
|
|
||||||
|
`Database::searchTheses()` builds a WHERE clause over `v_theses_public` with up to 9 optional filters including an `EXISTS` subquery over `thesis_tags → tags`. This is a deliberate, LIKE-escaped, parameterised query that maps onto a database view. Every ORM would either:
|
||||||
|
- Force re-implementation as raw SQL passed to the ORM's `raw()` escape hatch (no benefit), or
|
||||||
|
- Generate a massively suboptimal N+1 or multi-JOIN query that duplicates what the view already does.
|
||||||
|
|
||||||
|
### 2. The schema uses database views as the primary read layer
|
||||||
|
|
||||||
|
`v_theses_full` and `v_theses_public` join 15 tables and aggregate via `GROUP_CONCAT`. ORMs treat views as read-only proxies at best; the GROUP_CONCAT columns (`authors`, `keywords`, `languages`, etc.) aren't mappable to ORM collection properties without a custom hydrator. The view approach was a deliberate performance trade-off — an ORM would fight it.
|
||||||
|
|
||||||
|
### 3. SQLite is a poor fit for most PHP ORMs
|
||||||
|
|
||||||
|
Doctrine ORM and Eloquent have first-class SQLite support but with caveats: no `ALTER TABLE ADD COLUMN` rollback, no partial indexes, different date handling, no stored procedures. The current migration approach (plain `.sql` files applied manually/via script) is actually more reliable for SQLite than an ORM migration runner.
|
||||||
|
|
||||||
|
### 4. The project has zero dependencies by design
|
||||||
|
|
||||||
|
`composer.json` does not exist. There is no autoloader. The bootstrap is `require_once __DIR__ . '/../../src/Database.php'`. Introducing an ORM means either Doctrine (~4 MB, 15+ packages) or Eloquent standalone (~2 MB, plus Capsule bootstrapping). For a school project with one SQLite file, this dependency overhead is not justified.
|
||||||
|
|
||||||
|
### 5. The one "leaky abstraction" has a trivial fix
|
||||||
|
|
||||||
|
`edit.php` and `import.php` call `$db->getPDO()` because `Database.php` is missing two methods:
|
||||||
|
|
||||||
|
- `updateThesis(int $id, array $fields): void`
|
||||||
|
- `importThesisRow(array $data): int`
|
||||||
|
|
||||||
|
Adding these two methods completes the service-class encapsulation. After that, **no file outside `src/Database.php` would need raw SQL**.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## What should actually be done instead
|
||||||
|
|
||||||
|
| Priority | Action |
|
||||||
|
|---|---|
|
||||||
|
| Low | Add `Database::updateThesis()` method to encapsulate the `UPDATE theses SET …` in `edit.php`. |
|
||||||
|
| Low | Add `Database::importThesisRow()` method to encapsulate the CSV import INSERT chain in `import.php`. |
|
||||||
|
| Optional | Add `Database::relinkThesisAuthors()` to replace the delete+reinsert block in `edit.php` (already done for jury/languages/formats/tags). |
|
||||||
|
| Never | Adopt an ORM. |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
The current architecture — a single handwritten PDO service class backed by a SQLite database with view-based reads — is **the right tool for this project's scale and context**. The SQL is well-parameterised, the views handle join complexity efficiently, and the one rough edge (two admin pages calling `getPDO()` directly) is a 60-line refactor, not an architectural crisis.
|
||||||
|
|
||||||
|
An ORM would increase complexity, add a large dependency, and provide no benefit that isn't already delivered by `Database.php`.
|
||||||
Reference in New Issue
Block a user