Commit 1b0cc786 authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen
Browse files

Genre: Rely on a trigger to update the number of tracks

Instead of manually updating when assigning a track to the genre.
This allows us to update the number of tracks automatically when a media
gets converted to external, mainly following a removal of the associated
entrypoint.
Refs #432
parent 3eca5758
......@@ -83,11 +83,6 @@ uint32_t Genre::nbPresentTracks() const
bool Genre::updateNbTracks( int increment )
{
static const std::string req = "UPDATE " + Table::Name +
" SET nb_tracks = nb_tracks + ?1,"
" is_present = is_present + ?1 WHERE id_genre = ?2";
if ( sqlite::Tools::executeUpdate( m_ml->getConn(), req, increment, m_id ) == false )
return false;
m_nbTracks += increment;
m_nbPresentTracks += increment;
return true;
......@@ -216,6 +211,9 @@ void Genre::createTriggers( sqlite::Connection* dbConn )
trigger( Triggers::UpdateIsPresent, Settings::DbModelVersion ) );
sqlite::Tools::executeRequest( dbConn,
trigger( Triggers::DeleteEmpty, Settings::DbModelVersion ) );
sqlite::Tools::executeRequest( dbConn,
trigger( Triggers::UpdateOnMediaGenreIdChange,
Settings::DbModelVersion ) );
}
std::string Genre::schema( const std::string& tableName, uint32_t dbModel )
......@@ -359,6 +357,28 @@ std::string Genre::trigger( Triggers trigger, uint32_t dbModel )
" WHERE id_genre = old.id_genre;"
" END";
}
case Triggers::UpdateOnMediaGenreIdChange:
{
assert( dbModel >= 36 );
return "CREATE TRIGGER " + triggerName( trigger, dbModel ) +
" AFTER UPDATE OF genre_id ON " + Media::Table::Name +
" WHEN IFNULL(new.genre_id, 0) != IFNULL(old.genre_id, 0)"
" BEGIN"
/* Decrement the old genre first */
" UPDATE " + Table::Name +
" SET is_present = is_present -"
" IIF(old.is_present != 0, 1, 0),"
" nb_tracks = nb_tracks - 1"
" WHERE old.genre_id IS NOT NULL AND id_genre = old.genre_id;"
/* And increment the new one afterward */
" UPDATE " + Table::Name +
" SET is_present = is_present +"
" IIF(old.is_present != 0, 1, 0),"
" nb_tracks = nb_tracks + 1"
" WHERE new.genre_id IS NOT NULL AND id_genre = new.genre_id;"
" END";
}
default:
assert( !"Invalid trigger provided" );
}
......@@ -386,6 +406,9 @@ std::string Genre::triggerName( Triggers trigger, uint32_t dbModel )
case Triggers::DeleteEmpty:
assert( dbModel >= 34 );
return "genre_delete_empty";
case Triggers::UpdateOnMediaGenreIdChange:
assert( dbModel >= 36 );
return "genre_update_on_media_genre_id_change";
default:
assert( !"Invalid trigger provided" );
}
......@@ -412,7 +435,8 @@ bool Genre::checkDbModel(MediaLibraryPtr ml)
check( Triggers::DeleteFts ) &&
check( Triggers::UpdateOnTrackDelete ) &&
check( Triggers::UpdateIsPresent ) &&
check( Triggers::DeleteEmpty );
check( Triggers::DeleteEmpty ) &&
check( Triggers::UpdateOnMediaGenreIdChange );
}
std::shared_ptr<Genre> Genre::create( MediaLibraryPtr ml, std::string name )
......
......@@ -51,6 +51,7 @@ public:
UpdateOnTrackDelete,
UpdateIsPresent,
DeleteEmpty, // Introduced in model 34
UpdateOnMediaGenreIdChange, // Introduced in model 36
};
Genre( MediaLibraryPtr ml, sqlite::Row& row );
......
......@@ -2052,6 +2052,13 @@ void MediaLibrary::migrateModel35to36()
sqlite::Connection::WeakDbContext weakConnCtx{ dbConn };
auto t = dbConn->newTransaction();
std::string reqs[] = {
# include "database/migrations/migration35-36.sql"
};
for ( const auto& req : reqs )
sqlite::Tools::executeRequest( dbConn, req );
m_settings.setDbModelVersion( 36 );
t->commit();
}
......
Genre::trigger( Genre::Triggers::UpdateOnMediaGenreIdChange, 36 ),
......@@ -65,6 +65,7 @@ namespace
"folder_update_nb_media_on_media_update",
"genre_delete_empty",
"genre_update_is_present",
"genre_update_on_media_genre_id_change",
"genre_update_on_track_deleted",
"incr_thumbnail_refcount",
"increment_media_nb_playlist", "insert_album_fts", "insert_artist_fts",
......
......@@ -779,6 +779,30 @@ static void GenrePresence( DeviceFsTests* T )
ASSERT_EQ( 3u, genre->nbTracks() );
ASSERT_EQ( 1u, genre->nbPresentTracks() );
/*
* Convert the missing track to external and check that nb_tracks gets updated
* but is_present doesn't
*/
auto deviceId = media1->deviceId();
auto folderId = media1->folderId();
auto res = media1->convertToExternal();
ASSERT_TRUE( res );
genre = std::static_pointer_cast<Genre>( T->ml->genre( genre->id() ) );
ASSERT_EQ( 2u, genre->nbTracks() );
ASSERT_EQ( 0u, genre->nbPresentTracks() );
res = media1->markAsInternal( IMedia::Type::Audio, media1->duration(),
deviceId, folderId );
ASSERT_TRUE( res );
res = media1->markAsAlbumTrack( album->id(), 1, 1, 0, genre.get() );
ASSERT_TRUE( res );
genre = std::static_pointer_cast<Genre>( T->ml->genre( genre->id() ) );
ASSERT_EQ( 3u, genre->nbTracks() );
ASSERT_EQ( 1u, genre->nbPresentTracks() );
/* Remove the present track */
T->ml->deleteMedia( media1->id() );
......
......@@ -449,6 +449,69 @@ static void GetThumbnails( GenreTests* T )
ASSERT_EQ( 1u, T->ml->countNbThumbnails() );
}
static void ConvertToExternal( GenreTests* T )
{
ASSERT_EQ( 0u, T->g->nbTracks() );
auto extraGenre = T->ml->createGenre( "Progressive Otter Metal" );
ASSERT_NON_NULL( extraGenre );
auto genres = T->ml->genres( nullptr )->all();
ASSERT_EQ( 2u, genres.size() );
auto a = T->ml->createAlbum( "album" );
auto m = std::static_pointer_cast<Media>(
T->ml->addMedia( "track.mp3", IMedia::Type::Audio ) );
auto m2 = std::static_pointer_cast<Media>(
T->ml->addMedia( "track2.mp3", IMedia::Type::Audio ) );
auto res = a->addTrack( m, 1, 1, 0, T->g.get() );
ASSERT_TRUE( res );
res = a->addTrack( m2, 2, 1, 0, T->g.get() );
ASSERT_TRUE( res );
ASSERT_EQ( 2u, T->g->nbTracks() );
T->g = std::static_pointer_cast<Genre>( T->ml->genre( T->g->id() ) );
ASSERT_EQ( 2u, T->g->nbTracks() );
ASSERT_EQ( 2u, T->g->nbPresentTracks() );
auto deviceId = m->deviceId();
auto folderId = m->folderId();
res = m->convertToExternal();
ASSERT_TRUE( res );
T->g = std::static_pointer_cast<Genre>( T->ml->genre( T->g->id() ) );
ASSERT_EQ( 1u, T->g->nbTracks() );
ASSERT_EQ( 1u, T->g->nbPresentTracks() );
res = m->markAsInternal( IMedia::Type::Audio, m->duration(), deviceId, folderId );
ASSERT_TRUE( res );
/*
* The swich to internal in itself doesn't add the genre back. Outside of a
* test configuration, a switch back to internal is followed by a refresh
* for the media.
* Here, we need to simulate this
*/
T->g = std::static_pointer_cast<Genre>( T->ml->genre( T->g->id() ) );
ASSERT_EQ( 1u, T->g->nbTracks() );
ASSERT_EQ( 1u, T->g->nbPresentTracks() );
res = m->markAsAlbumTrack( a->id(), 1, 1, 0, T->g.get() );
ASSERT_TRUE( res );
T->g = std::static_pointer_cast<Genre>( T->ml->genre( T->g->id() ) );
ASSERT_EQ( 2u, T->g->nbTracks() );
ASSERT_EQ( 2u, T->g->nbPresentTracks() );
res = m->convertToExternal();
ASSERT_TRUE( res );
res = m2->convertToExternal();
ASSERT_TRUE( res );
T->g = std::static_pointer_cast<Genre>( T->ml->genre( T->g->id() ) );
ASSERT_EQ( nullptr, T->g );
}
int main( int ac, char** av )
{
INIT_TESTS_C( GenreTests );
......@@ -470,6 +533,7 @@ int main( int ac, char** av )
ADD_TEST( WithThumbnail );
ADD_TEST( CheckDbModel );
ADD_TEST( GetThumbnails );
ADD_TEST( ConvertToExternal );
END_TESTS
}
......@@ -474,6 +474,7 @@ genre_tests = [
'WithThumbnail',
'CheckDbModel',
'GetThumbnails',
'ConvertToExternal',
]
foreach t : genre_tests
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment