[amarok] /: QueryMaker: fix flawed API wrt ArtistQueryMode

Matěj Laitl matej at laitl.cz
Fri Nov 16 17:13:53 UTC 2012


Git commit 05b08e333086c064a6dfa7885984547279b446be by Matěj Laitl.
Committed on 16/11/2012 at 18:04.
Pushed by laitl into branch 'master'.

QueryMaker: fix flawed API wrt ArtistQueryMode

ArtistQueryMode was not really a "mode", just a switch for
addMatch( Meta::ArtistPtr ) behaviour.

Rename it to ArtistMatchBehaviour, remove setArtistQueryMode() method
and instead add a parameter to addMatch( Meta::ArtistPtr, .. ) method
defaulting to TrackArtist as in past.

CCing amarok-devel because this is a rather intrusive API change.

Phalgun, this should help you with NepomukQueryMaker as the interface
makes more sense now.

CCMAIL: amarok-devel at kde.org

M  +4    -4    src/browsers/CollectionTreeItem.cpp
M  +5    -16   src/core-impl/collections/db/sql/SqlQueryMaker.cpp
M  +1    -2    src/core-impl/collections/db/sql/SqlQueryMaker.h
M  +1    -2    src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp
M  +8    -8    src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp
M  +1    -1    src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h
M  +2    -10   src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.cpp
M  +1    -2    src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.h
M  +1    -1    src/core-impl/collections/support/MemoryMatcher.cpp
M  +2    -2    src/core-impl/collections/support/MemoryMatcher.h
M  +2    -12   src/core-impl/collections/support/MemoryQueryMaker.cpp
M  +1    -2    src/core-impl/collections/support/MemoryQueryMaker.h
M  +0    -13   src/core-impl/collections/support/XmlQueryReader.cpp
M  +2    -1    src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp
M  +1    -1    src/core-impl/collections/upnpcollection/UpnpQueryMaker.h
M  +3    -10   src/core/collections/MetaQueryMaker.cpp
M  +1    -2    src/core/collections/MetaQueryMaker.h
M  +0    -7    src/core/collections/QueryMaker.cpp
M  +13   -14   src/core/collections/QueryMaker.h
M  +2    -1    src/services/DynamicServiceQueryMaker.cpp
M  +1    -1    src/services/DynamicServiceQueryMaker.h
M  +6    -17   src/services/ServiceSqlQueryMaker.cpp
M  +1    -2    src/services/ServiceSqlQueryMaker.h
M  +4    -3    src/services/scriptable/ScriptableServiceQueryMaker.cpp
M  +1    -1    src/services/scriptable/ScriptableServiceQueryMaker.h
M  +1    -1    tests/mocks/MockQueryMaker.h

http://commits.kde.org/amarok/05b08e333086c064a6dfa7885984547279b446be

diff --git a/src/browsers/CollectionTreeItem.cpp b/src/browsers/CollectionTreeItem.cpp
index a0d55e5..9e429a3 100644
--- a/src/browsers/CollectionTreeItem.cpp
+++ b/src/browsers/CollectionTreeItem.cpp
@@ -292,10 +292,10 @@ CollectionTreeItem::addMatch( Collections::QueryMaker *qm, CategoryId::CatMenuId
         qm->addMatch( track );
     else if( Meta::ArtistPtr artist = Meta::ArtistPtr::dynamicCast( m_data ) )
     {
-        if( levelCategory == CategoryId::AlbumArtist )
-            // this needs to be BEFORE addMatch(), because addMatch() resets ArtistQueryMode
-            qm->setArtistQueryMode( Collections::QueryMaker::AlbumArtists );
-        qm->addMatch( artist );
+        Collections::QueryMaker::ArtistMatchBehaviour behaviour =
+                ( levelCategory == CategoryId::AlbumArtist ) ? Collections::QueryMaker::AlbumArtists :
+                                                               Collections::QueryMaker::TrackArtists;
+        qm->addMatch( artist, behaviour );
     }
     else if( Meta::AlbumPtr album = Meta::AlbumPtr::dynamicCast( m_data ) )
         qm->addMatch( album );
diff --git a/src/core-impl/collections/db/sql/SqlQueryMaker.cpp b/src/core-impl/collections/db/sql/SqlQueryMaker.cpp
index 2eafc65..e596fe1 100644
--- a/src/core-impl/collections/db/sql/SqlQueryMaker.cpp
+++ b/src/core-impl/collections/db/sql/SqlQueryMaker.cpp
@@ -85,7 +85,6 @@ struct SqlQueryMaker::Private
     bool withoutDuplicates;
     int maxResultSize;
     AlbumQueryMode albumMode;
-    ArtistQueryMode artistMode;
     LabelQueryMode labelMode;
     SqlWorkerThread *worker;
 
@@ -114,7 +113,6 @@ SqlQueryMaker::SqlQueryMaker( SqlCollection* collection )
     d->linkedTables = 0;
     d->withoutDuplicates = false;
     d->albumMode = AllAlbums;
-    d->artistMode = TrackArtists;
     d->labelMode = QueryMaker::NoConstraint;
     d->maxResultSize = -1;
     d->andStack.clear();
@@ -346,10 +344,13 @@ SqlQueryMaker::addMatch( const Meta::TrackPtr &track )
     return this;
 }
 
+
 QueryMaker*
-SqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+SqlQueryMaker::addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour )
 {
     d->linkedTables |= Private::ARTIST_TAB;
+    if( behaviour == AlbumArtists || behaviour == AlbumOrTrackArtists )
+        d->linkedTables |= Private::ALBUMARTIST_TAB;
 
     QString artistQuery;
     QString albumArtistQuery;
@@ -365,7 +366,7 @@ SqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
         albumArtistQuery = "( albumartists.name IS NULL OR albumartists.name = '')";
     }
 
-    switch( d->artistMode )
+    switch( behaviour )
     {
     case TrackArtists:
         d->queryMatch += " AND " + artistQuery;
@@ -377,8 +378,6 @@ SqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
         d->queryMatch += " AND ( (" + artistQuery + " ) OR ( " + albumArtistQuery + " ) )";
         break;
     }
-    //Turn back default value, so we don't need to worry about this until the next AlbumArtist request.
-    d->artistMode = TrackArtists;
     return this;
 }
 
@@ -634,16 +633,6 @@ SqlQueryMaker::setAlbumQueryMode( AlbumQueryMode mode )
 }
 
 QueryMaker*
-SqlQueryMaker::setArtistQueryMode( ArtistQueryMode mode )
-{
-    if( mode == AlbumArtists || mode == AlbumOrTrackArtists )
-        d->linkedTables |= Private::ALBUMARTIST_TAB;
-
-    d->artistMode = mode;
-    return this;
-}
-
-QueryMaker*
 SqlQueryMaker::setLabelQueryMode( LabelQueryMode mode )
 {
     d->labelMode = mode;
diff --git a/src/core-impl/collections/db/sql/SqlQueryMaker.h b/src/core-impl/collections/db/sql/SqlQueryMaker.h
index aa48663..b0a3e6f 100644
--- a/src/core-impl/collections/db/sql/SqlQueryMaker.h
+++ b/src/core-impl/collections/db/sql/SqlQueryMaker.h
@@ -41,7 +41,7 @@ class AMAROK_SQLCOLLECTION_EXPORT_TESTS SqlQueryMaker : public QueryMaker
         virtual QueryMaker* setQueryType( QueryType type );
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track );
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album );
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer );
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre );
@@ -61,7 +61,6 @@ class AMAROK_SQLCOLLECTION_EXPORT_TESTS SqlQueryMaker : public QueryMaker
         virtual QueryMaker* limitMaxResultSize( int size );
 
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
         virtual QueryMaker* setLabelQueryMode( LabelQueryMode mode );
 
         virtual QueryMaker* beginAnd();
diff --git a/src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp b/src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp
index 387a511..8a40c6c 100644
--- a/src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp
+++ b/src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp
@@ -262,8 +262,7 @@ IpodCopyTracksJob::slotStartDuplicateTrackSearch( const Meta::TrackPtr &track )
     // we cannot qm->addMatch( track ) - it matches by uidUrl()
     qm->addFilter( Meta::valTitle, track->name(), true, true );
     qm->addMatch( track->album() );
-    qm->setArtistQueryMode( Collections::QueryMaker::TrackArtists );
-    qm->addMatch( track->artist() );
+    qm->addMatch( track->artist(), Collections::QueryMaker::TrackArtists );
     qm->addMatch( track->composer() );
     qm->addMatch( track->year() );
     qm->addNumberFilter( Meta::valTrackNr, track->trackNumber(), Collections::QueryMaker::Equals );
diff --git a/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp b/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp
index 1e620b4..8676a17 100644
--- a/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp
+++ b/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp
@@ -201,24 +201,24 @@ namespace Collections
         
         return this;
     }
-    
+
     QueryMaker*
-    PlaydarQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+    PlaydarQueryMaker::addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour )
     {
         DEBUG_BLOCK
-        
-        CurriedUnaryQMFunction< const Meta::ArtistPtr& >::FunPtr funPtr = &QueryMaker::addMatch;
-        CurriedQMFunction *curriedFun = new CurriedUnaryQMFunction< const Meta::ArtistPtr& >( funPtr, artist );
+
+        CurriedBinaryQMFunction<const Meta::ArtistPtr &, ArtistMatchBehaviour>::FunPtr funPtr = &QueryMaker::addMatch;
+        CurriedQMFunction *curriedFun = new CurriedBinaryQMFunction<const Meta::ArtistPtr &, ArtistMatchBehaviour>( funPtr, artist, behaviour );
         m_queryMakerFunctions.append( curriedFun );
-        
+
         (*curriedFun)( m_memoryQueryMaker.data() );
 
         if( artist )
             m_filterMap.insert( Meta::valArtist, artist->name() );
-        
+
         return this;
     }
-    
+
     QueryMaker*
     PlaydarQueryMaker::addMatch( const Meta::AlbumPtr &album )
     {
diff --git a/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h b/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h
index 2b85f5f..dd83477 100644
--- a/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h
+++ b/src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h
@@ -53,7 +53,7 @@ namespace Collections
             QueryMaker* orderBy( qint64 value, bool descending = false );
 
             QueryMaker* addMatch( const Meta::TrackPtr &track );
-            QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+            QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
             QueryMaker* addMatch( const Meta::AlbumPtr &album );
             QueryMaker* addMatch( const Meta::ComposerPtr &composer );
             QueryMaker* addMatch( const Meta::GenrePtr &genre );
diff --git a/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.cpp b/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.cpp
index 05f0a8b..486eec9 100644
--- a/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.cpp
+++ b/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.cpp
@@ -196,10 +196,10 @@ ProxyQueryMaker::addMatch( const Meta::TrackPtr &track )
 }
 
 QueryMaker*
-ProxyQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+ProxyQueryMaker::addMatch( const Meta::ArtistPtr &artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
     foreach( QueryMaker *b, m_builders )
-        b->addMatch( artist );
+        b->addMatch( artist, behaviour );
     return this;
 }
 
@@ -288,14 +288,6 @@ ProxyQueryMaker::setAlbumQueryMode( AlbumQueryMode mode )
 }
 
 QueryMaker*
-ProxyQueryMaker::setArtistQueryMode( QueryMaker::ArtistQueryMode mode )
-{
-    foreach( QueryMaker *b, m_builders )
-        b->setArtistQueryMode( mode );
-    return this;
-}
-
-QueryMaker*
 ProxyQueryMaker::setLabelQueryMode( LabelQueryMode mode )
 {
     foreach( QueryMaker *b, m_builders )
diff --git a/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.h b/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.h
index ba989a7..0dec479 100644
--- a/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.h
+++ b/src/core-impl/collections/proxycollection/ProxyCollectionQueryMaker.h
@@ -54,7 +54,7 @@ class AMAROK_EXPORT_TESTS ProxyQueryMaker : public QueryMaker
         virtual QueryMaker* orderBy( qint64 value, bool descending = false );
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track );
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album );
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer );
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre );
@@ -74,7 +74,6 @@ class AMAROK_EXPORT_TESTS ProxyQueryMaker : public QueryMaker
         virtual QueryMaker* endAndOr();
 
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
         virtual QueryMaker* setLabelQueryMode( LabelQueryMode mode );
 
     private:
diff --git a/src/core-impl/collections/support/MemoryMatcher.cpp b/src/core-impl/collections/support/MemoryMatcher.cpp
index a79d61c..6e94647 100644
--- a/src/core-impl/collections/support/MemoryMatcher.cpp
+++ b/src/core-impl/collections/support/MemoryMatcher.cpp
@@ -81,7 +81,7 @@ TrackList TrackMatcher::match( const TrackList &tracks )
 
 
 
-ArtistMatcher::ArtistMatcher( ArtistPtr artist, Collections::QueryMaker::ArtistQueryMode artistMode )
+ArtistMatcher::ArtistMatcher( ArtistPtr artist, Collections::QueryMaker::ArtistMatchBehaviour artistMode )
     : MemoryMatcher()
     , m_artist( artist )
     , m_queryMode( artistMode )
diff --git a/src/core-impl/collections/support/MemoryMatcher.h b/src/core-impl/collections/support/MemoryMatcher.h
index 155bd98..e3566f6 100644
--- a/src/core-impl/collections/support/MemoryMatcher.h
+++ b/src/core-impl/collections/support/MemoryMatcher.h
@@ -59,14 +59,14 @@ class AMAROK_EXPORT TrackMatcher : public MemoryMatcher
 class AMAROK_EXPORT ArtistMatcher : public MemoryMatcher
 {
     public:
-        ArtistMatcher( Meta::ArtistPtr artist, Collections::QueryMaker::ArtistQueryMode artistMode
+        ArtistMatcher( Meta::ArtistPtr artist, Collections::QueryMaker::ArtistMatchBehaviour artistMode
                        = Collections::QueryMaker::TrackArtists );
         virtual Meta::TrackList match( Collections::MemoryCollection *memColl );
         virtual Meta::TrackList match( const Meta::TrackList &tracks );
 
     private:
         Meta::ArtistPtr m_artist;
-        Collections::QueryMaker::ArtistQueryMode m_queryMode;
+        Collections::QueryMaker::ArtistMatchBehaviour m_queryMode;
 };
 
 class AMAROK_EXPORT AlbumMatcher : public MemoryMatcher
diff --git a/src/core-impl/collections/support/MemoryQueryMaker.cpp b/src/core-impl/collections/support/MemoryQueryMaker.cpp
index 0c5a52b..eece2f7 100644
--- a/src/core-impl/collections/support/MemoryQueryMaker.cpp
+++ b/src/core-impl/collections/support/MemoryQueryMaker.cpp
@@ -79,7 +79,6 @@ struct MemoryQueryMaker::Private {
     bool orderDescending;
     bool orderByNumberField;
     AlbumQueryMode albumQueryMode;
-    ArtistQueryMode artistQueryMode;
     LabelQueryMode labelQueryMode;
     QString collectionId;
 };
@@ -103,7 +102,6 @@ MemoryQueryMaker::MemoryQueryMaker( QWeakPointer<MemoryCollection> mc, const QSt
     d->orderDescending = false;
     d->orderByNumberField = false;
     d->albumQueryMode = AllAlbums;
-    d->artistQueryMode = TrackArtists;
     d->labelQueryMode = QueryMaker::NoConstraint;
 }
 
@@ -304,9 +302,9 @@ MemoryQueryMaker::addMatch( const Meta::TrackPtr &track )
 }
 
 QueryMaker*
-MemoryQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+MemoryQueryMaker::addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour )
 {
-    MemoryMatcher *artistMatcher = new ArtistMatcher( artist, d->artistQueryMode );
+    MemoryMatcher *artistMatcher = new ArtistMatcher( artist, behaviour );
     if ( d->matcher == 0 )
         d->matcher = artistMatcher;
     else
@@ -316,8 +314,6 @@ MemoryQueryMaker::addMatch( const Meta::ArtistPtr &artist )
             tmp = tmp->next();
         tmp->setNext( artistMatcher );
     }
-
-    d->artistQueryMode = QueryMaker::TrackArtists;
     return this;
 }
 
@@ -486,12 +482,6 @@ QueryMaker * MemoryQueryMaker::setAlbumQueryMode( AlbumQueryMode mode )
     return this;
 }
 
-QueryMaker* MemoryQueryMaker::setArtistQueryMode( QueryMaker::ArtistQueryMode mode )
-{
-    d->artistQueryMode = mode;
-    return this;
-}
-
 QueryMaker*
 MemoryQueryMaker::setLabelQueryMode( LabelQueryMode mode )
 {
diff --git a/src/core-impl/collections/support/MemoryQueryMaker.h b/src/core-impl/collections/support/MemoryQueryMaker.h
index b3d4989..a9c500c 100644
--- a/src/core-impl/collections/support/MemoryQueryMaker.h
+++ b/src/core-impl/collections/support/MemoryQueryMaker.h
@@ -57,7 +57,7 @@ class AMAROK_EXPORT MemoryQueryMaker : public QueryMaker
         virtual QueryMaker* orderBy( qint64 value, bool descending = false );
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track );
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album );
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer );
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre );
@@ -77,7 +77,6 @@ class AMAROK_EXPORT MemoryQueryMaker : public QueryMaker
         virtual QueryMaker* endAndOr();
 
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
         virtual QueryMaker* setLabelQueryMode( LabelQueryMode mode );
 
     private slots:
diff --git a/src/core-impl/collections/support/XmlQueryReader.cpp b/src/core-impl/collections/support/XmlQueryReader.cpp
index 844b688..af1b3fb 100644
--- a/src/core-impl/collections/support/XmlQueryReader.cpp
+++ b/src/core-impl/collections/support/XmlQueryReader.cpp
@@ -123,18 +123,6 @@ XmlQueryReader::readQuery()
             {
                 d->qm->setAlbumQueryMode( Collections::QueryMaker::OnlyNormalAlbums );
             }
-            else if( name() == "onlyTrackArtists" )
-            {
-                d->qm->setArtistQueryMode( Collections::QueryMaker::TrackArtists );
-            }
-            else if( name() == "onlyAlbumArtists" )
-            {
-                d->qm->setArtistQueryMode( Collections::QueryMaker::AlbumArtists );
-            }
-            else if( name() == "AlbumOrTrackArtists" )
-            {
-                d->qm->setArtistQueryMode( Collections::QueryMaker::AlbumOrTrackArtists );
-            }
             else if( name() == "returnValues" )
                 readReturnValues();
             //add more container elements here
@@ -334,4 +322,3 @@ XmlQueryReader::compareVal( QStringRef compare )
     else
         return -1;
 }
-
diff --git a/src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp b/src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp
index ce53c9c..0e7a214 100644
--- a/src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp
+++ b/src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp
@@ -216,9 +216,10 @@ DEBUG_BLOCK
     return this;
 }
 
-QueryMaker* UpnpQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+QueryMaker* UpnpQueryMaker::addMatch( const Meta::ArtistPtr &artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
 DEBUG_BLOCK
+    Q_UNUSED( behaviour ); // TODO: does UPnP tell between track and album artists?
     debug() << this << "Adding artist match" << artist->name();
     m_query.addMatch( "( upnp:artist = \"" + artist->name() + "\" )" );
     return this;
diff --git a/src/core-impl/collections/upnpcollection/UpnpQueryMaker.h b/src/core-impl/collections/upnpcollection/UpnpQueryMaker.h
index 0bf7390..83424f8 100644
--- a/src/core-impl/collections/upnpcollection/UpnpQueryMaker.h
+++ b/src/core-impl/collections/upnpcollection/UpnpQueryMaker.h
@@ -58,7 +58,7 @@ class UpnpQueryMaker : public QueryMaker
         QueryMaker* orderBy( qint64 value, bool descending = false ) ;
 
         QueryMaker* addMatch( const Meta::TrackPtr &track ) ;
-        QueryMaker* addMatch( const Meta::ArtistPtr &artist ) ;
+        QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         QueryMaker* addMatch( const Meta::AlbumPtr &album ) ;
         QueryMaker* addMatch( const Meta::ComposerPtr &composer ) ;
         QueryMaker* addMatch( const Meta::GenrePtr &genre ) ;
diff --git a/src/core/collections/MetaQueryMaker.cpp b/src/core/collections/MetaQueryMaker.cpp
index bf9b09f..0b7a2dd 100644
--- a/src/core/collections/MetaQueryMaker.cpp
+++ b/src/core/collections/MetaQueryMaker.cpp
@@ -159,11 +159,12 @@ MetaQueryMaker::addMatch( const Meta::TrackPtr &track )
     return this;
 }
 
+
 QueryMaker*
-MetaQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+MetaQueryMaker::addMatch( const Meta::ArtistPtr &artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
     foreach( QueryMaker *b, builders )
-        b->addMatch( artist );
+        b->addMatch( artist, behaviour );
     return this;
 }
 
@@ -248,14 +249,6 @@ MetaQueryMaker::setAlbumQueryMode( AlbumQueryMode mode )
 }
 
 QueryMaker*
-MetaQueryMaker::setArtistQueryMode( ArtistQueryMode mode )
-{
-    foreach( QueryMaker *qm, builders )
-        qm->setArtistQueryMode( mode );
-    return this;
-}
-
-QueryMaker*
 MetaQueryMaker::setLabelQueryMode( LabelQueryMode mode )
 {
     foreach( QueryMaker *qm, builders )
diff --git a/src/core/collections/MetaQueryMaker.h b/src/core/collections/MetaQueryMaker.h
index e902bab..8c29b9b 100644
--- a/src/core/collections/MetaQueryMaker.h
+++ b/src/core/collections/MetaQueryMaker.h
@@ -44,7 +44,7 @@ class AMAROK_CORE_EXPORT MetaQueryMaker : public QueryMaker
         virtual QueryMaker* orderBy( qint64 value, bool descending = false );
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track );
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album );
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer );
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre );
@@ -64,7 +64,6 @@ class AMAROK_CORE_EXPORT MetaQueryMaker : public QueryMaker
         virtual QueryMaker* endAndOr();
 
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
         virtual QueryMaker* setLabelQueryMode( LabelQueryMode mode );
 
     private slots:
diff --git a/src/core/collections/QueryMaker.cpp b/src/core/collections/QueryMaker.cpp
index 9fad26f..877840f 100644
--- a/src/core/collections/QueryMaker.cpp
+++ b/src/core/collections/QueryMaker.cpp
@@ -37,13 +37,6 @@ QueryMaker::setAlbumQueryMode( AlbumQueryMode mode )
 }
 
 QueryMaker*
-QueryMaker::setArtistQueryMode( ArtistQueryMode mode )
-{
-    Q_UNUSED( mode )
-    return this;
-}
-
-QueryMaker*
 QueryMaker::setLabelQueryMode( LabelQueryMode mode )
 {
     Q_UNUSED( mode )
diff --git a/src/core/collections/QueryMaker.h b/src/core/collections/QueryMaker.h
index 02a2690..90c1357 100644
--- a/src/core/collections/QueryMaker.h
+++ b/src/core/collections/QueryMaker.h
@@ -36,14 +36,14 @@ class AMAROK_CORE_EXPORT QueryMaker : public QObject
                               , OnlyCompilations = 1
                               , OnlyNormalAlbums = 2 };
 
-        enum ArtistQueryMode { TrackArtists         = 0,
-                               AlbumArtists         = 1,
-                               AlbumOrTrackArtists  = 2 };
-
         enum LabelQueryMode { NoConstraint = 0
                                 , OnlyWithLabels = 1
                                 , OnlyWithoutLabels = 2 };
 
+        enum ArtistMatchBehaviour { TrackArtists,
+                                    AlbumArtists,
+                                    AlbumOrTrackArtists };
+
         //Filters that the QueryMaker accepts for searching.
         //not all implementations will accept all filter levels, so make it possible to
         //specify which ones make sense for a given qm. Add to this as needed
@@ -125,7 +125,15 @@ class AMAROK_CORE_EXPORT QueryMaker : public QObject
         virtual QueryMaker* orderBy( qint64 value, bool descending = false ) = 0;
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track ) = 0;
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist ) = 0;
+        /**
+         * Match given artist. Depending on @param behaviour matches:
+         *   track artist if TrackArtists is given,
+         *   album artist if AlbumArtists is given,
+         *   any of track or album artist if AlbumOrTrackArtists is given.
+         *
+         * By default matches only track artist.
+         */
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists ) = 0;
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album ) = 0;
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer ) = 0;
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre ) = 0;
@@ -166,15 +174,6 @@ class AMAROK_CORE_EXPORT QueryMaker : public QObject
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
 
         /**
-         * Set artist query mode. If this method is not called,
-         * QueryMaker defaults to ArtistQueryMode::TrackArtist.
-         *
-         * WARNING: this gets reset to TrackArtist at the end of the following call to
-         * addMatch( Meta::Artist )
-         */
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
-
-        /**
           * Sets the label query mode. This method restricts a query to tracks
           * that have labels assigned to them, no labels assigned to them, or no constraint.
           * The default is no constraint.
diff --git a/src/services/DynamicServiceQueryMaker.cpp b/src/services/DynamicServiceQueryMaker.cpp
index 326f084..66b9eae 100644
--- a/src/services/DynamicServiceQueryMaker.cpp
+++ b/src/services/DynamicServiceQueryMaker.cpp
@@ -55,10 +55,11 @@ QueryMaker * DynamicServiceQueryMaker::addMatch(const Meta::TrackPtr & track)
     return this;
 }
 
-QueryMaker * DynamicServiceQueryMaker::addMatch(const Meta::ArtistPtr & artist)
+QueryMaker * DynamicServiceQueryMaker::addMatch(const Meta::ArtistPtr & artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
     DEBUG_BLOCK
     Q_UNUSED( artist );
+    Q_UNUSED( behaviour );
     return this;
 }
 
diff --git a/src/services/DynamicServiceQueryMaker.h b/src/services/DynamicServiceQueryMaker.h
index ae39235..863d98e 100644
--- a/src/services/DynamicServiceQueryMaker.h
+++ b/src/services/DynamicServiceQueryMaker.h
@@ -58,7 +58,7 @@ public:
     virtual QueryMaker* orderBy ( qint64 value, bool descending = false );
 
     virtual QueryMaker* addMatch ( const Meta::TrackPtr &track );
-    virtual QueryMaker* addMatch ( const Meta::ArtistPtr &artist );
+    virtual QueryMaker* addMatch ( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
     virtual QueryMaker* addMatch ( const Meta::AlbumPtr &album );
     virtual QueryMaker* addMatch ( const Meta::GenrePtr &genre );
     virtual QueryMaker* addMatch ( const Meta::ComposerPtr &composer );
diff --git a/src/services/ServiceSqlQueryMaker.cpp b/src/services/ServiceSqlQueryMaker.cpp
index 23e24f4..77efa2a 100644
--- a/src/services/ServiceSqlQueryMaker.cpp
+++ b/src/services/ServiceSqlQueryMaker.cpp
@@ -79,7 +79,6 @@ struct ServiceSqlQueryMaker::Private
     //bool includedBuilder;
     //bool collectionRestriction;
     AlbumQueryMode albumMode;
-    ArtistQueryMode artistMode;
     bool withoutDuplicates;
     int maxResultSize;
     ServiceSqlWorkerThread *worker;
@@ -99,7 +98,6 @@ ServiceSqlQueryMaker::ServiceSqlQueryMaker( ServiceSqlCollection* collection, Se
 
     d->queryType = Private::NONE;
     d->linkedTables = 0;
-    d->artistMode = TrackArtists;
     d->withoutDuplicates = false;
     d->maxResultSize = -1;
     d->andStack.push( true );
@@ -281,19 +279,22 @@ ServiceSqlQueryMaker::addMatch( const Meta::TrackPtr &track )
 }
 
 QueryMaker*
-ServiceSqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
+ServiceSqlQueryMaker::addMatch( const Meta::ArtistPtr &artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
     QString prefix = m_metaFactory->tablePrefix();
 
     if( !d )
         return this;
 
+    if( behaviour == AlbumArtists || behaviour == AlbumOrTrackArtists )
+        d->linkedTables |= Private::ALBUMARTISTS_TABLE;
+
     //this should NOT be made into a static cast as this might get called with an incompatible type!
     const Meta::ServiceArtist * serviceArtist = dynamic_cast<const Meta::ServiceArtist *>( artist.data() );
     d->linkedTables |= Private::ARTISTS_TABLE;
     if( serviceArtist )
     {
-        switch( d->artistMode )
+        switch( behaviour )
         {
             case TrackArtists:
                  d->queryMatch += QString( " AND " + prefix + "_artists.id= '%1'" ).arg( serviceArtist->id() );
@@ -308,7 +309,7 @@ ServiceSqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
     }
     else
     {
-        switch( d->artistMode )
+        switch( behaviour )
         {
             case TrackArtists:
                  d->queryMatch += QString( " AND " + prefix + "_artists.name= '%1'" ).arg( escape( artist->name() ) );
@@ -321,7 +322,6 @@ ServiceSqlQueryMaker::addMatch( const Meta::ArtistPtr &artist )
                  break;
         }
     }
-    d->artistMode = TrackArtists;
     return this;
 }
 
@@ -845,15 +845,4 @@ ServiceSqlQueryMaker::setAlbumQueryMode(AlbumQueryMode mode)
     return this;
 }
 
-QueryMaker *
-ServiceSqlQueryMaker::setArtistQueryMode( QueryMaker::ArtistQueryMode mode )
-{
-    if( mode == AlbumArtists || mode == AlbumOrTrackArtists )
-        d->linkedTables |= Private::ALBUMARTISTS_TABLE;
-    d->artistMode = mode;
-    return this;
-}
-
-
 #include "ServiceSqlQueryMaker.moc"
-
diff --git a/src/services/ServiceSqlQueryMaker.h b/src/services/ServiceSqlQueryMaker.h
index 427ebb56..1b6e906 100644
--- a/src/services/ServiceSqlQueryMaker.h
+++ b/src/services/ServiceSqlQueryMaker.h
@@ -42,7 +42,7 @@ class ServiceSqlQueryMaker : public QueryMaker
         virtual QueryMaker* setQueryType( QueryType type );
 
         virtual QueryMaker* addMatch( const Meta::TrackPtr &track );
-        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist );
+        virtual QueryMaker* addMatch( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
         virtual QueryMaker* addMatch( const Meta::AlbumPtr &album );
         virtual QueryMaker* addMatch( const Meta::ComposerPtr &composer );
         virtual QueryMaker* addMatch( const Meta::GenrePtr &genre );
@@ -66,7 +66,6 @@ class ServiceSqlQueryMaker : public QueryMaker
         virtual QueryMaker* endAndOr();
 
         virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
-        virtual QueryMaker* setArtistQueryMode( ArtistQueryMode mode );
 
         QString query();
         QStringList runQuery( const QString &query );
diff --git a/src/services/scriptable/ScriptableServiceQueryMaker.cpp b/src/services/scriptable/ScriptableServiceQueryMaker.cpp
index d77c40c..856f570 100644
--- a/src/services/scriptable/ScriptableServiceQueryMaker.cpp
+++ b/src/services/scriptable/ScriptableServiceQueryMaker.cpp
@@ -162,12 +162,13 @@ QueryMaker * ScriptableServiceQueryMaker::addMatch( const Meta::GenrePtr &genre
     return this;
 }
 
-QueryMaker * ScriptableServiceQueryMaker::addMatch( const Meta::ArtistPtr & artist )
+QueryMaker * ScriptableServiceQueryMaker::addMatch( const Meta::ArtistPtr & artist, QueryMaker::ArtistMatchBehaviour behaviour )
 {
-    if ( d->closestParent > Private::ARTIST )
+    Q_UNUSED( behaviour );
+    const Meta::ScriptableServiceArtist *scriptableArtist = dynamic_cast<const Meta::ScriptableServiceArtist *>( artist.data() );
+    if ( scriptableArtist && d->closestParent > Private::ARTIST )
     {
         d->closestParent = Private::ARTIST;
-        const Meta::ScriptableServiceArtist * scriptableArtist = static_cast< const Meta::ScriptableServiceArtist * >( artist.data() );
         d->callbackString = scriptableArtist->callbackString();
         d->parentId = scriptableArtist->id();
     }
diff --git a/src/services/scriptable/ScriptableServiceQueryMaker.h b/src/services/scriptable/ScriptableServiceQueryMaker.h
index e6115aa..418ccbb 100644
--- a/src/services/scriptable/ScriptableServiceQueryMaker.h
+++ b/src/services/scriptable/ScriptableServiceQueryMaker.h
@@ -46,7 +46,7 @@ public:
 
     using QueryMaker::addMatch;
     virtual QueryMaker* addMatch ( const Meta::GenrePtr &genre );
-    virtual QueryMaker* addMatch ( const Meta::ArtistPtr &artist );
+    virtual QueryMaker* addMatch ( const Meta::ArtistPtr &artist, ArtistMatchBehaviour behaviour = TrackArtists );
     virtual QueryMaker* addMatch ( const Meta::AlbumPtr &album );
 
     virtual QueryMaker* setAlbumQueryMode( AlbumQueryMode mode );
diff --git a/tests/mocks/MockQueryMaker.h b/tests/mocks/MockQueryMaker.h
index 2a8af6b..55c2c85 100644
--- a/tests/mocks/MockQueryMaker.h
+++ b/tests/mocks/MockQueryMaker.h
@@ -84,7 +84,7 @@ class MockQueryMaker : public QueryMaker
             return 0;
         }
 
-        virtual QueryMaker *addMatch( const Meta::ArtistPtr& )
+        virtual QueryMaker *addMatch( const Meta::ArtistPtr& , ArtistMatchBehaviour )
         {
             Q_ASSERT_X( false, __PRETTY_FUNCTION__, "should not be called");
             return 0;


More information about the Amarok-devel mailing list