From 278cd8fff688fd01886bee330e595bf42a585956 Mon Sep 17 00:00:00 2001
From: Kubat <mael.martin31@gmail.com>
Date: Tue, 24 Jan 2023 12:45:37 +0100
Subject: [PATCH] RUST: New kara IDs are created in the repo_kara table in an
 atomic way.  We will no longer have race conditions on kara id creations.
 Moreover to delete a kara, we now only need to delete it from the repo_kara
 table

---
 src/rust/liblektor-rs/kurisu_api/src/v1.rs    | 12 ++---
 .../2022-09-30-204512_initial/up.sql          |  6 +--
 .../liblektor-rs/lektor_db/src/connexion.rs   | 48 ++++++++++---------
 src/rust/liblektor-rs/lektor_db/src/schema.rs |  1 -
 4 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/src/rust/liblektor-rs/kurisu_api/src/v1.rs b/src/rust/liblektor-rs/kurisu_api/src/v1.rs
index 438ab112..b91ba4ea 100644
--- a/src/rust/liblektor-rs/kurisu_api/src/v1.rs
+++ b/src/rust/liblektor-rs/kurisu_api/src/v1.rs
@@ -2,20 +2,20 @@ use serde::Deserialize;
 
 #[derive(Debug, Deserialize)]
 pub struct Kara<'a> {
-    pub id: u64,
+    pub id: i64,
     pub source_name: &'a str,
     pub song_name: &'a str,
     pub song_type: &'a str,
-    pub song_number: u64,
+    pub song_number: i64,
     pub category: &'a str,
     pub language: &'a str,
     pub author_name: &'a str,
     pub author_year: &'a str,
-    pub is_new: u64,
+    pub is_new: i64,
     pub upload_comment: &'a str,
-    pub popularity: u64,
-    pub unix_timestamp: u64,
-    pub size: u64,
+    pub popularity: i64,
+    pub unix_timestamp: i64,
+    pub size: i64,
 }
 
 #[derive(Debug, Deserialize)]
diff --git a/src/rust/liblektor-rs/lektor_db/migrations/2022-09-30-204512_initial/up.sql b/src/rust/liblektor-rs/lektor_db/migrations/2022-09-30-204512_initial/up.sql
index b75c9f72..0817dea3 100644
--- a/src/rust/liblektor-rs/lektor_db/migrations/2022-09-30-204512_initial/up.sql
+++ b/src/rust/liblektor-rs/lektor_db/migrations/2022-09-30-204512_initial/up.sql
@@ -9,15 +9,15 @@ CREATE TABLE repo
 -- Local IDs are all uniques and every client should be expected to have
 -- different ones.
 CREATE TABLE repo_kara
-  ( repo_id       BIGINT NOT NULL REFERENCES repo(id)
+  ( repo_id       BIGINT NOT NULL REFERENCES repo(id) ON DELETE CASCADE
   , repo_kara_id  BIGINT NOT NULL
-  , local_kara_id BIGINT NOT NULL REFERENCES kara(id)
+  , local_kara_id BIGINT NOT NULL
   , PRIMARY KEY (repo_id, repo_kara_id, local_kara_id)
   );
 
 -- A kara entry in the databse.
 CREATE TABLE kara
-  ( id          BIGINT  NOT NULL PRIMARY KEY
+  ( id          BIGINT  NOT NULL PRIMARY KEY REFERENCES repo(local_kara_id) ON DELETE CASCADE
   , is_dl       BOOLEAN NOT NULL DEFAULT false
   , song_title  TEXT    NOT NULL
   , song_type   TEXT    NOT NULL
diff --git a/src/rust/liblektor-rs/lektor_db/src/connexion.rs b/src/rust/liblektor-rs/lektor_db/src/connexion.rs
index fdd7a0c7..8829bb5b 100644
--- a/src/rust/liblektor-rs/lektor_db/src/connexion.rs
+++ b/src/rust/liblektor-rs/lektor_db/src/connexion.rs
@@ -107,15 +107,20 @@ impl LktDatabaseConnection {
         }}))
     }
 
-    /// Get a free local id for all karas. Note that using this function might
-    /// be unsafe because there is no guarenties that the returned ID will be
-    /// free by the time a kara is inserted with the said id...
-    #[deprecated(note = "We must create a new id in a given database to avoid race conditions")]
-    fn get_kara_new_local_id(&mut self) -> LktDatabaseResult<i64> {
-        Ok(with_dsl!(kara => kara.select(max(id))
-            .first::<Option<i64>>(&mut self.sqlite)?
-            .unwrap_or_default() + 1
-        ))
+    /// Create a new local id for a kara in a distant repo.
+    fn new_kara_id(&mut self, rid: i64, rkid: i64) -> LktDatabaseResult<i64> {
+        self.sqlite.exclusive_transaction(|conn| {
+            with_dsl!(repo_kara => {
+                let local_id = repo_kara
+                    .select(max(local_kara_id))
+                    .first::<Option<i64>>(conn)?
+                    .unwrap_or_default() + 1;
+                diesel::insert_into(repo_kara)
+                    .values(KaraId { repo_id: rid, repo_kara_id: rkid, local_kara_id: local_id })
+                    .execute(conn)?;
+                Ok(local_id)
+            })
+        })
     }
 
     /// Delete a kara with its id in a repo.
@@ -125,15 +130,15 @@ impl LktDatabaseConnection {
         arg_kara_id: i64,
     ) -> LktDatabaseResult<()> {
         self.sqlite.exclusive_transaction(|c| {
-            let local_id = with_dsl!(repo_kara => repo_kara
-                .filter(repo_id.eq(arg_repo_id))
-                .filter(repo_kara_id.eq(arg_kara_id))
-                .first::<KaraId>(c)?
-                .local_kara_id
-            );
-            with_dsl!(repo_kara => diesel::delete(repo_kara.filter(local_kara_id.eq(local_id))).execute(c)?);
-            with_dsl!(kara      => diesel::delete(kara.filter(id.eq(local_id))).execute(c)?);
-            Ok(())
+            with_dsl!(repo_kara => {
+                let local_id = repo_kara
+                    .filter(repo_id.eq(arg_repo_id))
+                    .filter(repo_kara_id.eq(arg_kara_id))
+                    .first::<KaraId>(c)?
+                    .local_kara_id;
+                diesel::delete(repo_kara.filter(local_kara_id.eq(local_id))).execute(c)?;
+                Ok(())
+            })
         })
     }
 
@@ -141,7 +146,6 @@ impl LktDatabaseConnection {
     pub fn delete_kara_by_local_id(&mut self, local_id: i64) -> LktDatabaseResult<()> {
         self.sqlite.exclusive_transaction(|c| {
             with_dsl!(repo_kara => diesel::delete(repo_kara.filter(local_kara_id.eq(local_id))).execute(c)?);
-            with_dsl!(kara      => diesel::delete(kara.filter(id.eq(local_id))).execute(c)?);
             Ok(())
         })
     }
@@ -191,11 +195,11 @@ impl LktDatabaseConnection {
         repo_id: i64,
         kara: api_v1::Kara<'a>,
     ) -> LktDatabaseResult<NewKaraRequest<'a>> {
-        let local_id = self.get_kara_new_local_id()?;
+        let local_id = self.new_kara_id(repo_id, kara.id)?;
         let id = KaraId {
             repo_id,
             local_kara_id: local_id,
-            repo_kara_id: i64::try_from(kara.id)?,
+            repo_kara_id: kara.id,
         };
         let lang = NewLanguage::from(kara.get_language());
         let kara_maker = vec![NewKaraMaker {
@@ -205,7 +209,7 @@ impl LktDatabaseConnection {
         let tags = vec![AddKaraTag {
             kara_id: local_id,
             tag_id: self.get_tag_id_by_name("number")?,
-            value: KaraTagValue::Integer(i64::try_from(kara.song_number)?),
+            value: KaraTagValue::Integer(kara.song_number),
         }];
         let kara = NewKara {
             id: local_id,
diff --git a/src/rust/liblektor-rs/lektor_db/src/schema.rs b/src/rust/liblektor-rs/lektor_db/src/schema.rs
index 977a01e8..82791c51 100644
--- a/src/rust/liblektor-rs/lektor_db/src/schema.rs
+++ b/src/rust/liblektor-rs/lektor_db/src/schema.rs
@@ -97,7 +97,6 @@ diesel::joinable!(kara_tag -> kara (kara_id));
 diesel::joinable!(kara_tag -> tag (tag_id));
 diesel::joinable!(playlist_kara -> kara (kara_id));
 diesel::joinable!(playlist_kara -> playlist (playlist_id));
-diesel::joinable!(repo_kara -> kara (local_kara_id));
 diesel::joinable!(repo_kara -> repo (repo_id));
 
 diesel::allow_tables_to_appear_in_same_query!(
-- 
GitLab