Make collections able to resolve queries

data/js/tomahawk.js

@@ -272,43 +272,18 @@ Tomahawk.Resolver = {
         return params;
     },
     _adapter_resolve: function (params) {
-        var that = this;
+        return RSVP.Promise.resolve(this.resolve(params)).then(function (results) {
-        var collectionPromises = [];
+            return {
-        Tomahawk.collections.forEach(function (col) {
+                'results': results
-            if (col.resolve) {
+            };
-                collectionPromises.push(col.resolve(params));
-            }
-        });
-
-        return RSVP.Promise.all(collectionPromises).then(function (collectionResults) {
-            var merged = [];
-            return merged.concat.apply(merged, collectionResults);
-        }).then(function (collectionResults) {
-            return RSVP.Promise.resolve(that.resolve(params)).then(function (results) {
-                return {
-                    'results': results.concat(collectionResults)
-                };
-            });
         });
     },
 
     _adapter_search: function (params) {
-        var that = this;
+        return RSVP.Promise.resolve(this.search(params)).then(function (results) {
-        var collectionPromises = [];
+            return {
-        Tomahawk.collections.forEach(function (col) {
+                'results': results
-            if (col.search) {
+            };
-                collectionPromises.push(col.search(params));
-            }
-        });
-        return RSVP.Promise.all(collectionPromises).then(function (collectionResults) {
-            var merged = [];
-            return merged.concat.apply(merged, collectionResults);
-        }).then(function (collectionResults) {
-            return RSVP.Promise.resolve(that.search(params)).then(function (results) {
-                return {
-                    'results': results.concat(collectionResults)
-                };
-            });
         });
     },
 
@@ -1587,11 +1562,27 @@ Tomahawk.Collection = {
         });
     },
 
+    _adapter_resolve: function (params) {
+        return RSVP.Promise.resolve(this.resolve(params)).then(function (results) {
+            return {
+                'results': results
+            };
+        });
+    },
+
     resolve: function (params) {
         var resultIds = Tomahawk.resolveFromFuzzyIndex(params.artist, params.album, params.track);
         return this._fuzzyIndexIdsToTracks(resultIds);
     },
 
+    _adapter_search: function (params) {
+        return RSVP.Promise.resolve(this.search(params)).then(function (results) {
+            return {
+                'results': results
+            };
+        });
+    },
+
     search: function (params) {
         var resultIds = Tomahawk.searchFuzzyIndex(params.query);
         return this._fuzzyIndexIdsToTracks(resultIds);

src/libtomahawk/CMakeLists.txt

@@ -199,7 +199,6 @@ list(APPEND libSources
     MetaPlaylistInterface.cpp
     Query.cpp
     Result.cpp
-    ResultProvider.cpp
     Source.cpp
     Track.cpp
     TrackData.cpp

src/libtomahawk/Pipeline.cpp

@@ -340,7 +340,7 @@ Pipeline::reportResults( QID qid, Tomahawk::Resolver* r, const QList< result_ptr
     {
         if ( !results.isEmpty() )
         {
-            ResultProvider* resolvedBy = results[0]->resolvedBy();
+            Resolver* resolvedBy = results[0]->resolvedBy();
             if ( resolvedBy )
             {
                 tDebug() << "Result arrived too late for:" << qid << "by" << resolvedBy->name();

src/libtomahawk/Result.cpp

@@ -491,7 +491,7 @@ Result::setFileId( unsigned int id )
 }
 
 
-Tomahawk::ResultProvider*
+Tomahawk::Resolver*
 Result::resolvedBy() const
 {
     if ( !m_collection.isNull() )

src/libtomahawk/Result.h

@@ -37,7 +37,6 @@ class MetadataEditor;
 namespace Tomahawk
 {
 
-class ResultProvider;
 class Resolver;
 
 class DLLEXPORT Result : public QObject
@@ -86,9 +85,9 @@ public:
     void setResolvedByResolver( Tomahawk::Resolver* resolver );
 
     /**
-     *  This is very bad. ResultProvider is not a QObject and thus can not be tracked by a qt smart pointer ... :-(
+     *  TODO: Make this a smart pointer
      */
-    ResultProvider* resolvedBy() const;
+    Resolver* resolvedBy() const;
 
     RID id() const;
     bool isOnline() const;

src/libtomahawk/ResultProvider.cpp

@@ -1,37 +0,0 @@
-/* === This file is part of Tomahawk Player - <http://tomahawk-player.org> ===
- *
- *   Copyright (C) 2015  Dominik Schmidt <domme@tomahawk-player.org>
- *
- *   Tomahawk is free software: you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation, either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   Tomahawk is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- *   GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with Tomahawk. If not, see <http://www.gnu.org/licenses/>.
- */
-#include "ResultProvider.h"
-
-#include "Result.h"
-#include "resolvers/SyncScriptJob.h"
-
-using namespace Tomahawk;
-
-ResultProvider::~ResultProvider()
-{
-}
-
-
-ScriptJob*
-ResultProvider::getStreamUrl( const result_ptr& result )
-{
-    QVariantMap data;
-    data[ "url" ] = result->url();
-
-    return new SyncScriptJob( data );
-}

src/libtomahawk/ResultProvider.h

@@ -1,46 +0,0 @@
-/* === This file is part of Tomahawk Player - <http://tomahawk-player.org> ===
- *
- *   Copyright (C) 2015  Dominik Schmidt <domme@tomahawk-player.org>
- *
- *   Tomahawk is free software: you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation, either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   Tomahawk is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- *   GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with Tomahawk. If not, see <http://www.gnu.org/licenses/>.
- */
-#pragma once
-#ifndef TOMAHAWK_RESULTPROVIDER_H
-#define TOMAHAWK_RESULTPROVIDER_H
-
-#include "DllMacro.h"
-#include "Typedefs.h"
-
-class QPixmap;
-class QString;
-class QSize;
-
-namespace Tomahawk
-{
-class ScriptJob;
-
-class DLLEXPORT ResultProvider
-{
-public:
-    virtual ~ResultProvider();
-
-    virtual QString name() const = 0;
-    virtual QPixmap icon( const QSize& size ) const = 0;
-
-    virtual ScriptJob* getStreamUrl( const result_ptr& result );
-};
-
-}
-
-#endif // TOMAHAWK_RESULTPROVIDER_H

src/libtomahawk/collection/Collection.cpp

@@ -35,7 +35,7 @@ using namespace Tomahawk;
 
 
 Collection::Collection( const source_ptr& source, const QString& name, QObject* parent )
-    : QObject( parent )
+    : Resolver( parent )
     , m_name( name )
     , m_lastmodified( 0 )
     , m_changed( false )

src/libtomahawk/collection/Collection.h

@@ -33,7 +33,7 @@
 #include "collection/ArtistsRequest.h"
 #include "collection/AlbumsRequest.h"
 #include "collection/TracksRequest.h"
-#include "../ResultProvider.h"
+#include "../resolvers/Resolver.h"
 
 #include "DllMacro.h"
 
@@ -46,7 +46,7 @@
 namespace Tomahawk
 {
 
-class DLLEXPORT Collection : public QObject, public ResultProvider
+class DLLEXPORT Collection : public Resolver
 {
 Q_OBJECT
 

src/libtomahawk/database/DatabaseCollection.cpp

@@ -251,3 +251,32 @@ DatabaseCollection::stationCreated( const source_ptr& source, const QVariantList
 }
 
 
+
+/*
+ * Resolver interface
+ *
+ * We implement searching the database in the DatabaseResolver which avoids a n+1 query here.
+ * We can't simply let ScriptCollection inherit Collection and Resolver because both are QObjects,
+ * although Resolver doesn't need to be a QObject atm, blocking adding signals/slots to Resolver
+ * in future seems to me worse than violating Liskov's law here. ~ domme
+ */
+unsigned int
+DatabaseCollection::timeout() const
+{
+    return 0;
+}
+
+
+unsigned int
+DatabaseCollection::weight() const
+{
+    return 0;
+}
+
+
+void
+DatabaseCollection::resolve( const Tomahawk::query_ptr& query )
+{
+    Q_UNUSED( query );
+    Q_ASSERT(false);
+}

src/libtomahawk/database/DatabaseCollection.h

@@ -65,6 +65,11 @@ public:
     int trackCount() const override;
     QPixmap icon( const QSize& size ) const override;
 
+    // Resolver interface
+    unsigned int weight() const override;
+    unsigned int timeout() const override;
+    void resolve( const Tomahawk::query_ptr& query ) override;
+
 public slots:
     virtual void addTracks( const QList<QVariant>& newitems );
     virtual void removeTracks( const QDir& dir );

src/libtomahawk/resolvers/JSResolver.cpp

@@ -538,30 +538,43 @@ JSResolver::error() const
 void
 JSResolver::resolve( const Tomahawk::query_ptr& query )
 {
-    ScriptJob* job = nullptr;
+    ScriptJob* job = scriptAccount()->resolve( scriptObject(), query );
-    if ( !query->isFullTextQuery() )
-    {
-        QVariantMap arguments;
-        arguments["artist"] = query->queryTrack()->artist();
-        arguments["album"] = query->queryTrack()->album();
-        arguments["track"] = query->queryTrack()->track();
 
-        job = scriptObject()->invoke( "resolve", arguments );
-    }
-    else
-    {
-        QVariantMap arguments;
-        arguments["query"] = query->fullTextQuery();
-        job = scriptObject()->invoke( "search", arguments );
-    }
-
-
-    job->setProperty( "qid", query->id() );
     connect( job, SIGNAL( done( QVariantMap ) ), SLOT( onResolveRequestDone( QVariantMap ) ) );
 
     job->start();
 }
 
+void
+JSResolver::onResolveRequestDone( const QVariantMap& data )
+{
+    Q_ASSERT( QThread::currentThread() == thread() );
+    Q_D( JSResolver );
+
+    ScriptJob* job = qobject_cast< ScriptJob* >( sender() );
+
+    QID qid = job->property( "qid" ).toString();
+
+    if ( job->error() )
+    {
+        Tomahawk::Pipeline::instance()->reportError( qid, this );
+    }
+    else
+    {
+
+        QList< Tomahawk::result_ptr > results = scriptAccount()->parseResultVariantList( data.value( "results" ).toList() );
+
+        foreach( const result_ptr& result, results )
+        {
+            result->setResolvedByResolver( this );
+            result->setFriendlySource( name() );
+        }
+
+        Tomahawk::Pipeline::instance()->reportResults( qid, this, results );
+    }
+
+    sender()->deleteLater();
+}
 
 void
 JSResolver::stop()
@@ -672,38 +685,6 @@ JSResolver::resolverUserConfig()
 }
 
 
-void
-JSResolver::onResolveRequestDone( const QVariantMap& data )
-{
-    Q_ASSERT( QThread::currentThread() == thread() );
-    Q_D( JSResolver );
-
-    ScriptJob* job = qobject_cast< ScriptJob* >( sender() );
-
-    QID qid = job->property( "qid" ).toString();
-
-    if ( job->error() )
-    {
-        Tomahawk::Pipeline::instance()->reportError( qid, this );
-    }
-    else
-    {
-
-        QList< Tomahawk::result_ptr > results = scriptAccount()->parseResultVariantList( data.value( "results" ).toList() );
-
-        foreach( const result_ptr& result, results )
-        {
-            result->setResolvedByResolver( this );
-            result->setFriendlySource( name() );
-        }
-
-        Tomahawk::Pipeline::instance()->reportResults( qid, this, results );
-    }
-
-    sender()->deleteLater();
-}
-
-
 QString
 JSResolver::instanceUUID()
 {

src/libtomahawk/resolvers/Resolver.cpp

@@ -17,10 +17,31 @@
  */
 #include "Resolver.h"
 
+#include "../resolvers/SyncScriptJob.h"
+#include "../Result.h"
+
 #include <QPixmap>
 
+
+Tomahawk::Resolver::Resolver( QObject* parent )
+    : QObject( parent )
+{
+}
+
+
 QPixmap
 Tomahawk::Resolver::icon( const QSize& ) const
 {
+    Q_ASSERT(false);
     return QPixmap();
 }
+
+
+Tomahawk::ScriptJob*
+Tomahawk::Resolver::getStreamUrl( const result_ptr& result )
+{
+    QVariantMap data;
+    data[ "url" ] = result->url();
+
+    return new SyncScriptJob( data );
+}

src/libtomahawk/resolvers/Resolver.h

@@ -19,7 +19,6 @@
 #ifndef RESOLVER_H
 #define RESOLVER_H
 
-#include "../ResultProvider.h"
 #include "Typedefs.h"
 #include "DllMacro.h"
 
@@ -36,21 +35,23 @@
 
 namespace Tomahawk
 {
+class ScriptJob;
 
-class DLLEXPORT Resolver : public QObject, public ResultProvider
+class DLLEXPORT Resolver : public QObject
 {
 Q_OBJECT
 
 public:
-    Resolver() {}
+    Resolver( QObject* parent = nullptr );
+
+    virtual QString name() const = 0;
+    virtual QPixmap icon( const QSize& size ) const;
 
     virtual unsigned int weight() const = 0;
     virtual unsigned int timeout() const = 0;
 
-    virtual QPixmap icon( const QSize& size ) const override;
-
-public slots:
     virtual void resolve( const Tomahawk::query_ptr& query ) = 0;
+    virtual ScriptJob* getStreamUrl( const result_ptr& result );
 };
 
 } //ns

src/libtomahawk/resolvers/ScriptAccount.cpp

@@ -353,3 +353,29 @@ ScriptAccount::scriptCollection( const QString& id ) const
 {
     return m_collectionFactory->scriptPlugins().value( id );
 }
+
+
+ScriptJob*
+ScriptAccount::resolve( const scriptobject_ptr& scriptObject, const query_ptr& query )
+{
+    ScriptJob* job = nullptr;
+    if ( !query->isFullTextQuery() )
+    {
+        QVariantMap arguments;
+        arguments["artist"] = query->queryTrack()->artist();
+        arguments["album"] = query->queryTrack()->album();
+        arguments["track"] = query->queryTrack()->track();
+
+        job = scriptObject->invoke( "resolve", arguments );
+    }
+    else
+    {
+        QVariantMap arguments;
+        arguments["query"] = query->fullTextQuery();
+        job = scriptObject->invoke( "search", arguments );
+    }
+
+    job->setProperty( "qid", query->id() );
+
+    return job;
+}

src/libtomahawk/resolvers/ScriptAccount.h

@@ -76,10 +76,13 @@ public:
 
     virtual void scriptPluginFactory( const QString& type, const scriptobject_ptr& object );
 
-    QList< Tomahawk::result_ptr > parseResultVariantList( const QVariantList& reslist );
-
     QSharedPointer< ScriptCollection > scriptCollection( const QString& id ) const;
 
+
+    // helpers
+    QList< Tomahawk::result_ptr > parseResultVariantList( const QVariantList& reslist );
+    ScriptJob* resolve( const scriptobject_ptr& scriptObject, const query_ptr& query );
+
 private slots:
     void onJobDeleted( const QString& jobId );
 

src/libtomahawk/resolvers/ScriptCollection.cpp

@@ -26,8 +26,10 @@
 #include "resolvers/ScriptCommand_AllArtists.h"
 #include "resolvers/ScriptCommand_AllAlbums.h"
 #include "resolvers/ScriptCommand_AllTracks.h"
+#include "resolvers/ScriptJob.h"
 #include "ScriptAccount.h"
 #include "Result.h"
+#include "Pipeline.h"
 
 #include <QImageReader>
 #include <QPainter>
@@ -336,3 +338,58 @@ ScriptCollection::onIconFetched()
         reply->deleteLater();
     }
 }
+
+
+unsigned int
+ScriptCollection::timeout() const
+{
+    return 0;
+}
+
+
+unsigned int
+ScriptCollection::weight() const
+{
+    return 0;
+}
+
+
+void
+ScriptCollection::resolve( const Tomahawk::query_ptr& query )
+{
+    ScriptJob* job = scriptAccount()->resolve( scriptObject(), query );
+
+    connect( job, SIGNAL( done( QVariantMap ) ), SLOT( onResolveRequestDone( QVariantMap ) ) );
+
+    job->start();
+}
+
+
+void
+ScriptCollection::onResolveRequestDone( const QVariantMap& data )
+{
+    Q_ASSERT( QThread::currentThread() == thread() );
+
+    ScriptJob* job = qobject_cast< ScriptJob* >( sender() );
+
+    QID qid = job->property( "qid" ).toString();
+
+    if ( job->error() )
+    {
+        Tomahawk::Pipeline::instance()->reportError( qid, this );
+    }
+    else
+    {
+        QList< Tomahawk::result_ptr > results = scriptAccount()->parseResultVariantList( data.value( "results" ).toList() );
+
+        foreach( const result_ptr& result, results )
+        {
+            result->setResolvedByCollection( weakRef().toStrongRef() );
+            result->setFriendlySource( name() );
+        }
+
+        Tomahawk::Pipeline::instance()->reportResults( qid, this, results );
+    }
+
+    sender()->deleteLater();
+}

src/libtomahawk/resolvers/ScriptCollection.h

@@ -91,10 +91,15 @@ public:
     void parseMetaData();
     void parseMetaData( const QVariantMap& metadata );
 
+    // Resolver interface
+    unsigned int weight() const override;
+    unsigned int timeout() const override;
+    void resolve( const Tomahawk::query_ptr& query ) override;
     ScriptJob* getStreamUrl( const result_ptr& result ) override;
 
 private slots:
     void onIconFetched();
+    void onResolveRequestDone( const QVariantMap& data );
 
 private:
     ScriptAccount* m_scriptAccount;

src/libtomahawk/resolvers/plugins/ScriptCollectionFactory.cpp

@@ -19,6 +19,7 @@
 
 #include "SourceList.h"
 #include "../ScriptAccount.h"
+#include "../../Pipeline.h"
 
 using namespace Tomahawk;
 
@@ -29,12 +30,14 @@ void ScriptCollectionFactory::addPlugin( const QSharedPointer<ScriptCollection>&
 
     collection->setOnline( true );
     SourceList::instance()->addScriptCollection( collection );
+    Pipeline::instance()->addResolver( collection.data() );
 }
 
 void ScriptCollectionFactory::removePlugin( const QSharedPointer<ScriptCollection>& collection ) const
 {
     collection->setOnline( false );
     SourceList::instance()->removeScriptCollection( collection );
+    Pipeline::instance()->removeResolver( collection.data() );
 }
 
 QSharedPointer< ScriptCollection > ScriptCollectionFactory::createPlugin( const scriptobject_ptr& object, ScriptAccount* scriptAccount )