RUST: New kara IDs are created in the repo_kara table in an atomic way. We...

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

@@ -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)]

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

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,23 +130,22 @@ impl LktDatabaseConnection {
         arg_kara_id: i64,
     ) -> LktDatabaseResult<()> {
         self.sqlite.exclusive_transaction(|c| {
-            let local_id = with_dsl!(repo_kara => repo_kara
+            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
-            );
-            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)?);
+                    .local_kara_id;
+                diesel::delete(repo_kara.filter(local_kara_id.eq(local_id))).execute(c)?;
                 Ok(())
             })
+        })
     }
 
     /// Delete a kara by its local ID.
     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,

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!(