feat(core): add postgres feature + PostgresProfileRepository#181
Merged
Conversation
Phase 1.b.2a (RFC-001 §6.5) — give waveflow-core a parallel Postgres
backend so waveflow-server (the upcoming HTTP service) can consume the
same repository traits without rewriting the storage layer.
What's wired:
- New `postgres` Cargo feature mirroring `sqlite`: enables
`sqlx/postgres` + `sqlx/runtime-tokio`. The two features are
mutually compatible (Cargo unifies them) for the rare case where a
consumer wants both backends in one build.
- `repository::postgres::PostgresProfileRepository` mirrors the SQLite
impl one-for-one: same trait, same semantics, `$N` placeholders,
`RETURNING id` instead of `last_insert_rowid()`. The atomic guarded
delete (TOCTOU-free) is preserved as-is — the subquery pattern is
portable across both engines.
- Domain types (`Profile`, `Library`, `Playlist`, `Track`, …) derive
`sqlx::FromRow` on `any(feature = "sqlite", feature = "postgres")`
so they work with both backends.
- Sqlite-only surfaces (`repository::sqlite`, `scanner::upserts`, the
smart-playlist materialisers in `smart_playlists::{custom,generator,
on_repeat}`) are gated behind `cfg(feature = "sqlite")` so a
postgres-only build skips them. `repository::sqlite` is gated the
same way for symmetry.
- `scanner::canonical_name` moved out of `upserts` into its own
`scanner::canonical` module so the postgres-only build can still use
it from `scanner::extract`. Re-exported from `upserts` for source
compatibility.
Zero behaviour change for the desktop app — it still depends on
`waveflow-core` with `features = ["sqlite"]` and every existing test
passes (66 + 45). Validated cleanly under `--features sqlite`,
`--features postgres`, and `--features sqlite,postgres`.
The actual schema lives in `waveflow-server/migrations/` and is
introduced alongside the consumer in Phase 1.b.2b.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAjoute la feature Cargo ChangesSupport PostgreSQL dans waveflow-core
Effort de révision estimé🎯 4 (Complexe) | ⏱️ ~45 minutes PRs possiblement connexes
Labels suggérés
Poème
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
InstaZDLL
added
scope: backend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/core/src/repository/postgres/profile.rs`:
- Around line 107-129: The current delete_guarded implementation can suffer a
race under READ COMMITTED; replace the single statement with a serialized
transaction that takes an explicit lock, e.g. begin a transaction
(self.pool.begin()), acquire a lock on the profile table or the relevant rows
(e.g. execute "LOCK TABLE profile IN SHARE ROW EXCLUSIVE MODE" or SELECT ... FOR
UPDATE on the target row), re-check the COUNT(*) > 1 invariant inside that
transaction and only then perform the DELETE, and finally commit; keep using the
same return logic (ProfileDeleteOutcome::Deleted / WasLast / NotFound) and
reference delete_guarded, exists, and ProfileDeleteOutcome when making the
change; alternatively implement SERIALIZABLE transactions with retry on
serialization failures if you prefer optimistic retry semantics.
In `@src-tauri/crates/core/src/smart_playlists/custom.rs`:
- Around line 347-348: The tests call sql_of(...) which relies on build_node_sql
but build_node_sql is currently gated with #[cfg(feature = "sqlite")], causing
compilation failures when running tests with other feature sets (e.g.,
--features postgres). Make build_node_sql available during testing by changing
its cfg to include test builds (for example #[cfg(any(feature = "sqlite",
test))]) so mod tests can call sql_of(...) without requiring the sqlite feature;
alternatively, gate the test module itself with #[cfg(any(test, feature =
"sqlite"))]—refer to the build_node_sql function and the sql_of call in mod
tests to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ccf69907-2406-4133-b4f1-77d198c2dde0
📒 Files selected for processing (14)
src-tauri/crates/core/Cargo.tomlsrc-tauri/crates/core/src/domain/library.rssrc-tauri/crates/core/src/domain/playlist.rssrc-tauri/crates/core/src/domain/profile.rssrc-tauri/crates/core/src/domain/track.rssrc-tauri/crates/core/src/repository/mod.rssrc-tauri/crates/core/src/repository/postgres/mod.rssrc-tauri/crates/core/src/repository/postgres/profile.rssrc-tauri/crates/core/src/scanner/canonical.rssrc-tauri/crates/core/src/scanner/extract.rssrc-tauri/crates/core/src/scanner/mod.rssrc-tauri/crates/core/src/scanner/upserts.rssrc-tauri/crates/core/src/smart_playlists/custom.rssrc-tauri/crates/core/src/smart_playlists/mod.rs
1. `PostgresProfileRepository::delete_guarded` had a race under READ COMMITTED: two concurrent DELETEs against distinct rows could each read `COUNT = 2`, decide they were safe, and both commit — emptying the table. SQLite is immune because it serialises writers at the file level, but Postgres is not. Wrap the check + delete in a transaction that acquires `LOCK TABLE profile IN SHARE ROW EXCLUSIVE MODE` first; the lock blocks concurrent writers while letting SELECTs proceed, so the COUNT(*) re-check observes the same row set the DELETE will see. 2. `smart_playlists::custom::tests` calls `build_node_sql`, which is now gated on `feature = "sqlite"`, so `cargo test -p waveflow-core --no-default-features --features postgres` failed to type-check. Gate the test module on the same feature so the postgres-only run still compiles. A future postgres-specific test module would live alongside under its own gate. Validated: - cargo check -p waveflow-core (sqlite | postgres | both) - cargo test -p waveflow-core --no-default-features --features postgres (33 ok) - cargo test --workspace (66 + 45 ok, zero regression) - cargo clippy --workspace --all-targets -- -D warnings
PR #181 added the `postgres` feature + `PostgresProfileRepository` to waveflow-core; the architectural docs still described the feature matrix as sqlite-only ("a future server will add a postgres feature"), which is no longer accurate. - `docs/architecture/crates.md`: feature table now has both rows (sqlite + postgres) with the full scope of what each enables (`repository::*`, the sqlite-only `scanner::upserts` + smart-playlist materialisers, the `FromRow` derives). New note on the `scanner::canonical` helper sitting outside both gates for postgres- build reuse. - `docs/architecture/crates.md`: repository-traits bullet now lists both implementations as shipped, not "a future postgres/". - `CLAUDE.md`: backend overview mentions both impls behind their matching feature.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/core/src/smart_playlists/custom.rs`:
- Around line 524-529: The module-level #[cfg(all(test, feature = "sqlite"))]
currently hides tests that validate serialization/migration contracts
(migrate_v1_flat_rules_to_tree and v2_tree_round_trips); instead, restrict only
the SQLite-specific SQL assertion around build_node_sql to #[cfg(feature =
"sqlite")] and keep the serialization/migration tests under #[cfg(test)] so they
run on Postgres-only builds; move or split the SQL-specific assertions into a
helper or separate test annotated with #[cfg(feature = "sqlite")] and ensure
migrate_v1_flat_rules_to_tree and v2_tree_round_trips remain un-gated (or
explicitly #[cfg(test)]).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d0e9f50d-023e-4c03-9ea3-b30d5e784202
📒 Files selected for processing (4)
CLAUDE.mddocs/architecture/crates.mdsrc-tauri/crates/core/src/repository/postgres/profile.rssrc-tauri/crates/core/src/smart_playlists/custom.rs
PR #181's earlier round gated the whole `custom::tests` module on `feature = "sqlite"` to keep the SQL-shape assertions compiling, but that also hid the storage-agnostic serde/migration tests (`migrate_v1_flat_rules_to_tree`, `v2_tree_round_trips`) — running `cargo test -p waveflow-core --no-default-features --features postgres` silently skipped them, so the v1 → v2 rule-tree migration contract wasn't being exercised on the server build. Split: keep `mod tests` under plain `#[cfg(test)]` so the 2 serde tests run on both backends; move the 7 SQL-shape tests + the `sql_of` helper into a nested `sql_tests` module gated on `feature = "sqlite"`.
4 tasks
4 tasks
InstaZDLL
added a commit
that referenced
this pull request
May 30, 2026
) Same shape as the profile work in PRs #181 / #183 / #184: server-only inherent methods that scope every query to both `profile_id` (the resource's owning profile) and `user_id` (the request's authenticated user). The single-tenant `LibraryRepository` trait stays untouched on the desktop side, and `PostgresLibraryRepository` deliberately does NOT implement it — a careless `Box<dyn LibraryRepository>` over the Postgres backend would otherwise let user A walk user B's libraries. Methods (5): - `list_for_profile(profile_id, user_id)` — MRU-first, empty list when the user doesn't own the profile (no tenancy leak, no auth pre-check round-trip) - `get_for_profile(id, profile_id, user_id)` — single row, `None` blurs missing / foreign-profile / foreign-user - `insert_for_profile(draft, profile_id, user_id)` — `INSERT ... SELECT FROM profile WHERE id = $1 AND user_id = $7`, returns the inserted row via `RETURNING *` so the caller skips a follow-up SELECT (same race elimination as PR #184's rename_for_user) - `update_for_profile(id, patch, now_ms, profile_id, user_id)` — COALESCE partial update, `UPDATE ... RETURNING *` for the same reason - `delete_for_profile(id, profile_id, user_id)` — `EXISTS` clause on profile validates ownership without a separate join Every SQL statement encodes the (profile_id, user_id) ownership pair in its WHERE clause so the storage layer is the single point of enforcement — no convention-by-comment, no handler-discipline gap. Domain: - `Library` gains `profile_id: i64` with `#[sqlx(default)]`, mirroring `Profile.user_id`. Desktop SELECTs that omit the column (no `profile_id` on the per-profile SQLite `library` table) still round-trip cleanly via the default. The lone desktop call site (`commands/library::create_library`) now sets `profile_id: 0` explicitly to match. Counts (`track_count`, `album_count`, `artist_count`, `genre_count`, `folder_count`) are stubbed at `0::bigint` in every SELECT for this phase; they become real aggregates as tracks / albums / playlists land in 1.b.5b+, without changing the wire shape. Schema lives in `waveflow-server/migrations/` (next PR): `library.profile_id BIGINT NOT NULL REFERENCES profile(id) ON DELETE CASCADE` + the usual indices. Zero behaviour change on the desktop. Validated: workspace check + clippy + 111 tests pass.
This was referenced May 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1.b.2a — give `waveflow-core` a parallel Postgres backend so the upcoming `waveflow-server` (RFC-001 §6.5) can consume the same repository traits without rewriting the storage layer.
What's wired
Out of scope
Test plan
Next
waveflow-server#2 (1.b.2b) consumes this via `waveflow-core = { git, features = ["postgres"] }` and ships the first migration.
Summary by CodeRabbit
Notes de version
Nouvelles fonctionnalités
Refactorisation
Documentation