aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCrystalP <crystalp@kodi.tv>2024-07-14 11:42:23 -0400
committerGitHub <noreply@github.com>2024-07-14 11:42:23 -0400
commitf9515f2bd9dba219bc070035b98976e9d280b714 (patch)
treea37acb5a044a1456bfd3f7f0208e1753771e2505
parentafaeb388740551d196fe89db4f341c6c6ba4c24b (diff)
parent4cc89d47cab88dfad1a2af8406e45d316ad90b5f (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.cpp99
-rw-r--r--xbmc/video/VideoDatabase.h20
-rw-r--r--xbmc/video/VideoInfoScanner.cpp5
-rw-r--r--xbmc/video/dialogs/GUIDialogVideoManager.cpp3
-rw-r--r--xbmc/video/dialogs/GUIDialogVideoManagerExtras.cpp8
-rw-r--r--xbmc/video/dialogs/GUIDialogVideoManagerVersions.cpp8
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;
}