From 93065c4bfd97be4e39acb4914d42db7abe1754bc Mon Sep 17 00:00:00 2001 From: Aoife Brennan <276835204+aoife-brennan-bf@users.noreply.github.com> Date: Sun, 9 Aug 2026 05:22:49 +0800 Subject: [PATCH] Wrap user create/delete flows in a Postgres transaction Signup: post-user-insert + session-insert + CREATE MATERIALIZED VIEW all ran un-transacted. Any failure between them (DB blip, view collision from a prior failed attempt, container kill, max_connections, etc.) left the users and session_ids rows orphaned and the materialized view missing. Re-registering the same email then 500'd on the view-create collision. Logged-in users hit 500 on /feed/subscriptions because the view lookup referenced a non-existent relation. Same pattern in account-delete (Users.delete + SessionIDs.delete + DROP MATERIALIZED VIEW). Fix: add an optional `conn` parameter (DB::Database | DB::Connection | DB::Transaction, default PG_DB) to Users.insert/delete and all four SessionIDs overloads, then wrap the create/delete call sites in PG_DB.transaction { |tx| ... }. Default arg keeps every existing call site working unchanged. Verified on a fresh local stack: - registration of a clean user: user + session + view all created - registration with a pre-existing view: 500, zero rows in users and session_ids (rollback worked), pre-existing view untouched Closes iv-org/invidious#2509 --- src/invidious/database/sessions.cr | 16 ++++++++-------- src/invidious/database/users.cr | 8 ++++---- src/invidious/routes/account.cr | 8 +++++--- src/invidious/routes/login.cr | 10 ++++++---- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/invidious/database/sessions.cr b/src/invidious/database/sessions.cr index 96587082..ca083d38 100644 --- a/src/invidious/database/sessions.cr +++ b/src/invidious/database/sessions.cr @@ -7,7 +7,7 @@ module Invidious::Database::SessionIDs # Insert # ------------------- - def insert(sid : String, email : String, handle_conflicts : Bool = false) + def insert(sid : String, email : String, handle_conflicts : Bool = false, *, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) request = <<-SQL INSERT INTO session_ids VALUES ($1, $2, now()) @@ -15,38 +15,38 @@ module Invidious::Database::SessionIDs request += " ON CONFLICT (id) DO NOTHING" if handle_conflicts - PG_DB.exec(request, sid, email) + conn.exec(request, sid, email) end # ------------------- # Delete # ------------------- - def delete(*, sid : String) + def delete(*, sid : String, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) request = <<-SQL DELETE FROM session_ids * WHERE id = $1 SQL - PG_DB.exec(request, sid) + conn.exec(request, sid) end - def delete(*, email : String) + def delete(*, email : String, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) request = <<-SQL DELETE FROM session_ids * WHERE email = $1 SQL - PG_DB.exec(request, email) + conn.exec(request, email) end - def delete(*, sid : String, email : String) + def delete(*, sid : String, email : String, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) request = <<-SQL DELETE FROM session_ids * WHERE id = $1 AND email = $2 SQL - PG_DB.exec(request, sid, email) + conn.exec(request, sid, email) end # ------------------- diff --git a/src/invidious/database/users.cr b/src/invidious/database/users.cr index 4a3056ea..95edef4b 100644 --- a/src/invidious/database/users.cr +++ b/src/invidious/database/users.cr @@ -7,7 +7,7 @@ module Invidious::Database::Users # Insert / delete # ------------------- - def insert(user : User, update_on_conflict : Bool = false) + def insert(user : User, update_on_conflict : Bool = false, *, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) user_array = user.to_a user_array[4] = user_array[4].to_json # User preferences @@ -23,16 +23,16 @@ module Invidious::Database::Users SQL end - PG_DB.exec(request, args: user_array) + conn.exec(request, args: user_array) end - def delete(user : User) + def delete(user : User, *, conn : DB::Database | DB::Connection | DB::Transaction = PG_DB) request = <<-SQL DELETE FROM users * WHERE email = $1 SQL - PG_DB.exec(request, user.email) + conn.exec(request, user.email) end # ------------------- diff --git a/src/invidious/routes/account.cr b/src/invidious/routes/account.cr index c8db207c..4cfc5ac9 100644 --- a/src/invidious/routes/account.cr +++ b/src/invidious/routes/account.cr @@ -124,9 +124,11 @@ module Invidious::Routes::Account end view_name = "subscriptions_#{sha256(user.email)}" - Invidious::Database::Users.delete(user) - Invidious::Database::SessionIDs.delete(email: user.email) - PG_DB.exec("DROP MATERIALIZED VIEW #{view_name}") + PG_DB.transaction do |tx| + Invidious::Database::Users.delete(user, conn: tx.connection) + Invidious::Database::SessionIDs.delete(email: user.email, conn: tx.connection) + tx.connection.exec("DROP MATERIALIZED VIEW #{view_name}") + end env.request.cookies.each do |cookie| cookie.expires = Time.utc(1990, 1, 1) diff --git a/src/invidious/routes/login.cr b/src/invidious/routes/login.cr index 7d7da487..3a1d13dd 100644 --- a/src/invidious/routes/login.cr +++ b/src/invidious/routes/login.cr @@ -122,11 +122,13 @@ module Invidious::Routes::Login end end - Invidious::Database::Users.insert(user) - Invidious::Database::SessionIDs.insert(sid, email) + PG_DB.transaction do |tx| + Invidious::Database::Users.insert(user, conn: tx.connection) + Invidious::Database::SessionIDs.insert(sid, email, conn: tx.connection) - view_name = "subscriptions_#{sha256(user.email)}" - PG_DB.exec("CREATE MATERIALIZED VIEW #{view_name} AS #{MATERIALIZED_VIEW_SQL.call(user.email)}") + view_name = "subscriptions_#{sha256(user.email)}" + tx.connection.exec("CREATE MATERIALIZED VIEW #{view_name} AS #{MATERIALIZED_VIEW_SQL.call(user.email)}") + end if alt = CONFIG.alternative_domains.index(host) env.response.cookies["SID"] = Invidious::User::Cookies.sid(CONFIG.alternative_domains[alt], sid)