MediaGroup: Rework media count update triggers

To avoid a situation in which after updating a media type, the
decrement trigger would run, then the empty group deletion trigger
would kick in, causing the increment trigger to fail.

Fix #247

src/MediaGroup.cpp

@@ -412,9 +412,7 @@ void MediaGroup::createTriggers( sqlite::Connection* connection )
     sqlite::Tools::executeRequest( connection,
                                    trigger( Triggers::DeleteFts, Settings::DbModelVersion ) );
     sqlite::Tools::executeRequest( connection,
-                                   trigger( Triggers::IncrementNbMediaOnGroupChange, Settings::DbModelVersion ) );
-    sqlite::Tools::executeRequest( connection,
-                                   trigger( Triggers::DecrementNbMediaOnGroupChange, Settings::DbModelVersion ) );
+                                   trigger( Triggers::UpdateNbMedia, Settings::DbModelVersion ) );
     sqlite::Tools::executeRequest( connection,
                                    trigger( Triggers::DecrementNbMediaOnDeletion, Settings::DbModelVersion ) );
     sqlite::Tools::executeRequest( connection,
@@ -498,6 +496,8 @@ std::string MediaGroup::trigger( MediaGroup::Triggers t, uint32_t dbModel )
                        " WHERE rowid = old.id_group;"
                    " END";
         case Triggers::IncrementNbMediaOnGroupChange:
+        {
+            assert( dbModel < 26 );
             return "CREATE TRIGGER " + triggerName( t, dbModel ) +
                     " AFTER UPDATE OF type, group_id ON " + Media::Table::Name +
                     " WHEN new.group_id IS NOT NULL AND"
@@ -522,7 +522,10 @@ std::string MediaGroup::trigger( MediaGroup::Triggers t, uint32_t dbModel )
                         " last_modification_date = strftime('%s')"
                     " WHERE id_group = new.group_id;"
                     " END";
+        }
         case Triggers::DecrementNbMediaOnGroupChange:
+        {
+            assert( dbModel < 26 );
             return "CREATE TRIGGER " + triggerName( t, dbModel ) +
                     " AFTER UPDATE OF type, group_id ON " + Media::Table::Name +
                     " WHEN old.group_id IS NOT NULL AND"
@@ -547,6 +550,55 @@ std::string MediaGroup::trigger( MediaGroup::Triggers t, uint32_t dbModel )
                         " last_modification_date = strftime('%s')"
                     " WHERE id_group = old.group_id;"
                     " END";
+        }
+        case Triggers::UpdateNbMedia:
+        {
+            assert( dbModel >= 26 );
+            return "CREATE TRIGGER " + triggerName( t, dbModel ) +
+                    " AFTER UPDATE OF type, group_id ON " + Media::Table::Name +
+                        " WHEN IFNULL(old.group_id, 0) != IFNULL(new.group_id, 0) OR"
+                        " old.type != new.type"
+                    " BEGIN"
+                    // Handle increment
+                    " UPDATE " + Table::Name + " SET"
+                        " nb_video = nb_video + "
+                            "(CASE new.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Video ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " nb_audio = nb_audio + "
+                            "(CASE new.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Audio ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " nb_unknown = nb_unknown + "
+                            "(CASE new.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Unknown ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " last_modification_date = strftime('%s')"
+                    " WHERE new.group_id IS NOT NULL AND id_group = new.group_id;"
+                    // Handle decrement
+                    " UPDATE " + Table::Name + " SET"
+                        " nb_video = nb_video - "
+                            "(CASE old.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Video ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " nb_audio = nb_audio - "
+                            "(CASE old.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Audio ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " nb_unknown = nb_unknown - "
+                            "(CASE old.type WHEN " +
+                                std::to_string( static_cast<std::underlying_type_t<IMedia::Type>>(
+                                                    IMedia::Type::Unknown ) ) +
+                                " THEN 1 ELSE 0 END),"
+                        " last_modification_date = strftime('%s')"
+                    " WHERE old.group_id IS NOT NULL AND id_group = old.group_id;"
+                    " END";
+        }
         case Triggers::DecrementNbMediaOnDeletion:
             return "CREATE TRIGGER " + triggerName( t, dbModel ) +
                    " AFTER DELETE ON " + Media::Table::Name +
@@ -627,8 +679,10 @@ std::string MediaGroup::triggerName(MediaGroup::Triggers t, uint32_t dbModel)
         case Triggers::DeleteFts:
             return "media_group_delete_fts";
         case Triggers::IncrementNbMediaOnGroupChange:
+            assert( dbModel < 26 );
             return "media_group_increment_nb_media";
         case Triggers::DecrementNbMediaOnGroupChange:
+            assert( dbModel < 26 );
             return "media_group_decrement_nb_media";
         case Triggers::DecrementNbMediaOnDeletion:
             return "media_group_decrement_nb_media_on_deletion";
@@ -644,6 +698,9 @@ std::string MediaGroup::triggerName(MediaGroup::Triggers t, uint32_t dbModel)
         case Triggers::UpdateDurationOnMediaDeletion:
             assert( dbModel >= 25 );
             return "media_group_update_duration_on_media_deletion";
+        case Triggers::UpdateNbMedia:
+            assert( dbModel >= 26 );
+            return "media_group_update_nb_media";
         default:
             assert( !"Invalid trigger" );
     }
@@ -724,8 +781,7 @@ bool MediaGroup::checkDbModel( MediaLibraryPtr ml )
 
     return check( ml->getConn(), Triggers::InsertFts ) &&
             check( ml->getConn(), Triggers::DeleteFts ) &&
-            check( ml->getConn(), Triggers::IncrementNbMediaOnGroupChange ) &&
-            check( ml->getConn(), Triggers::DecrementNbMediaOnGroupChange ) &&
+            check( ml->getConn(), Triggers::UpdateNbMedia ) &&
             check( ml->getConn(), Triggers::DecrementNbMediaOnDeletion ) &&
             check( ml->getConn(), Triggers::DeleteEmptyGroups ) &&
             check( ml->getConn(), Triggers::RenameForcedSingleton ) &&

src/MediaGroup.h

@@ -48,13 +48,14 @@ public:
     {
         InsertFts,
         DeleteFts,
-        IncrementNbMediaOnGroupChange,
-        DecrementNbMediaOnGroupChange,
+        IncrementNbMediaOnGroupChange, // Deprecated in model 26
+        DecrementNbMediaOnGroupChange, // Deprecated in model 26
         DecrementNbMediaOnDeletion,
         DeleteEmptyGroups,
         RenameForcedSingleton,
         UpdateDurationOnMediaChange,
         UpdateDurationOnMediaDeletion,
+        UpdateNbMedia,
     };
     enum class Indexes : uint8_t
     {

src/database/SqliteConnection.cpp

@@ -30,6 +30,8 @@
 
 #include <cstring>
 
+#define DEBUG_SQLITE_TRIGGERS 0
+
 namespace medialibrary
 {
 namespace sqlite
@@ -94,7 +96,13 @@ Connection::Handle Connection::handle()
         // Should solve `Failed to run request <DELETE FROM File WHERE id_file = ?>: disk I/O error(6410)`
         setPragma( dbConnection, "temp_store", "2" );
 #endif
-
+#if DEBUG_SQLITE_TRIGGERS
+        sqlite3_trace_v2( dbConnection, SQLITE_TRACE_STMT, [](unsigned int, void* , void* , void* x) {
+            const char* str = static_cast<const char*>( x );
+            LOG_ERROR( "Executed: ", str );
+            return 0;
+        }, nullptr );
+#endif
         m_conns.emplace( compat::this_thread::get_id(), std::move( dbConn ) );
         sqlite3_update_hook( dbConnection, &updateHook, this );
         static thread_local ThreadSpecificConnection tsc( shared_from_this() );

src/database/migrations/migration25-26.sql

@@ -28,3 +28,8 @@ Show::schema( Show::FtsTable::Name, 26 ),
 
 Show::trigger( Show::Triggers::InsertFts, 26 ),
 Show::trigger( Show::Triggers::DeleteFts, 26 ),
+
+/* Update MediaGroup triggers */
+"DROP TRIGGER " + MediaGroup::triggerName( MediaGroup::Triggers::IncrementNbMediaOnGroupChange, 25 ),
+"DROP TRIGGER " + MediaGroup::triggerName( MediaGroup::Triggers::DecrementNbMediaOnGroupChange, 25 ),
+MediaGroup::trigger( MediaGroup::Triggers::UpdateNbMedia, 26 ),

test/unittest/MediaGroupTests.cpp

@@ -282,6 +282,33 @@ TEST_F( MediaGroups, UpdateNbMediaTypeChange )
     ASSERT_EQ( 0u, group2->nbUnknown() );
 }
 
+TEST_F( MediaGroups, UpdateNbMediaNoDelete )
+{
+    auto group1 = ml->createMediaGroup( "group" );
+    ASSERT_NE( nullptr, group1 );
+    ASSERT_EQ( 0u, group1->nbAudio() );
+    ASSERT_EQ( 0u, group1->nbVideo() );
+    ASSERT_EQ( 0u, group1->nbUnknown() );
+
+    // Insert an unknown media in a group
+    // Also insert a media for each group, to avoid their count to reach 0 which
+    // would cause the group to be deleted
+    auto m = ml->addMedia( "media.mkv", IMedia::Type::Unknown );
+    group1->add( *m );
+
+    ASSERT_EQ( 0u, group1->nbAudio() );
+    ASSERT_EQ( 0u, group1->nbVideo() );
+    ASSERT_EQ( 1u, group1->nbUnknown() );
+
+    // Now change the media type
+    m->setType( IMedia::Type::Audio );
+
+    group1 = std::static_pointer_cast<MediaGroup>( ml->mediaGroup( group1->id() ) );
+    ASSERT_EQ( 1u, group1->nbAudio() );
+    ASSERT_EQ( 0u, group1->nbVideo() );
+    ASSERT_EQ( 0u, group1->nbUnknown() );
+}
+
 TEST_F( MediaGroups, SortByNbMedia )
 {
     auto mg1 = ml->createMediaGroup( "A group" );

test/unittest/MiscTests.cpp

@@ -67,15 +67,14 @@ namespace
         "insert_folder_fts", "insert_genre_fts", "insert_media_fts",
         "insert_playlist_fts", "insert_show_fts",
         "media_cascade_file_deletion",
-        "media_group_decrement_nb_media",
         "media_group_decrement_nb_media_on_deletion",
         "media_group_delete_empty_group",
         "media_group_delete_fts",
-        "media_group_increment_nb_media",
         "media_group_insert_fts",
         "media_group_rename_forced_singleton",
         "media_group_update_duration_on_media_change",
         "media_group_update_duration_on_media_deletion",
+        "media_group_update_nb_media",
         "media_update_device_presence",
         "show_decrement_nb_episode",
         "show_increment_nb_episode",