From 978be0b4a9a904a2ffbd227821cf8f14cf4e4243 Mon Sep 17 00:00:00 2001 From: BlackDex Date: Tue, 22 Sep 2020 12:13:02 +0200 Subject: [PATCH] Fixed foreign-key (mariadb) errors. When using MariaDB v10.5+ Foreign-Key errors were popping up because of some changes in that version. To mitigate this on MariaDB and other MySQL forks those errors are now catched, and instead of a replace_into an update will happen. I have tested this as thorough as possible with MariaDB 10.5, 10.4, 10.3 and the default MySQL on Ubuntu Focal. And tested it again using sqlite, all seems to be ok on all tables. resolves #1081. resolves #1065, resolves #1050 --- src/db/models/attachment.rs | 15 +++++++++-- src/db/models/cipher.rs | 21 +++++++++++---- src/db/models/collection.rs | 49 ++++++++++++++++++++++++++++------- src/db/models/folder.rs | 30 +++++++++++++++------ src/db/models/org_policy.rs | 17 +++++++++--- src/db/models/organization.rs | 41 ++++++++++++++++++++++------- src/db/models/two_factor.rs | 19 +++++++++++--- src/db/models/user.rs | 23 ++++++++++++---- 8 files changed, 170 insertions(+), 45 deletions(-) diff --git a/src/db/models/attachment.rs b/src/db/models/attachment.rs index 5cff557..f89a6db 100644 --- a/src/db/models/attachment.rs +++ b/src/db/models/attachment.rs @@ -63,10 +63,21 @@ impl Attachment { pub fn save(&self, conn: &DbConn) -> EmptyResult { db_run! { conn: sqlite, mysql { - diesel::replace_into(attachments::table) + match diesel::replace_into(attachments::table) .values(AttachmentDb::to_db(self)) .execute(conn) - .map_res("Error saving attachment") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(attachments::table) + .filter(attachments::id.eq(&self.id)) + .set(AttachmentDb::to_db(self)) + .execute(conn) + .map_res("Error saving attachment") + } + Err(e) => Err(e.into()), + }.map_res("Error saving attachment") } postgresql { let value = AttachmentDb::to_db(self); diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index 32d8a27..0536d46 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -194,14 +194,25 @@ impl Cipher { pub fn save(&mut self, conn: &DbConn) -> EmptyResult { self.update_users_revision(conn); self.updated_at = Utc::now().naive_utc(); - - db_run! { conn: + + db_run! { conn: sqlite, mysql { - diesel::replace_into(ciphers::table) + match diesel::replace_into(ciphers::table) .values(CipherDb::to_db(self)) .execute(conn) - .map_res("Error saving cipher") - } + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(ciphers::table) + .filter(ciphers::uuid.eq(&self.uuid)) + .set(CipherDb::to_db(self)) + .execute(conn) + .map_res("Error saving cipher") + } + Err(e) => Err(e.into()), + }.map_res("Error saving cipher") + } postgresql { let value = CipherDb::to_db(self); diesel::insert_into(ciphers::table) diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index c773b9b..888523f 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -67,12 +67,23 @@ impl Collection { pub fn save(&self, conn: &DbConn) -> EmptyResult { self.update_users_revision(conn); - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(collections::table) + match diesel::replace_into(collections::table) .values(CollectionDb::to_db(self)) .execute(conn) - .map_res("Error saving collection") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(collections::table) + .filter(collections::uuid.eq(&self.uuid)) + .set(CollectionDb::to_db(self)) + .execute(conn) + .map_res("Error saving collection") + } + Err(e) => Err(e.into()), + }.map_res("Error saving collection") } postgresql { let value = CollectionDb::to_db(self); @@ -82,7 +93,7 @@ impl Collection { .do_update() .set(&value) .execute(conn) - .map_res("Error saving collection") + .map_res("Error saving collection") } } } @@ -245,9 +256,9 @@ impl CollectionUser { pub fn save(user_uuid: &str, collection_uuid: &str, read_only: bool, hide_passwords: bool, conn: &DbConn) -> EmptyResult { User::update_uuid_revision(&user_uuid, conn); - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(users_collections::table) + match diesel::replace_into(users_collections::table) .values(( users_collections::user_uuid.eq(user_uuid), users_collections::collection_uuid.eq(collection_uuid), @@ -255,7 +266,24 @@ impl CollectionUser { users_collections::hide_passwords.eq(hide_passwords), )) .execute(conn) - .map_res("Error adding user to collection") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(users_collections::table) + .filter(users_collections::user_uuid.eq(user_uuid)) + .filter(users_collections::collection_uuid.eq(collection_uuid)) + .set(( + users_collections::user_uuid.eq(user_uuid), + users_collections::collection_uuid.eq(collection_uuid), + users_collections::read_only.eq(read_only), + users_collections::hide_passwords.eq(hide_passwords), + )) + .execute(conn) + .map_res("Error adding user to collection") + } + Err(e) => Err(e.into()), + }.map_res("Error adding user to collection") } postgresql { diesel::insert_into(users_collections::table) @@ -344,8 +372,11 @@ impl CollectionCipher { pub fn save(cipher_uuid: &str, collection_uuid: &str, conn: &DbConn) -> EmptyResult { Self::update_users_revision(&collection_uuid, conn); - db_run! { conn: + db_run! { conn: sqlite, mysql { + // Not checking for ForeignKey Constraints here. + // Table ciphers_collections does not have ForeignKey Constraints which would cause conflicts. + // This table has no constraints pointing to itself, but only to others. diesel::replace_into(ciphers_collections::table) .values(( ciphers_collections::cipher_uuid.eq(cipher_uuid), @@ -370,7 +401,7 @@ impl CollectionCipher { pub fn delete(cipher_uuid: &str, collection_uuid: &str, conn: &DbConn) -> EmptyResult { Self::update_users_revision(&collection_uuid, conn); - + db_run! { conn: { diesel::delete( ciphers_collections::table diff --git a/src/db/models/folder.rs b/src/db/models/folder.rs index 5ff72b7..3656afb 100644 --- a/src/db/models/folder.rs +++ b/src/db/models/folder.rs @@ -74,12 +74,23 @@ impl Folder { User::update_uuid_revision(&self.user_uuid, conn); self.updated_at = Utc::now().naive_utc(); - db_run! { conn: - sqlite, mysql { - diesel::replace_into(folders::table) + db_run! { conn: + sqlite, mysql { + match diesel::replace_into(folders::table) .values(FolderDb::to_db(self)) .execute(conn) - .map_res("Error saving folder") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(folders::table) + .filter(folders::uuid.eq(&self.uuid)) + .set(FolderDb::to_db(self)) + .execute(conn) + .map_res("Error saving folder") + } + Err(e) => Err(e.into()), + }.map_res("Error saving folder") } postgresql { let value = FolderDb::to_db(self); @@ -98,7 +109,7 @@ impl Folder { User::update_uuid_revision(&self.user_uuid, conn); FolderCipher::delete_all_by_folder(&self.uuid, &conn)?; - + db_run! { conn: { diesel::delete(folders::table.filter(folders::uuid.eq(&self.uuid))) .execute(conn) @@ -136,8 +147,11 @@ impl Folder { impl FolderCipher { pub fn save(&self, conn: &DbConn) -> EmptyResult { - db_run! { conn: + db_run! { conn: sqlite, mysql { + // Not checking for ForeignKey Constraints here. + // Table folders_ciphers does not have ForeignKey Constraints which would cause conflicts. + // This table has no constraints pointing to itself, but only to others. diesel::replace_into(folders_ciphers::table) .values(FolderCipherDb::to_db(self)) .execute(conn) @@ -149,9 +163,9 @@ impl FolderCipher { .on_conflict((folders_ciphers::cipher_uuid, folders_ciphers::folder_uuid)) .do_nothing() .execute(conn) - .map_res("Error adding cipher to folder") + .map_res("Error adding cipher to folder") } - } + } } pub fn delete(self, conn: &DbConn) -> EmptyResult { diff --git a/src/db/models/org_policy.rs b/src/db/models/org_policy.rs index a58ca53..fa18ac9 100644 --- a/src/db/models/org_policy.rs +++ b/src/db/models/org_policy.rs @@ -56,12 +56,23 @@ impl OrgPolicy { /// Database methods impl OrgPolicy { pub fn save(&self, conn: &DbConn) -> EmptyResult { - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(org_policies::table) + match diesel::replace_into(org_policies::table) .values(OrgPolicyDb::to_db(self)) .execute(conn) - .map_res("Error saving org_policy") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(org_policies::table) + .filter(org_policies::uuid.eq(&self.uuid)) + .set(OrgPolicyDb::to_db(self)) + .execute(conn) + .map_res("Error saving org_policy") + } + Err(e) => Err(e.into()), + }.map_res("Error saving org_policy") } postgresql { let value = OrgPolicyDb::to_db(self); diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 6a4012d..5378ad2 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -204,12 +204,24 @@ impl Organization { User::update_uuid_revision(&user_org.user_uuid, conn); }); - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(organizations::table) + match diesel::replace_into(organizations::table) .values(OrganizationDb::to_db(self)) .execute(conn) - .map_res("Error saving organization") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(organizations::table) + .filter(organizations::uuid.eq(&self.uuid)) + .set(OrganizationDb::to_db(self)) + .execute(conn) + .map_res("Error saving organization") + } + Err(e) => Err(e.into()), + }.map_res("Error saving organization") + } postgresql { let value = OrganizationDb::to_db(self); @@ -219,7 +231,7 @@ impl Organization { .do_update() .set(&value) .execute(conn) - .map_res("Error saving organization") + .map_res("Error saving organization") } } } @@ -259,7 +271,7 @@ impl Organization { impl UserOrganization { pub fn to_json(&self, conn: &DbConn) -> Value { let org = Organization::find_by_uuid(&self.org_uuid, conn).unwrap(); - + json!({ "Id": self.org_uuid, "Name": org.name, @@ -343,12 +355,23 @@ impl UserOrganization { pub fn save(&self, conn: &DbConn) -> EmptyResult { User::update_uuid_revision(&self.user_uuid, conn); - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(users_organizations::table) + match diesel::replace_into(users_organizations::table) .values(UserOrganizationDb::to_db(self)) .execute(conn) - .map_res("Error adding user to organization") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(users_organizations::table) + .filter(users_organizations::uuid.eq(&self.uuid)) + .set(UserOrganizationDb::to_db(self)) + .execute(conn) + .map_res("Error adding user to organization") + } + Err(e) => Err(e.into()), + }.map_res("Error adding user to organization") } postgresql { let value = UserOrganizationDb::to_db(self); @@ -358,7 +381,7 @@ impl UserOrganization { .do_update() .set(&value) .execute(conn) - .map_res("Error adding user to organization") + .map_res("Error adding user to organization") } } } diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index a1b925c..79a35c1 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -71,12 +71,23 @@ impl TwoFactor { /// Database methods impl TwoFactor { pub fn save(&self, conn: &DbConn) -> EmptyResult { - db_run! { conn: + db_run! { conn: sqlite, mysql { - diesel::replace_into(twofactor::table) + match diesel::replace_into(twofactor::table) .values(TwoFactorDb::to_db(self)) .execute(conn) - .map_res("Error saving twofactor") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(twofactor::table) + .filter(twofactor::uuid.eq(&self.uuid)) + .set(TwoFactorDb::to_db(self)) + .execute(conn) + .map_res("Error saving twofactor") + } + Err(e) => Err(e.into()), + }.map_res("Error saving twofactor") } postgresql { let value = TwoFactorDb::to_db(self); @@ -93,7 +104,7 @@ impl TwoFactor { .do_update() .set(&value) .execute(conn) - .map_res("Error saving twofactor") + .map_res("Error saving twofactor") } } } diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 471e09e..23154d6 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -175,10 +175,21 @@ impl User { db_run! {conn: sqlite, mysql { - diesel::replace_into(users::table) // Insert or update - .values(&UserDb::to_db(self)) + match diesel::replace_into(users::table) + .values(UserDb::to_db(self)) .execute(conn) - .map_res("Error saving user") + { + Ok(_) => Ok(()), + // Record already exists and causes a Foreign Key Violation because replace_into() wants to delete the record first. + Err(diesel::result::Error::DatabaseError(diesel::result::DatabaseErrorKind::ForeignKeyViolation, _)) => { + diesel::update(users::table) + .filter(users::uuid.eq(&self.uuid)) + .set(UserDb::to_db(self)) + .execute(conn) + .map_res("Error saving user") + } + Err(e) => Err(e.into()), + }.map_res("Error saving user") } postgresql { let value = UserDb::to_db(self); @@ -290,10 +301,12 @@ impl Invitation { db_run! {conn: sqlite, mysql { + // Not checking for ForeignKey Constraints here + // Table invitations does not have any ForeignKey Constraints. diesel::replace_into(invitations::table) .values(InvitationDb::to_db(self)) .execute(conn) - .map_res("Error saving invitation") + .map_res("Error saving invitation") } postgresql { diesel::insert_into(invitations::table) @@ -301,7 +314,7 @@ impl Invitation { .on_conflict(invitations::email) .do_nothing() .execute(conn) - .map_res("Error saving invitation") + .map_res("Error saving invitation") } } }