chore(db): migrate to postgreSQL#3
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates the server-side persistence layer from SurrealDB to PostgreSQL using Diesel (async + deadpool), including new schema/migrations and updated CRUD endpoints to use a shared connection pool.
Changes:
- Replaced SurrealDB usage with an async PostgreSQL pool and Diesel-based queries across backend endpoints.
- Added Diesel schema/model definitions plus SQL migrations to bootstrap the Postgres schema.
- Updated session storage to a Postgres-backed
tower-sessionsstore and adjusted shared types to match relational IDs.
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Switch session store to Postgres implementation; DB init now initializes a pool. |
| src/backend/mod.rs | Exposes Diesel schema and models under ssr. |
| src/backend/config.rs | Replaces Surreal config with DATABASE_URL-based config. |
| src/backend/db.rs | Implements async Postgres pool initialization + global accessor. |
| src/backend/schema.rs | New Diesel-generated table/relationship definitions. |
| src/backend/models.rs | New Diesel models + conversion/helpers for app types. |
| src/backend/auth.rs | Replaces Surreal session store with Postgres session store implementation. |
| src/backend/user.rs | Migrates user CRUD/auth queries to Diesel + UUID IDs. |
| src/backend/category.rs | Migrates category CRUD to Diesel. |
| src/backend/event.rs | Migrates event CRUD to Diesel. |
| src/backend/product.rs | Migrates product CRUD to Diesel. |
| src/backend/order.rs | Migrates order CRUD to Diesel; delegates item creation. |
| src/backend/item.rs | Migrates item CRUD to Diesel; implements bulk creation helper. |
| src/backend/station.rs | Migrates station + junction tables CRUD to Diesel. |
| src/backend/settings.rs | Migrates settings singleton to Diesel. |
| src/common/types.rs | Aligns shared types with Postgres schema (e.g., order_id required; Order.event_id). |
| src/app/components/station_view.rs | Updates UI grouping logic for non-optional order_id. |
| src/app/components/cashier/order.rs | Updates temporary items to use String order_id. |
| migrations/2026-03-12-163145-0000_initial_schema/up.sql | Adds initial Postgres schema for all core tables + indexes. |
| migrations/2026-03-12-163145-0000_initial_schema/down.sql | Drops Postgres schema objects. |
| migrations/00000000000000_diesel_initial_setup/up.sql | Diesel helper functions migration. |
| migrations/00000000000000_diesel_initial_setup/down.sql | Rollback for Diesel helper functions migration. |
| diesel.toml | Configures Diesel schema output location. |
| docker-compose.yaml | Adds a local Postgres service for dev/test. |
| Cargo.toml | Adds Diesel/Diesel-async/UUID/chrono deps and bumps Leptos ecosystem versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let station: DbStation = diesel::update(stations.filter(id.eq(sid))) | ||
| .set(UpdateStation { | ||
| name: Some(name), | ||
| output_status: Some(order_status_str(output_status).to_string()), | ||
| }) | ||
| .returning(DbStation::as_returning()) | ||
| .get_result(&mut conn) | ||
| .await | ||
| .map_err(|e| Error::InternalError(format!("Failed to update station: {}", e)))?; | ||
|
|
||
| // Replace junction rows | ||
| diesel::delete(sc_dsl::station_categories.filter(sc_dsl::station_id.eq(sid))) | ||
| .execute(&mut conn) | ||
| .await | ||
| .map_err(|e| Error::InternalError(e.to_string()))?; | ||
|
|
||
| diesel::delete(sis_dsl::station_input_statuses.filter(sis_dsl::station_id.eq(sid))) | ||
| .execute(&mut conn) | ||
| .await | ||
| .map_err(|e| Error::InternalError(e.to_string()))?; | ||
|
|
There was a problem hiding this comment.
Station update replaces junction rows via delete+insert without a transaction. Any failure after the deletes (e.g., inserting new junction rows) will leave the station with missing categories/statuses. Wrap the update + junction replacement steps in a transaction.
| CREATE TABLE items ( | ||
| id SERIAL PRIMARY KEY, | ||
| order_id INTEGER NOT NULL REFERENCES orders(id) ON DELETE CASCADE, | ||
| product_id INTEGER NOT NULL REFERENCES products(id) ON DELETE RESTRICT, | ||
| quantity INTEGER NOT NULL, | ||
| price DOUBLE PRECISION NOT NULL, | ||
| status TEXT NOT NULL DEFAULT 'Ordered' | ||
| ); |
There was a problem hiding this comment.
The schema allows negative/zero items.quantity and negative price, even though the application validates these as positive/non-negative. Add DB-level CHECK constraints (e.g., quantity >= 1, price >= 0) to prevent invalid data from being persisted via direct DB writes or buggy clients.
| ns: std::env::var("SURREAL_NS").unwrap_or_else(|_| "dev".to_string()), | ||
| }; | ||
| let database_url = std::env::var("DATABASE_URL") | ||
| .unwrap_or_else(|_| "postgres://user:password@localhost:5432/my_database".to_string()); |
There was a problem hiding this comment.
AppConfig::from_env silently falls back to a placeholder DATABASE_URL. If DATABASE_URL is missing in production, the app may connect to an unintended database. Consider failing fast when DATABASE_URL is not set (or at least using a clearly invalid default and logging a warning).
| .unwrap_or_else(|_| "postgres://user:password@localhost:5432/my_database".to_string()); | |
| .map_err(|e| -> Box<dyn std::error::Error> { Box::new(e) })?; |
| pub async fn initialize_database() -> Result<(), String> { | ||
| use crate::backend::config::AppConfig; | ||
|
|
||
| let config = AppConfig::from_env().map_err(|e| format!("Could not load env: {}", e))?; | ||
|
|
||
| DB.connect::<Ws>(&config.database.url) | ||
| let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new(&config.database_url); | ||
| let pool = Pool::builder(manager) | ||
| .build() | ||
| .map_err(|e| format!("Failed to build connection pool: {}", e))?; | ||
|
|
||
| // Verify connectivity | ||
| pool.get() | ||
| .await | ||
| .map_err(|e| format!("Failed to connect to database: {}", e))?; | ||
|
|
||
| DB.signin(Root { | ||
| username: &config.database.user, | ||
| password: &config.database.pass, | ||
| }) | ||
| .await | ||
| .map_err(|e| format!("Failed to sign in: {}", e))?; | ||
|
|
||
| DB.use_ns(&config.database.ns) | ||
| .use_db(&config.database.db) | ||
| .await | ||
| .map_err(|e| format!("Failed to select namespace/database: {}", e))?; | ||
| POOL.set(pool).map_err(|_| "Pool already initialized".to_string())?; | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
Postgres migrations are present and diesel_migrations is included, but the app never runs migrations at startup. If deployments/dev environments don't run diesel migration run separately, the server will start with an empty schema. Consider running pending migrations during initialize_database (or clearly documenting the required migration step).
| let new_item = NewItem { | ||
| order_id: oid, | ||
| product_id: pid, | ||
| quantity: item.quantity as i32, |
There was a problem hiding this comment.
NewItem.quantity is derived via item.quantity as i32. Casting u32→i32 with as will wrap for values > i32::MAX, potentially inserting negative quantities. Use a checked conversion (try_into) and return an error when the value is out of range (and consider adding DB constraints as a second line of defense).
| let new_item = NewItem { | |
| order_id: oid, | |
| product_id: pid, | |
| quantity: item.quantity as i32, | |
| let quantity: i32 = i32::try_from(item.quantity) | |
| .map_err(|_| Error::InternalError("Quantity out of range".to_string()))?; | |
| let new_item = NewItem { | |
| order_id: oid, | |
| product_id: pid, | |
| quantity, |
| id: i.id.to_string(), | ||
| order_id: i.order_id.to_string(), | ||
| product_id: i.product_id.to_string(), | ||
| quantity: i.quantity as u32, |
There was a problem hiding this comment.
DbItem.quantity is converted with i.quantity as u32. If the DB ever contains a negative quantity (no DB CHECK constraint currently), this cast will wrap to a large u32 and surface corrupt data. Prefer a checked conversion and/or enforce quantity > 0 at the database level.
| quantity: i.quantity as u32, | |
| quantity: u32::try_from(i.quantity).unwrap_or(0), |
| let count: i64 = users | ||
| .count() | ||
| .get_result(&mut conn) | ||
| .await | ||
| .map_err(|e| Error::InternalError(e.to_string()))?; | ||
|
|
||
| let assigned_role = if count == 0 { types::Role::Admin } else { types::Role::Visitor }; | ||
|
|
||
| let new_user = NewUser { | ||
| email: &req.email, | ||
| password_hash: &pw_hash, | ||
| role: role_str(&assigned_role), | ||
| }; |
There was a problem hiding this comment.
Role assignment based on users.count() is race-prone: concurrent registrations can both observe count == 0 and each get Admin. If "first user is admin" must be enforced, wrap in a transaction with appropriate locking or use a DB-level mechanism to guarantee a single admin bootstrap.
| let pool = get_pool(); | ||
| let mut conn = pool.get().await.expect("pool unavailable"); | ||
|
|
There was a problem hiding this comment.
get_user_by_email panics on pool acquisition failure via expect("pool unavailable"), which will crash the server under transient DB/pool issues. Return an error instead (and consider aligning the return type to propagate pool errors, e.g. map to Error::InternalError like the other DB accessors).
| let changes = UpdateItem { | ||
| product_id: pid, | ||
| quantity: update.quantity.map(|q| q as i32), | ||
| price: new_price, | ||
| status: update.status.or_else(|| Some(item.status)).unwrap(), | ||
| status: update.status.map(|s| order_status_str(s).to_string()), |
There was a problem hiding this comment.
UpdateItem builds quantity: update.quantity.map(|q| q as i32), which has the same u32→i32 wrapping risk as item creation. Use a checked conversion and surface a validation error if the client sends an out-of-range quantity.
| let order: DbOrder = diesel::insert_into(orders) | ||
| .values(NewOrder { event_id: eid }) | ||
| .returning(DbOrder::as_returning()) | ||
| .get_result(&mut conn) | ||
| .await | ||
| .map_err(|e| Error::InternalError(format!("Failed to create order: {}", e)))?; | ||
|
|
||
| let order_type: types::Order = order.into(); | ||
| broadcast_add(order_type.clone()); | ||
|
|
||
| // Then create all the items | ||
|
|
||
| if !req.items.is_empty() { | ||
| create_items(order_id, req.items).await?; | ||
| use crate::backend::item::create_items; | ||
| create_items(order_type.id.clone(), req.items).await?; | ||
| } |
There was a problem hiding this comment.
Order creation is not atomic: the order row is inserted and broadcast before item creation runs, so a failure in create_items can leave an order without its items and clients notified of a non-existent/partial order. Wrap order+items creation in a DB transaction and only broadcast after the transaction commits successfully.
Migrate the whole database from a relational document mutant on surreal to relational postgreSQL