From a6627ff828f87956c690faf2c07bfded64914105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= Date: Sun, 18 May 2025 23:42:15 +0200 Subject: [PATCH] fix(app-lib): delay account skin data deletion to next reboot This gives users an opportunity to not unexpectedly lose skin data in case they log off on accident. --- ...b878b6789927df5adf50986fe642c8afcb681.json | 12 +++++ ...d1821b46d60f5cb79685c489aaebf13b35d24.json | 12 +++++ .../20250413162050_skin-selector.sql | 54 +++++++++++++++++-- packages/app-lib/src/api/settings.rs | 2 + packages/app-lib/src/state/db.rs | 28 ++++++++++ 5 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 packages/app-lib/.sqlx/query-e9449930a74c6a6151c3d868042b878b6789927df5adf50986fe642c8afcb681.json create mode 100644 packages/app-lib/.sqlx/query-fd494269d944b179ade61876669d1821b46d60f5cb79685c489aaebf13b35d24.json diff --git a/packages/app-lib/.sqlx/query-e9449930a74c6a6151c3d868042b878b6789927df5adf50986fe642c8afcb681.json b/packages/app-lib/.sqlx/query-e9449930a74c6a6151c3d868042b878b6789927df5adf50986fe642c8afcb681.json new file mode 100644 index 000000000..a09ac2ff7 --- /dev/null +++ b/packages/app-lib/.sqlx/query-e9449930a74c6a6151c3d868042b878b6789927df5adf50986fe642c8afcb681.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "DELETE FROM default_minecraft_capes WHERE minecraft_user_uuid NOT IN (SELECT uuid FROM minecraft_users)", + "describe": { + "columns": [], + "parameters": { + "Right": 0 + }, + "nullable": [] + }, + "hash": "e9449930a74c6a6151c3d868042b878b6789927df5adf50986fe642c8afcb681" +} diff --git a/packages/app-lib/.sqlx/query-fd494269d944b179ade61876669d1821b46d60f5cb79685c489aaebf13b35d24.json b/packages/app-lib/.sqlx/query-fd494269d944b179ade61876669d1821b46d60f5cb79685c489aaebf13b35d24.json new file mode 100644 index 000000000..ee41aad88 --- /dev/null +++ b/packages/app-lib/.sqlx/query-fd494269d944b179ade61876669d1821b46d60f5cb79685c489aaebf13b35d24.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "DELETE FROM custom_minecraft_skins WHERE minecraft_user_uuid NOT IN (SELECT uuid FROM minecraft_users)", + "describe": { + "columns": [], + "parameters": { + "Right": 0 + }, + "nullable": [] + }, + "hash": "fd494269d944b179ade61876669d1821b46d60f5cb79685c489aaebf13b35d24" +} diff --git a/packages/app-lib/migrations/20250413162050_skin-selector.sql b/packages/app-lib/migrations/20250413162050_skin-selector.sql index f76e667b4..615318f5c 100644 --- a/packages/app-lib/migrations/20250413162050_skin-selector.sql +++ b/packages/app-lib/migrations/20250413162050_skin-selector.sql @@ -2,11 +2,34 @@ CREATE TABLE default_minecraft_capes ( minecraft_user_uuid TEXT NOT NULL, id TEXT NOT NULL, - PRIMARY KEY (minecraft_user_uuid, id), - FOREIGN KEY (minecraft_user_uuid) REFERENCES minecraft_users(uuid) - ON DELETE CASCADE ON UPDATE CASCADE + PRIMARY KEY (minecraft_user_uuid, id) ); +-- Emulate a ON UPDATE CASCADE foreign key constraint for the user UUID on the default_minecraft_capes table, +-- but allowing deletion of the user UUID in the minecraft_users table. This allows the application to temporarily +-- keep skin state around for logged-out users, allowing them to retain their skins under the right conditions +CREATE TRIGGER default_minecraft_capes_user_uuid_insert_check + BEFORE INSERT ON default_minecraft_capes FOR EACH ROW + BEGIN + SELECT CASE WHEN NOT EXISTS ( + SELECT 1 FROM minecraft_users WHERE uuid = NEW.minecraft_user_uuid + ) THEN RAISE(ABORT, 'Cannot add a default cape for an unknown Minecraft user UUID') END; + END; + +CREATE TRIGGER default_minecraft_capes_user_uuid_update_check + BEFORE UPDATE ON default_minecraft_capes FOR EACH ROW + BEGIN + SELECT CASE WHEN NOT EXISTS ( + SELECT 1 FROM minecraft_users WHERE uuid = NEW.minecraft_user_uuid + ) THEN RAISE(ABORT, 'Cannot change a default cape to refer to an unknown Minecraft user UUID') END; + END; + +CREATE TRIGGER default_minecraft_capes_user_uuid_update_cascade + AFTER UPDATE OF uuid ON minecraft_users FOR EACH ROW + BEGIN + UPDATE default_minecraft_capes SET minecraft_user_uuid = NEW.uuid WHERE minecraft_user_uuid = OLD.uuid; + END; + CREATE TABLE custom_minecraft_skins ( minecraft_user_uuid TEXT NOT NULL, texture_key TEXT NOT NULL, @@ -14,12 +37,33 @@ CREATE TABLE custom_minecraft_skins ( cape_id TEXT, PRIMARY KEY (minecraft_user_uuid, texture_key, variant, cape_id), - FOREIGN KEY (minecraft_user_uuid) REFERENCES minecraft_users(uuid) - ON DELETE CASCADE ON UPDATE CASCADE, FOREIGN KEY (texture_key) REFERENCES custom_minecraft_skin_textures(texture_key) ON DELETE CASCADE ON UPDATE CASCADE DEFERRABLE INITIALLY DEFERRED ); +-- Similar partial foreign key emulation as above +CREATE TRIGGER custom_minecraft_skins_user_uuid_insert_check + BEFORE INSERT ON custom_minecraft_skins FOR EACH ROW + BEGIN + SELECT CASE WHEN NOT EXISTS ( + SELECT 1 FROM minecraft_users WHERE uuid = NEW.minecraft_user_uuid + ) THEN RAISE(ABORT, 'Cannot add a custom skin for an unknown Minecraft user UUID') END; + END; + +CREATE TRIGGER custom_minecraft_skins_user_uuid_update_check + BEFORE UPDATE ON custom_minecraft_skins FOR EACH ROW + BEGIN + SELECT CASE WHEN NOT EXISTS ( + SELECT 1 FROM minecraft_users WHERE uuid = NEW.minecraft_user_uuid + ) THEN RAISE(ABORT, 'Cannot change a custom skin to refer to an unknown Minecraft user UUID') END; + END; + +CREATE TRIGGER custom_minecraft_skins_user_uuid_update_cascade + AFTER UPDATE OF uuid ON minecraft_users FOR EACH ROW + BEGIN + UPDATE custom_minecraft_skins SET minecraft_user_uuid = NEW.uuid WHERE minecraft_user_uuid = OLD.uuid; + END; + CREATE TABLE custom_minecraft_skin_textures ( texture_key TEXT NOT NULL, texture PNG BLOB NOT NULL, diff --git a/packages/app-lib/src/api/settings.rs b/packages/app-lib/src/api/settings.rs index 75e34d33c..761959683 100644 --- a/packages/app-lib/src/api/settings.rs +++ b/packages/app-lib/src/api/settings.rs @@ -24,6 +24,8 @@ pub async fn set(settings: Settings) -> crate::Result<()> { #[tracing::instrument] pub async fn cancel_directory_change() -> crate::Result<()> { + // This is called to handle state initialization errors due to folder migrations + // failing, so fetching a DB connection pool from `State::get` is not reliable here let pool = crate::state::db::connect().await?; let mut settings = Settings::get(&pool).await?; diff --git a/packages/app-lib/src/state/db.rs b/packages/app-lib/src/state/db.rs index 387d381f2..f386eaeb9 100644 --- a/packages/app-lib/src/state/db.rs +++ b/packages/app-lib/src/state/db.rs @@ -36,5 +36,33 @@ pub(crate) async fn connect() -> crate::Result> { sqlx::migrate!().run(&pool).await?; + if let Err(err) = stale_data_cleanup(&pool).await { + tracing::warn!( + "Failed to clean up stale data from state database: {err}" + ); + } + Ok(pool) } + +/// Cleans up data from the database that is no longer referenced, but must be +/// kept around for a little while to allow users to recover from accidental +/// deletions. +async fn stale_data_cleanup(pool: &Pool) -> crate::Result<()> { + let mut tx = pool.begin().await?; + + sqlx::query!( + "DELETE FROM default_minecraft_capes WHERE minecraft_user_uuid NOT IN (SELECT uuid FROM minecraft_users)" + ) + .execute(&mut *tx) + .await?; + sqlx::query!( + "DELETE FROM custom_minecraft_skins WHERE minecraft_user_uuid NOT IN (SELECT uuid FROM minecraft_users)" + ) + .execute(&mut *tx) + .await?; + + tx.commit().await?; + + Ok(()) +}