From 783203e71ec8aca71904bd8e19d22110f3d9b210 Mon Sep 17 00:00:00 2001 From: CrystalP Date: Sun, 21 Apr 2024 15:26:12 -0400 Subject: [videodb] Fix Movie/Asset Removal Add Transaction Add missing deletions (art, stream details, path invalidation) and rename RemoveVideoVersion and its parameter for consistency. Nested transactions are not supported by sqllite, mysql and mariadb. To keep things simple, the caller ("owner" of the open transaction) is responsible for the transaction management. --- xbmc/video/VideoDatabase.cpp | 64 +++++++++++++++++----- xbmc/video/VideoDatabase.h | 2 +- xbmc/video/dialogs/GUIDialogVideoManager.cpp | 3 +- xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp | 2 +- .../dialogs/GUIDialogVideoManagerVersions.cpp | 2 +- 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/xbmc/video/VideoDatabase.cpp b/xbmc/video/VideoDatabase.cpp index 1372e54173..af8cdf582c 100644 --- a/xbmc/video/VideoDatabase.cpp +++ b/xbmc/video/VideoDatabase.cpp @@ -3672,19 +3672,38 @@ void CVideoDatabase::DeleteMovie(int idMovie, // the ancillary tables are still purged if (!bKeepId) { - std::string path = GetSingleValue(PrepareSQL("SELECT strPath FROM path JOIN files ON files.idPath=path.idPath WHERE files.idFile=%i", idFile)); + const std::string path = GetSingleValue(PrepareSQL( + "SELECT strPath FROM path JOIN files ON files.idPath=path.idPath WHERE files.idFile=%i", + idFile)); if (!path.empty()) InvalidatePathHash(path); - std::string strSQL = PrepareSQL("delete from movie where idMovie=%i", idMovie); + const std::string strSQL = PrepareSQL("delete from movie where idMovie=%i", idMovie); m_pDS->exec(strSQL); if (ca == DeleteMovieCascadeAction::ALL_ASSETS) { - const std::string strSQL{ - PrepareSQL("DELETE FROM videoversion WHERE idMedia = %i AND media_type = '%s'", idMovie, - MediaTypeMovie)}; - m_pDS->exec(strSQL); + // The default version of the movie was removed by a delete trigger. + // Clean up the other assets attached to the movie, if any. + + // need local dataset due to nested DeleteVideoAsset query + const std::unique_ptr pDS{m_pDB->CreateDataset()}; + + pDS->query( + PrepareSQL("SELECT idFile FROM videoversion WHERE idMedia=%i AND media_type='%s'", + idMovie, MediaTypeMovie)); + + while (!pDS->eof()) + { + if (!DeleteVideoAsset(pDS->fv(0).get_asInt())) + { + RollbackTransaction(); + pDS->close(); + return; + } + pDS->next(); + } + pDS->close(); } } @@ -3697,7 +3716,7 @@ void CVideoDatabase::DeleteMovie(int idMovie, } catch (...) { - CLog::Log(LOGERROR, "{} failed", __FUNCTION__); + CLog::LogF(LOGERROR, "failed"); RollbackTransaction(); } } @@ -12401,31 +12420,48 @@ bool CVideoDatabase::IsDefaultVideoVersion(int idFile) return false; } -bool CVideoDatabase::RemoveVideoVersion(int dbId) +bool CVideoDatabase::DeleteVideoAsset(int idFile) { if (!m_pDB || !m_pDS) return false; - if (IsDefaultVideoVersion(dbId)) + if (IsDefaultVideoVersion(idFile)) return false; + const bool inTransaction{m_pDB->in_transaction()}; + try { - std::string path = GetSingleValue(PrepareSQL( + if (!inTransaction) + BeginTransaction(); + + const std::string path = GetSingleValue(PrepareSQL( "SELECT strPath FROM path JOIN files ON files.idPath=path.idPath WHERE files.idFile=%i", - dbId)); + idFile)); if (!path.empty()) InvalidatePathHash(path); - m_pDS->exec(PrepareSQL("DELETE FROM videoversion WHERE idFile = %i", dbId)); + /*! \todo replace with a delete trigger on videoversion */ + m_pDS->exec(PrepareSQL("DELETE FROM art WHERE media_id=%i and media_type='%s'", idFile, + MediaTypeVideoVersion)); + + /*! \todo replace with a delete trigger on videoversion */ + DeleteStreamDetails(idFile); + + m_pDS->exec(PrepareSQL("DELETE FROM videoversion WHERE idFile=%i", idFile)); + + if (!inTransaction) + CommitTransaction(); return true; } catch (...) { - CLog::Log(LOGERROR, "{} failed for {}", __FUNCTION__, dbId); + CLog::LogF(LOGERROR, "failed for {}", idFile); + if (!inTransaction) + RollbackTransaction(); + return false; } - return false; } void CVideoDatabase::SetVideoVersion(int idFile, int idVideoVersion) diff --git a/xbmc/video/VideoDatabase.h b/xbmc/video/VideoDatabase.h index aa0bee416d..9366e0729d 100644 --- a/xbmc/video/VideoDatabase.h +++ b/xbmc/video/VideoDatabase.h @@ -1074,7 +1074,7 @@ public: int dbId, int idVideoVersion, CFileItem& item); - bool RemoveVideoVersion(int dbId); + bool DeleteVideoAsset(int idFile); bool IsDefaultVideoVersion(int idFile); bool GetVideoVersionTypes(VideoDbContentType idContent, VideoAssetType asset, diff --git a/xbmc/video/dialogs/GUIDialogVideoManager.cpp b/xbmc/video/dialogs/GUIDialogVideoManager.cpp index 92cb8b3f8c..4f6d865a96 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManager.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManager.cpp @@ -309,8 +309,7 @@ void CGUIDialogVideoManager::Remove() return; } - //! @todo db refactor: should not be version, but asset - m_database.RemoveVideoVersion(m_selectedVideoAsset->GetVideoInfoTag()->m_iDbId); + m_database.DeleteVideoAsset(m_selectedVideoAsset->GetVideoInfoTag()->m_iDbId); // refresh data and controls Refresh(); diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp index 28bae13d04..2564bf4fd1 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp @@ -217,7 +217,7 @@ bool CGUIDialogVideoManagerExtras::AddVideoExtra() else { // @todo: should be in a database transaction with the addition as a new asset below - if (!m_database.RemoveVideoVersion(newAsset.m_idFile)) + if (!m_database.DeleteVideoAsset(newAsset.m_idFile)) return false; } } diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp index f2dada7087..2faa7baffc 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp @@ -586,7 +586,7 @@ bool CGUIDialogVideoManagerVersions::AddVideoVersionFilePicker() else { // @todo: should be in a database transaction with the addition as a new asset below - if (!m_database.RemoveVideoVersion(newAsset.m_idFile)) + if (!m_database.DeleteVideoAsset(newAsset.m_idFile)) return false; } } -- cgit v1.2.3 From a16cef728957692e7b36203f58df15a8e2bec705 Mon Sep 17 00:00:00 2001 From: CrystalP Date: Thu, 25 Apr 2024 23:50:15 -0400 Subject: [videodb] Add Transaction and Rename AddVideoVersion --- xbmc/video/VideoDatabase.cpp | 35 ++++++++-------------- xbmc/video/VideoDatabase.h | 18 ++++------- xbmc/video/VideoInfoScanner.cpp | 5 ++-- xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp | 2 +- .../dialogs/GUIDialogVideoManagerVersions.cpp | 2 +- 5 files changed, 23 insertions(+), 39 deletions(-) diff --git a/xbmc/video/VideoDatabase.cpp b/xbmc/video/VideoDatabase.cpp index af8cdf582c..cab8998b44 100644 --- a/xbmc/video/VideoDatabase.cpp +++ b/xbmc/video/VideoDatabase.cpp @@ -12480,31 +12480,17 @@ void CVideoDatabase::SetVideoVersion(int idFile, int idVideoVersion) } } -void CVideoDatabase::AddPrimaryVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - CFileItem& item) -{ - AddVideoVersion(itemType, dbId, idVideoVersion, VideoAssetType::VERSION, item); -} - -void CVideoDatabase::AddExtrasVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - CFileItem& item) -{ - AddVideoVersion(itemType, dbId, idVideoVersion, VideoAssetType::EXTRA, item); -} - -void CVideoDatabase::AddVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - VideoAssetType videoAssetType, - CFileItem& item) +void CVideoDatabase::AddVideoAsset(VideoDbContentType itemType, + int dbId, + int idVideoVersion, + VideoAssetType videoAssetType, + CFileItem& item) { if (!m_pDB || !m_pDS) return; + assert(m_pDB->in_transaction() == false); + MediaType mediaType; if (itemType == VideoDbContentType::MOVIES) { @@ -12519,6 +12505,8 @@ void CVideoDatabase::AddVideoVersion(VideoDbContentType itemType, try { + BeginTransaction(); + m_pDS->query(PrepareSQL("SELECT idFile FROM videoversion WHERE idFile = %i", idFile)); if (m_pDS->num_rows() == 0) @@ -12534,10 +12522,13 @@ void CVideoDatabase::AddVideoVersion(VideoDbContentType itemType, if (videoAssetType == VideoAssetType::VERSION) SetVideoVersionDefaultArt(idFile, item.GetVideoInfoTag()->m_iDbId, itemType); + + CommitTransaction(); } catch (...) { - CLog::Log(LOGERROR, "{} failed for video {}", __FUNCTION__, dbId); + CLog::LogF(LOGERROR, "failed for video {}", dbId); + RollbackTransaction(); } } diff --git a/xbmc/video/VideoDatabase.h b/xbmc/video/VideoDatabase.h index 9366e0729d..e9cb3ed8c0 100644 --- a/xbmc/video/VideoDatabase.h +++ b/xbmc/video/VideoDatabase.h @@ -1061,19 +1061,11 @@ public: int AddVideoVersionType(const std::string& typeVideoVersion, VideoAssetTypeOwner owner, VideoAssetType assetType); - void AddVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - VideoAssetType videoAssetType, - CFileItem& item); - void AddPrimaryVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - CFileItem& item); - void AddExtrasVideoVersion(VideoDbContentType itemType, - int dbId, - int idVideoVersion, - CFileItem& item); + void AddVideoAsset(VideoDbContentType itemType, + int dbId, + int idVideoVersion, + VideoAssetType videoAssetType, + CFileItem& item); bool DeleteVideoAsset(int idFile); bool IsDefaultVideoVersion(int idFile); bool GetVideoVersionTypes(VideoDbContentType idContent, diff --git a/xbmc/video/VideoInfoScanner.cpp b/xbmc/video/VideoInfoScanner.cpp index aca4162fd5..79b4d1ffc6 100644 --- a/xbmc/video/VideoInfoScanner.cpp +++ b/xbmc/video/VideoInfoScanner.cpp @@ -2484,8 +2484,9 @@ namespace VIDEO const int idVideoVersion = m_database.AddVideoVersionType( typeVideoVersion, VideoAssetTypeOwner::AUTO, VideoAssetType::EXTRA); - m_database.AddExtrasVideoVersion(ContentToVideoDbType(content), dbId, idVideoVersion, - *item.get()); + m_database.AddVideoAsset(ContentToVideoDbType(content), dbId, idVideoVersion, + VideoAssetType::EXTRA, *item.get()); + CLog::Log(LOGDEBUG, "VideoInfoScanner: Added video extras {}", CURL::GetRedacted(item->GetPath())); }, diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp index 2564bf4fd1..c0e78b9e69 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp @@ -238,7 +238,7 @@ bool CGUIDialogVideoManagerExtras::AddVideoExtra() if (idNewVideoVersion == -1) return false; - m_database.AddExtrasVideoVersion(itemType, dbId, idNewVideoVersion, item); + m_database.AddVideoAsset(itemType, dbId, idNewVideoVersion, VideoAssetType::EXTRA, item); return true; } diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp index 2faa7baffc..eb55add00c 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp @@ -605,7 +605,7 @@ bool CGUIDialogVideoManagerVersions::AddVideoVersionFilePicker() if (idNewVideoVersion == -1) return false; - m_database.AddPrimaryVideoVersion(itemType, dbId, idNewVideoVersion, item); + m_database.AddVideoAsset(itemType, dbId, idNewVideoVersion, VideoAssetType::VERSION, item); return true; } -- cgit v1.2.3 From 4cc89d47cab88dfad1a2af8406e45d316ad90b5f Mon Sep 17 00:00:00 2001 From: CrystalP Date: Fri, 26 Apr 2024 01:42:54 -0400 Subject: [video] Remove Unnecessary Video Version/Extra Removal in Update Case AddVideoAsset is called later and follows the pattern of Add* functions able to "upsert", making a Delete* unnecessary for an update (and detrimental for artwork in this case). --- xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp | 6 ------ xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp | 6 ------ 2 files changed, 12 deletions(-) diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp index c0e78b9e69..6fc39e830e 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp @@ -214,12 +214,6 @@ bool CGUIDialogVideoManagerExtras::AddVideoExtra() return m_database.ConvertVideoToVersion(itemType, newAsset.m_idMedia, dbId, idNewVideoVersion, VideoAssetType::EXTRA); } - else - { - // @todo: should be in a database transaction with the addition as a new asset below - if (!m_database.DeleteVideoAsset(newAsset.m_idFile)) - return false; - } } CFileItem item{path, false}; diff --git a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp index eb55add00c..6cad4b0760 100644 --- a/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp +++ b/xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp @@ -583,12 +583,6 @@ bool CGUIDialogVideoManagerVersions::AddVideoVersionFilePicker() return false; } } - else - { - // @todo: should be in a database transaction with the addition as a new asset below - if (!m_database.DeleteVideoAsset(newAsset.m_idFile)) - return false; - } } CFileItem item{path, false}; -- cgit v1.2.3