Commit 9dd52bc2 authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen

Album: Simplify artists management

Drop AlbumArtistRelation table and only use the AlbumTrack table.
This reduces the database size, simplifies the code, and increase the
similarity between unittests & actual metadata parsing handling
parent 43e5e0f9
......@@ -53,11 +53,13 @@ public:
virtual Query<IMedia> tracks( GenrePtr genre, const QueryParameters* params = nullptr ) const = 0;
/**
* @brief albumArtist Returns the album main artist (generally tagged as album-artist)
* This can be an artist that doesn't appear on the album, and is solely dependent
* on the most present AlbumArtist tag for all of this album's tracks
*/
virtual ArtistPtr albumArtist() const = 0;
/**
* @brief artists Returns a Query object representing all additional
* artists appearing on the album.
* @brief artists Returns a Query object representing all artists appearing
* on at least one track for this album.
* Artists are sorted by name.
* @param desc
*/
......
......@@ -390,36 +390,19 @@ bool Album::setAlbumArtist( std::shared_ptr<Artist> artist )
Query<IArtist> Album::artists( const QueryParameters* params ) const
{
std::string req = "FROM " + Artist::Table::Name + " art "
"INNER JOIN AlbumArtistRelation aar ON aar.artist_id = art.id_artist "
"WHERE aar.album_id = ?";
std::string orderBy = "ORDER BY art.name";
"INNER JOIN " + AlbumTrack::Table::Name + " att "
"ON att.artist_id = art.id_artist "
"WHERE att.album_id = ?";
if ( params != nullptr && ( params->sort != SortingCriteria::Alpha &&
params->sort != SortingCriteria::Default ) )
LOG_WARN( "Unsupported sorting criteria, falling back to SortingCriteria::Alpha" );
LOG_WARN( "Unsupported sorting criteria, falling back to SortingCriteria::Alpha" );
std::string orderBy = "GROUP BY art.id_artist ORDER BY art.name";
if ( params != nullptr && params->desc == true )
orderBy += " DESC";
return make_query<Artist, IArtist>( m_ml, "art.*", std::move( req ),
std::move( orderBy ), m_id );
}
bool Album::addArtist( std::shared_ptr<Artist> artist )
{
static const std::string req = "INSERT OR IGNORE INTO AlbumArtistRelation VALUES(?, ?)";
if ( m_id == 0 || artist->id() == 0 )
{
LOG_ERROR("Both artist & album need to be inserted in database before being linked together" );
return false;
}
return sqlite::Tools::executeInsert( m_ml->getConn(), req, m_id, artist->id() ) != 0;
}
bool Album::removeArtist(Artist* artist)
{
static const std::string req = "DELETE FROM AlbumArtistRelation WHERE album_id = ? "
"AND id_artist = ?";
return sqlite::Tools::executeDelete( m_ml->getConn(), req, m_id, artist->id() );
}
void Album::createTable( sqlite::Connection* dbConnection )
{
const std::string req = "CREATE TABLE IF NOT EXISTS " +
......@@ -439,15 +422,6 @@ void Album::createTable( sqlite::Connection* dbConnection )
"FOREIGN KEY(thumbnail_id) REFERENCES " + Thumbnail::Table::Name
+ "(id_thumbnail)"
")";
const std::string reqRel = "CREATE TABLE IF NOT EXISTS AlbumArtistRelation("
"album_id INTEGER,"
"artist_id INTEGER,"
"PRIMARY KEY (album_id, artist_id),"
"FOREIGN KEY(album_id) REFERENCES " + Table::Name + "("
+ Table::PrimaryKeyColumn + ") ON DELETE CASCADE,"
"FOREIGN KEY(artist_id) REFERENCES " + Artist::Table::Name + "("
+ Artist::Table::PrimaryKeyColumn + ") ON DELETE CASCADE"
")";
const std::string vtableReq = "CREATE VIRTUAL TABLE IF NOT EXISTS "
+ Table::Name + "Fts USING FTS3("
"title,"
......@@ -455,7 +429,6 @@ void Album::createTable( sqlite::Connection* dbConnection )
")";
sqlite::Tools::executeRequest( dbConnection, req );
sqlite::Tools::executeRequest( dbConnection, reqRel );
sqlite::Tools::executeRequest( dbConnection, vtableReq );
}
......
......@@ -94,8 +94,6 @@ class Album : public IAlbum, public DatabaseHelpers<Album>
virtual ArtistPtr albumArtist() const override;
bool setAlbumArtist( std::shared_ptr<Artist> artist );
virtual Query<IArtist> artists( const QueryParameters* params ) const override;
bool addArtist( std::shared_ptr<Artist> artist );
bool removeArtist( Artist* artist );
virtual Query<IMedia> searchTracks( const std::string& pattern,
const QueryParameters* params = nullptr ) const override;
......
......@@ -232,6 +232,7 @@ IMedia::Type::Unknown ) ),
/******************* Delete other tables **************************************/
"DROP TABLE " + Album::Table::Name,
"DROP TABLE AlbumArtistRelation",
"DELETE FROM " + Album::Table::Name + "Fts",
"DROP TABLE " + Artist::Table::Name,
"DELETE FROM " + Artist::Table::Name + "Fts",
......
......@@ -976,7 +976,6 @@ bool MetadataAnalyzer::link( Media& media, std::shared_ptr<Album> album,
// has a different artist)
album->setAlbumArtist( albumArtist );
// Always add the album artist as an artist
album->addArtist( albumArtist );
// Always update the album artist number of tracks.
// The artist might be different, and will be handled a few lines below
albumArtist->updateNbTrack( 1 );
......@@ -986,7 +985,6 @@ bool MetadataAnalyzer::link( Media& media, std::shared_ptr<Album> album,
// album artist track count as well.
if ( albumArtist->id() != artist->id() )
artist->updateNbTrack( 1 );
album->addArtist( artist );
}
}
else
......@@ -1009,14 +1007,9 @@ bool MetadataAnalyzer::link( Media& media, std::shared_ptr<Album> album,
{
m_variousArtists->updateNbTrack( 1 );
}
// Add this artist as "featuring".
album->addArtist( albumArtist );
}
if ( artist != nullptr && artist->id() != albumArtist->id() )
{
album->addArtist( artist );
artist->updateNbTrack( 1 );
}
albumArtist->updateNbTrack( 1 );
}
......
......@@ -224,40 +224,29 @@ TEST_F( Albums, Artists )
ASSERT_NE( artist1, nullptr );
ASSERT_NE( artist2, nullptr );
auto res = album->addArtist( artist1 );
ASSERT_EQ( res, true );
res = album->addArtist( artist2 );
ASSERT_EQ( res, true );
auto artists = album->artists( nullptr )->all();
ASSERT_EQ( artists.size(), 2u );
Reload();
album = std::static_pointer_cast<Album>( ml->album( album->id() ) );
artists = album->artists( nullptr )->all();
ASSERT_EQ( album->albumArtist(), nullptr );
ASSERT_EQ( artists.size(), 2u );
}
TEST_F( Albums, SortArtists )
{
auto album = ml->createAlbum( "album" );
auto artist1 = ml->createArtist( "john" );
auto artist2 = ml->createArtist( "doe" );
auto m1 = std::static_pointer_cast<Media>( ml->addMedia( "media1.mp3" ) );
album->addTrack( m1, 1, 0, artist1->id(), nullptr );
m1->save();
album->addArtist( artist1 );
album->addArtist( artist2 );
auto m2 = std::static_pointer_cast<Media>( ml->addMedia( "media2.mp3" ) );
album->addTrack( m2, 2, 0, artist2->id(), nullptr );
m2->save();
QueryParameters params { SortingCriteria::Default, false };
auto artists = album->artists( &params )->all();
auto query = album->artists( &params );
ASSERT_EQ( 2u, query->count() );
auto artists = query->all();
ASSERT_EQ( artists.size(), 2u );
ASSERT_EQ( artist1->id(), artists[1]->id() );
ASSERT_EQ( artist2->id(), artists[0]->id() );
Reload();
params.desc = true;
artists = album->artists( &params )->all();
album = std::static_pointer_cast<Album>( ml->album( album->id() ) );
query = album->artists( &params );
ASSERT_EQ( 2u, query->count() );
artists = query->all();
ASSERT_EQ( artists.size(), 2u );
ASSERT_EQ( artist1->id(), artists[0]->id() );
ASSERT_EQ( artist2->id(), artists[1]->id() );
......
Markdown is supported
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