feat: profile CRUD + X-User-Id shim (Phase 1.b.4b)#6
Merged
Conversation
Wire the first real data routes on top of the tenant-scoped repository
methods that landed in waveflow#183. Phase 1.b.4 in three commits:
schema, middleware, handlers.
Schema:
- `20260530000000_users.sql` — new `users` table (BIGSERIAL id +
epoch-ms `created_at`). Phase 1.d will plug Better Auth metadata
onto the same row, but the FK destination is what matters now.
- `20260530000001_profile_user_id.sql` — DROP + CREATE `profile` with
`user_id BIGINT NOT NULL REFERENCES users(id) ON DELETE CASCADE` +
composite `(user_id, last_used_at DESC)` index (powers
`list_for_user`) + dedicated `user_id` index (powers the
delete-time `ORDER BY id FOR UPDATE` lock). The old migration is
immutable but the table was empty everywhere (CRUD wasn't live yet),
so DROP + CREATE is simpler than an ALTER with backfill.
Middleware (`src/middleware.rs`):
- `require_user_id` extracts `X-User-Id` off the request, parses it
as `i64`, rejects 401 if missing / non-positive / unparseable,
injects a typed `UserId(i64)` extension. Documented as a dev shim
that 1.d replaces with JWT verification — the `UserId` shape stays
the same so handlers don't need to change.
Handlers:
- `POST /api/v1/users` — bootstrap: no auth, returns the new id.
Without this the dev caller has no way to populate `X-User-Id`.
Retires in 1.d when Better Auth owns user creation.
- `GET /api/v1/profiles` — `list_for_user`, MRU-first.
- `POST /api/v1/profiles` — `insert_for_user` + read-back. FK
violation (X-User-Id with no matching `users` row) surfaces as 409.
- `GET /api/v1/profiles/{id}` — `get_for_user`, 404 when missing OR
owned by someone else (the repository deliberately blurs the two
so we don't leak the existence of other tenants' rows).
- `PATCH /api/v1/profiles/{id}` — `rename_for_user` + read-back.
- `DELETE /api/v1/profiles/{id}` — `delete_guarded_for_user`, 409
when the row was the user's last remaining profile (same invariant
the desktop's ProfileSelectorModal enforces client-side).
Each handler carries a `#[utoipa::path]` declaration with status
codes + body schemas, so the OpenAPI doc at `/openapi.json` and the
Scalar UI at `/reference` stay in lockstep.
Tests in `tests/profiles.rs` (8 scenarios):
- `create_user_returns_201_with_id`
- `profiles_require_x_user_id` (5 verbs × 401)
- `malformed_x_user_id_rejected` (empty/abc/0/-1 → 401)
- `create_then_list_then_get` — happy path round-trip
- `tenants_are_isolated` — user B sees nothing of user A's profile,
not via list nor get nor delete
- `update_renames_in_place`
- `delete_blocks_last_profile` (409 + still 1 in list)
- `delete_succeeds_when_more_than_one` (204 + remaining row visible)
- `create_with_unknown_user_id_returns_409` (FK violation surface)
Also bumps the `waveflow-core` git rev to 24ffa58 (the merge SHA of
the tenant-method PR) and updates CLAUDE.md + README so the next
session sees the new endpoints in the published surface.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
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)
📝 WalkthroughWalkthroughCette PR ajoute la table ChangesTenant-Scoped Profile Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@migrations/20260530000001_profile_user_id.sql`:
- Around line 33-36: Supprimez la création de l'index redondant
profile_user_id_idx : le composite profile_user_last_used_idx (user_id,
last_used_at DESC) couvre déjà les recherches et verrous sur user_id, donc
retirez la ligne CREATE INDEX profile_user_id_idx ON profile (user_id); et, si
nécessaire pour les migrations déjà appliquées, fournissez dans la migration
correspondante une instruction DROP INDEX IF EXISTS profile_user_id_idx pour
supprimer l'index existant afin d'éviter l'amplification d'écriture et
l'utilisation de stockage inutile.
In `@src/api/mod.rs`:
- Around line 42-45: La route profiles::router est montée avec le middleware
auth_middleware::require_user_id qui accepte le header forgeable X-User-Id;
change la branche pour ne pas l’exposer en production: guarder l’ajout du
middleware derrière un flag de configuration (ex.
config.allow_unsafe_user_id_header) ou un env dev/test check, ou remplacer
require_user_id par un middleware de production qui vérifie une authentification
robuste (JWT/session lookup) avant d’injecter UserId; modifyez l’endroit où
.merge(profiles::router(...).layer(middleware::from_fn(auth_middleware::require_user_id)))
est appelé pour appliquer ce contrôle conditionnel.
In `@src/api/profiles.rs`:
- Around line 250-269: Current rename_then_read is non-atomic:
repo.rename_for_user followed by repo.get_for_user can yield a false 404 if a
concurrent DELETE occurs; change repo.rename_for_user to return the updated row
(use an UPDATE ... RETURNING in the repository and have rename_for_user return
Option<Profile> or Result<Profile, _>) or implement rename_for_user to run
UPDATE + SELECT within a DB transaction and return the updated profile, then in
profiles.rs replace the post-rename get_for_user call with the returned Profile
and construct ProfileResponse directly (update any call sites and signatures
such as rename_for_user and usages that expect a bool).
- Around line 92-95: The OpenAPI annotations for the profile handlers are
missing the 500 response the implementations already emit; update each
#[utoipa::path] on the profile endpoints (the attributes covering the blocks at
the commented ranges) to include a (status = 500, description = "Internal Server
Error", body = <error-type>) entry, where <error-type> is the public
serde::Serialize error response struct used by your repository/handlers (or
create one stable public ErrorResponse type if none exists), and ensure the
attribute lists that error type as the body so /openapi.json reflects the real
behavior; apply this change to each affected handler attribute group referenced
in the comment.
In `@src/api/users.rs`:
- Around line 42-44: The OpenAPI responses macro currently lists only (status =
201, description = "User created", body = CreateUserResponse) but the handler
can return INTERNAL_SERVER_ERROR; update the responses(...) invocation to also
declare a 500 response (e.g. (status = 500, description = "Internal Server
Error")) so the generated /openapi.json matches runtime behavior; modify the
same responses(...) block near the handler that returns INTERNAL_SERVER_ERROR
and ensure the 500 entry is added alongside the 201 entry.
In `@tests/profiles.rs`:
- Around line 6-7: Le doc-comment fait référence à un helper inexistant
`expect_user_id`; remplace cette référence par le nom réel du helper `mint_user`
dans le module de tests (mettre à jour toute occurrence de `expect_user_id` dans
les doc-comments en haut de tests/profiles.rs afin qu'elle mentionne
`mint_user`), pour que la documentation reflète correctement la fonction
définie.
🪄 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: 83d4cc29-7371-44bf-a977-02f9f6c29668
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock,!*.lock
📒 Files selected for processing (12)
CLAUDE.mdCargo.tomlREADME.mdmigrations/20260530000000_users.sqlmigrations/20260530000001_profile_user_id.sqlsrc/api/mod.rssrc/api/profiles.rssrc/api/users.rssrc/db.rssrc/lib.rssrc/middleware.rstests/profiles.rs
Four valid issues applied, one skipped with rationale.
1. APPLIED — Drop the redundant `profile_user_id_idx` index. The
composite `(user_id, last_used_at DESC)` already serves equality
filters on its leading column (the `delete_guarded_for_user` lock
query, the FK cascade lookup) on top of the MRU-ordered list.
Keeping the dedicated single-column index was pure write
amplification for no measurable read benefit.
2. APPLIED — Gate the dev-shim middleware behind
`WAVEFLOW_DEV_AUTH=1`. `Config::dev_auth_enabled` defaults to
`false`; in that state every `/api/v1/*` request short-circuits to
503 via a new `reject_dev_auth_disabled` layer (mounted on both
`users::router` and `profiles::router`). The 503 — rather than
401 — intentionally hides that the shim is even compiled in, so a
production probe can't tell whether `X-User-Id` does anything.
Tests opt in via `Config { dev_auth_enabled: true, .. }` on
spawn_app, plus a new `dev_auth_gate_returns_503_when_disabled`
test that verifies the production-default path.
3. APPLIED — Add the `(status = 500, description = ...)` response
entry to every `#[utoipa::path]` block that has a 500 path. Body
is documented as plain text (matches what `(StatusCode, &str)`
produces). Affects all 5 profile handlers + the `users::create`
handler.
4. APPLIED — Fix the dangling `expect_user_id` reference in
`tests/profiles.rs`'s doc-comment header. The actual helper is
`mint_user`; the docstring now matches.
5. SKIPPED — Atomic UPDATE + SELECT for `rename_profile`. The race
produces a 404 when a concurrent DELETE commits between our
`UPDATE` and the read-back — which is the same response a
delete-then-rename request order would naturally produce. A
proper fix requires changing
`PostgresProfileRepository::rename_for_user` (and ideally
`insert_for_user`, `touch_last_used_for_user`) to use
`UPDATE ... RETURNING *` in waveflow-core. Deferring to a paired
sweep on those methods rather than a piecemeal hotfix on PR #6.
Validated: cargo check + clippy + fmt + the (offline) workspace
tests pass; the new gate test will exercise the production path on
CI.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/support.rs (1)
20-77: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winFactoriser les deux helpers quasi identiques.
spawn_app_prod_gateetspawn_appne diffèrent que pardev_auth_enabled. Cette duplication a déjà dû être maintenue en double (ajout récent du champ) et finira par diverger (timeout, max_connections…). Extraire un helper paramétré.♻️ Extraction proposée
-#[allow(dead_code)] // some test files don't use this helper -pub async fn spawn_app_prod_gate(pool: PgPool) -> String { - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); - - let config = Config { - bind_addr: addr, - request_timeout_secs: 5, - database_url: "<test>".into(), - db_max_connections: 1, - dev_auth_enabled: false, - }; - - let state = AppState { db: pool }; - tokio::spawn(async move { - axum::serve( - listener, - app(config, state).into_make_service_with_connect_info::<SocketAddr>(), - ) - .await - .unwrap(); - }); - - format!("http://{addr}") -} - -/// Spawn the app in the background and return its base URL. -pub async fn spawn_app(pool: PgPool) -> String { - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); - - let config = Config { - bind_addr: addr, - request_timeout_secs: 5, - // `database_url` is unused by `app()` once the pool is built — - // it lives in the config purely so `Config::from_env` is the - // single source of truth for env reading. Placeholder string - // is fine inside the test. - database_url: "<test>".into(), - db_max_connections: 1, - // Integration tests exercise the dev `X-User-Id` shim — the - // production gate is what we want to verify *separately* - // (see `dev_auth_gate_returns_503_when_disabled`), here we - // just want the routes reachable. - dev_auth_enabled: true, - }; - - let state = AppState { db: pool }; - tokio::spawn(async move { - axum::serve( - listener, - app(config, state).into_make_service_with_connect_info::<SocketAddr>(), - ) - .await - .unwrap(); - }); - - format!("http://{addr}") -} +async fn spawn_with(pool: PgPool, dev_auth_enabled: bool) -> String { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let config = Config { + bind_addr: addr, + request_timeout_secs: 5, + // `database_url` is unused by `app()` once the pool is built — + // placeholder is fine inside the test. + database_url: "<test>".into(), + db_max_connections: 1, + dev_auth_enabled, + }; + + let state = AppState { db: pool }; + tokio::spawn(async move { + axum::serve( + listener, + app(config, state).into_make_service_with_connect_info::<SocketAddr>(), + ) + .await + .unwrap(); + }); + + format!("http://{addr}") +} + +/// Spawn the app with the production-default config — dev auth +/// **disabled**. Verifies the 503 gate when `WAVEFLOW_DEV_AUTH != "1"`. +#[allow(dead_code)] // some test files don't use this helper +pub async fn spawn_app_prod_gate(pool: PgPool) -> String { + spawn_with(pool, false).await +} + +/// Spawn the app in the background (dev shim enabled) and return its base URL. +pub async fn spawn_app(pool: PgPool) -> String { + spawn_with(pool, true).await +}🤖 Prompt for 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. In `@tests/support.rs` around lines 20 - 77, Les deux helpers spawn_app_prod_gate et spawn_app sont dupliqués; refactorise en extrayant une fonction unique (par ex. spawn_app_with_dev_auth(pool: PgPool, dev_auth_enabled: bool) -> String) qui construit le Config (même request_timeout_secs, database_url, db_max_connections) et lance l'app via axum::serve/app(...). Remplace spawn_app_prod_gate et spawn_app par de simples wrappers qui appellent spawn_app_with_dev_auth(..., false) et spawn_app_with_dev_auth(..., true) respectivement, et mets à jour les appels de tests pour utiliser ces wrappers.
🤖 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.
Outside diff comments:
In `@tests/support.rs`:
- Around line 20-77: Les deux helpers spawn_app_prod_gate et spawn_app sont
dupliqués; refactorise en extrayant une fonction unique (par ex.
spawn_app_with_dev_auth(pool: PgPool, dev_auth_enabled: bool) -> String) qui
construit le Config (même request_timeout_secs, database_url,
db_max_connections) et lance l'app via axum::serve/app(...). Remplace
spawn_app_prod_gate et spawn_app par de simples wrappers qui appellent
spawn_app_with_dev_auth(..., false) et spawn_app_with_dev_auth(..., true)
respectivement, et mets à jour les appels de tests pour utiliser ces wrappers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81f41650-b14b-4fd6-8047-127ebe480533
📒 Files selected for processing (10)
.env.exampleREADME.mdmigrations/20260530000001_profile_user_id.sqlsrc/api/mod.rssrc/api/profiles.rssrc/api/users.rssrc/config.rssrc/lib.rstests/profiles.rstests/support.rs
4 tasks
CodeRabbit caught the duplication: spawn_app and spawn_app_prod_gate were copies that differed only in the `dev_auth_enabled` bool. Extract the common boot path into `spawn_app_with_dev_auth(pool, dev_auth_enabled)` and reduce the two public helpers to one-liner wrappers. Test call sites are unchanged. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
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.4b — first real data routes on top of the tenant-scoped repository methods landed in waveflow#183. `waveflow-core` git rev bumped to the merge SHA (`24ffa58`).
Schema (2 new migrations)
Middleware (`src/middleware.rs`)
`require_user_id` extracts `X-User-Id` off the request, parses it as `i64`, rejects 401 on missing / non-positive / unparseable, injects a typed `UserId(i64)` extension. Phase 1.d swaps the body for JWT verification — the `UserId` shape stays so handlers don't change.
Handlers
Every handler carries a `#[utoipa::path]` declaration with status codes + body schemas, so `/openapi.json` and the Scalar UI at `/reference` stay in lockstep.
Tests (`tests/profiles.rs`, 8 scenarios)
Plus CLAUDE.md + README updated with the new endpoints and the tenancy discipline.
Test plan
Out of scope
Signed-off-by: InstaZDLL github.105mh@8shield.net
Summary by CodeRabbit