diff options
author | CrystalP <crystalp@kodi.tv> | 2024-07-14 11:42:23 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-14 11:42:23 -0400 |
commit | f9515f2bd9dba219bc070035b98976e9d280b714 (patch) | |
tree | a37acb5a044a1456bfd3f7f0208e1753771e2505 | |
parent | afaeb388740551d196fe89db4f341c6c6ba4c24b (diff) | |
parent | 4cc89d47cab88dfad1a2af8406e45d316ad90b5f (diff) |
Merge pull request #25462 from CrystalP/bp-fix-versiondelete
[backport][videodb][videoversion] Fix Leaks on Movie / Asset Removal
-rw-r--r-- | xbmc/video/VideoDatabase.cpp | 99 | ||||
-rw-r--r-- | xbmc/video/VideoDatabase.h | 20 | ||||
-rw-r--r-- | xbmc/video/VideoInfoScanner.cpp | 5 | ||||
-rw-r--r-- | xbmc/video/dialogs/GUIDialogVideoManager.cpp | 3 | ||||
-rw-r--r-- | xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp | 8 | ||||
-rw-r--r-- | xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp | 8 |
6 files changed, 75 insertions, 68 deletions
diff --git a/xbmc/video/VideoDatabase.cpp b/xbmc/video/VideoDatabase.cpp index 1372e54173..cab8998b44 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<Dataset> 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) @@ -12444,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) { @@ -12483,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) @@ -12498,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 aa0bee416d..e9cb3ed8c0 100644 --- a/xbmc/video/VideoDatabase.h +++ b/xbmc/video/VideoDatabase.h @@ -1061,20 +1061,12 @@ 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); - bool RemoveVideoVersion(int dbId); + void AddVideoAsset(VideoDbContentType itemType, + int dbId, + int idVideoVersion, + VideoAssetType videoAssetType, + CFileItem& item); + bool DeleteVideoAsset(int idFile); bool IsDefaultVideoVersion(int idFile); bool GetVideoVersionTypes(VideoDbContentType idContent, VideoAssetType asset, 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/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..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.RemoveVideoVersion(newAsset.m_idFile)) - return false; - } } CFileItem item{path, false}; @@ -238,7 +232,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 f2dada7087..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.RemoveVideoVersion(newAsset.m_idFile)) - return false; - } } CFileItem item{path, false}; @@ -605,7 +599,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; } |