From c060e207c8a2cbc5251a257571e18e182a73804d Mon Sep 17 00:00:00 2001
From: CrystalP <crystalp@kodi.tv>
Date: Sun, 3 Nov 2024 15:23:12 -0500
Subject: [PATCH] [videodb] Remove nested transaction when saving state after
 stopping PVR playback

More work is needed to fix CSaveFileState::DoWork() error/transaction handling and restore data
integrity. Only the immediate area of the issue was addressed in this commit.
---
 xbmc/utils/SaveFileStateJob.cpp | 31 +++++++++++++++++++++++--------
 xbmc/video/VideoDatabase.cpp    | 22 ++++++++++++----------
 xbmc/video/VideoDatabase.h      |  3 ++-
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/xbmc/utils/SaveFileStateJob.cpp b/xbmc/utils/SaveFileStateJob.cpp
index be2b6c7c88..a771896e65 100644
--- a/xbmc/utils/SaveFileStateJob.cpp
+++ b/xbmc/utils/SaveFileStateJob.cpp
@@ -86,7 +86,9 @@ void CSaveFileState::DoWork(CFileItem& item,
       }
       else
       {
+        //! @todo check possible failure of BeginTransaction
         videodatabase.BeginTransaction();
+        bool videoDbSuccess{true};
 
         if (URIUtils::IsPlugin(progressTrackingFile) && !(item.HasVideoInfoTag() && item.GetVideoInfoTag()->m_iDbId >= 0))
         {
@@ -105,6 +107,7 @@ void CSaveFileState::DoWork(CFileItem& item,
         // No resume & watched status for livetv
         if (!item.IsLiveTV())
         {
+          //! @todo handle db failures to maintain data integrity
           if (updatePlayCount)
           {
             // no watched for not yet finished pvr recordings
@@ -182,17 +185,29 @@ void CSaveFileState::DoWork(CFileItem& item,
           {
             const int idFile = videodatabase.SetStreamDetailsForFile(
                 item.GetVideoInfoTag()->m_streamDetails, progressTrackingFile);
-            if (item.GetVideoContentType() == VideoDbContentType::MOVIES)
-              videodatabase.SetFileForMovie(item.GetDynPath(), item.GetVideoInfoTag()->m_iDbId,
-                                            idFile);
-            else if (item.GetVideoContentType() == VideoDbContentType::EPISODES)
-              videodatabase.SetFileForEpisode(item.GetDynPath(), item.GetVideoInfoTag()->m_iDbId,
-                                              idFile);
-            updateListing = true;
+
+            if (idFile == -2)
+            {
+              videoDbSuccess = false;
+            }
+            else if (idFile > 0)
+            {
+              updateListing = true;
+
+              if (item.GetVideoContentType() == VideoDbContentType::MOVIES)
+                videoDbSuccess = videodatabase.SetFileForMovie(
+                    item.GetDynPath(), item.GetVideoInfoTag()->m_iDbId, idFile);
+              else if (item.GetVideoContentType() == VideoDbContentType::EPISODES)
+                videoDbSuccess = videodatabase.SetFileForEpisode(
+                    item.GetDynPath(), item.GetVideoInfoTag()->m_iDbId, idFile);
+            }
           }
         }
 
-        videodatabase.CommitTransaction();
+        if (videoDbSuccess)
+          videodatabase.CommitTransaction();
+        else
+          videodatabase.RollbackTransaction();
 
         if (updateListing)
         {
diff --git a/xbmc/video/VideoDatabase.cpp b/xbmc/video/VideoDatabase.cpp
index 747b21a8f2..8923224163 100644
--- a/xbmc/video/VideoDatabase.cpp
+++ b/xbmc/video/VideoDatabase.cpp
@@ -3021,11 +3021,9 @@ bool CVideoDatabase::SetFileForEpisode(const std::string& fileAndPath, int idEpi
 {
   try
   {
-    BeginTransaction();
     std::string sql = PrepareSQL("UPDATE episode SET c18='%s', idFile=%i WHERE idEpisode=%i",
                                  fileAndPath.c_str(), idFile, idEpisode);
     m_pDS->exec(sql);
-    CommitTransaction();
 
     return true;
   }
@@ -3033,7 +3031,6 @@ bool CVideoDatabase::SetFileForEpisode(const std::string& fileAndPath, int idEpi
   {
     CLog::Log(LOGERROR, "{} ({}) failed", __FUNCTION__, idEpisode);
   }
-  RollbackTransaction();
   return false;
 }
 
@@ -3041,14 +3038,14 @@ bool CVideoDatabase::SetFileForMovie(const std::string& fileAndPath, int idMovie
 {
   try
   {
-    BeginTransaction();
+    assert(m_pDB->in_transaction());
+
     std::string sql = PrepareSQL("UPDATE movie SET c22='%s', idFile=%i WHERE idMovie=%i",
                                  fileAndPath.c_str(), idFile, idMovie);
     m_pDS->exec(sql);
     sql = PrepareSQL("UPDATE videoversion SET idFile=%i WHERE idMedia=%i AND media_type='movie'",
                      idFile, idMovie);
     m_pDS->exec(sql);
-    CommitTransaction();
 
     return true;
   }
@@ -3056,7 +3053,6 @@ bool CVideoDatabase::SetFileForMovie(const std::string& fileAndPath, int idMovie
   {
     CLog::Log(LOGERROR, "{} ({}) failed", __FUNCTION__, idMovie);
   }
-  RollbackTransaction();
   return false;
 }
 
@@ -3256,14 +3252,18 @@ int CVideoDatabase::SetStreamDetailsForFile(const CStreamDetails& details,
   int idFile = AddFile(strFileNameAndPath);
   if (idFile < 0)
     return -1;
-  SetStreamDetailsForFileId(details, idFile);
-  return idFile;
+
+  //! @todo ugly error return mechanism, fixme
+  if (SetStreamDetailsForFileId(details, idFile))
+    return idFile;
+  else
+    return -2;
 }
 
-void CVideoDatabase::SetStreamDetailsForFileId(const CStreamDetails& details, int idFile)
+bool CVideoDatabase::SetStreamDetailsForFileId(const CStreamDetails& details, int idFile)
 {
   if (idFile < 0)
-    return;
+    return false;
 
   try
   {
@@ -3313,11 +3313,13 @@ void CVideoDatabase::SetStreamDetailsForFileId(const CStreamDetails& details, in
         m_pDS->exec(sql);
       }
     }
+    return true;
   }
   catch (...)
   {
     CLog::Log(LOGERROR, "{} ({}) failed", __FUNCTION__, idFile);
   }
+  return false;
 }
 
 //********************************************************************************************************************************
diff --git a/xbmc/video/VideoDatabase.h b/xbmc/video/VideoDatabase.h
index 2c8ea6357d..526f89cd46 100644
--- a/xbmc/video/VideoDatabase.h
+++ b/xbmc/video/VideoDatabase.h
@@ -601,8 +601,9 @@ public:
    * \brief Clear any existing stream details and add the new provided details to a file.
    * \param[in] details New stream details
    * \param[in] idFile Identifier of the file
+   * \return operation success. true for success, false for failure
    */
-  void SetStreamDetailsForFileId(const CStreamDetails& details, int idFile);
+  bool SetStreamDetailsForFileId(const CStreamDetails& details, int idFile);
 
   bool SetSingleValue(VideoDbContentType type, int dbId, int dbField, const std::string& strValue);
   bool SetSingleValue(VideoDbContentType type,
-- 
2.11.4.GIT