Merge pull request xbmc#25919 from CrystalP/fix-videodbcreation

[videodb] Fix Nested Transactions - Video DB Creation/Upgrade, PVR Stop
jjd-uk · Nov 7, 2024 · f9ea21c · f9ea21c
2 parents 0ef1eb6 + c060e20
commit f9ea21c
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 25 deletions.
diff --git 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
@@ -208,6 +208,7 @@ void CVideoDatabase::CreateTables()
   CLog::Log(LOGINFO, "create videoversiontype table");
   m_pDS->exec("CREATE TABLE videoversiontype (id INTEGER PRIMARY KEY, name TEXT, owner INTEGER, "
               "itemType INTEGER)");
+  CLog::Log(LOGINFO, "populate videoversiontype table");
   InitializeVideoVersionTypeTable(GetSchemaVersion());
 
   CLog::Log(LOGINFO, "create videoversion table");
@@ -3020,42 +3021,38 @@ 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;
   }
   catch (...)
   {
     CLog::Log(LOGERROR, "{} ({}) failed", __FUNCTION__, idEpisode);
   }
-  RollbackTransaction();
   return false;
 }
 
 bool CVideoDatabase::SetFileForMovie(const std::string& fileAndPath, int idMovie, int idFile)
 {
   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;
   }
   catch (...)
   {
     CLog::Log(LOGERROR, "{} ({}) failed", __FUNCTION__, idMovie);
   }
-  RollbackTransaction();
   return false;
 }
 
@@ -3255,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
   {
@@ -3312,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;
 }
 
 //********************************************************************************************************************************
@@ -12063,10 +12066,10 @@ std::string CVideoDatabase::GetVideoItemTitle(VideoDbContentType itemType, int d
 
 void CVideoDatabase::InitializeVideoVersionTypeTable(int schemaVersion)
 {
+  assert(m_pDB->in_transaction());
+
   try
   {
-    BeginTransaction();
-
     for (int id = VIDEO_VERSION_ID_BEGIN; id <= VIDEO_VERSION_ID_END; ++id)
     {
       // Exclude removed pre-populated "quality" values
@@ -12087,13 +12090,11 @@ void CVideoDatabase::InitializeVideoVersionTypeTable(int schemaVersion)
             type.c_str(), VideoAssetTypeOwner::SYSTEM, VideoAssetType::VERSION));
       }
     }
-
-    CommitTransaction();
   }
   catch (...)
   {
-    CLog::Log(LOGERROR, "{} failed", __FUNCTION__);
-    RollbackTransaction();
+    CLog::LogF(LOGERROR, "failed");
+    throw;
   }
 }
 

diff --git a/xbmc/video/VideoDatabase.h b/xbmc/video/VideoDatabase.h
@@ -602,8 +602,9 @@ class CVideoDatabase : public CDatabase
    * \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,