Test no commandFactory is available before it was registered

Thanks a lot to @rioderelfte for a long discussion on how to make this
more elegant - sorry, for not realizing your suggestions, too much work
for now just to be able to call commandname() statically.

src/libtomahawk/Result.cpp

@@ -376,9 +376,11 @@ Result::sourceIcon( TomahawkUtils::ImageMode style, const QSize& desiredSize ) c
 {
     if ( collection().isNull() )
     {
-        const ExternalResolverGui* guiResolver = qobject_cast< ExternalResolverGui* >( m_resolvedBy.data() );
-        if ( !guiResolver )
+        const ExternalResolver* resolver = qobject_cast< ExternalResolver* >( m_resolvedBy.data() );
+        if ( !resolver )
         {
+            tLog() << "Result was not resolved by an ExternalResolver but the collection is empty too. What is going on?";
+            Q_ASSERT( resolver );
             return QPixmap();
         }
         else
@@ -388,7 +390,7 @@ Result::sourceIcon( TomahawkUtils::ImageMode style, const QSize& desiredSize ) c
             const QString key = sourceCacheKey( m_resolvedBy.data(), desiredSize, style );
             if ( !sourceIconCache()->contains( key ) )
             {
-                QPixmap pixmap = guiResolver->icon();
+                QPixmap pixmap = resolver->icon();
                 if ( !desiredSize.isEmpty() )
                     pixmap = pixmap.scaled( desiredSize, Qt::KeepAspectRatio, Qt::SmoothTransformation );
 

src/libtomahawk/database/Database.cpp

@@ -27,11 +27,43 @@
 #include "IdThreadWorker.h"
 #include "PlaylistEntry.h"
 
+#include "DatabaseCommand_AddFiles.h"
+#include "DatabaseCommand_CreatePlaylist.h"
+#include "DatabaseCommand_DeleteFiles.h"
+#include "DatabaseCommand_DeletePlaylist.h"
+#include "DatabaseCommand_LogPlayback.h"
+#include "DatabaseCommand_RenamePlaylist.h"
+#include "DatabaseCommand_SetPlaylistRevision.h"
+#include "DatabaseCommand_CreateDynamicPlaylist.h"
+#include "DatabaseCommand_DeleteDynamicPlaylist.h"
+#include "DatabaseCommand_SetDynamicPlaylistRevision.h"
+#include "DatabaseCommand_SocialAction.h"
+#include "DatabaseCommand_ShareTrack.h"
+#include "DatabaseCommand_SetCollectionAttributes.h"
+#include "DatabaseCommand_SetTrackAttributes.h"
+
+// Forward Declarations breaking QSharedPointer
+#if QT_VERSION < QT_VERSION_CHECK( 5, 0, 0 )
+    #include "collection/Collection.h"
+#endif
+
 #include <boost/concept_check.hpp>
 
 #define DEFAULT_WORKER_THREADS 4
 #define MAX_WORKER_THREADS 16
 
+
+DatabaseCommand*
+DatabaseCommandFactory::newInstance()
+{
+    DatabaseCommand* command = create();
+    emit created( command );
+    return command;
+}
+
+
+
+
 Database* Database::s_instance = 0;
 
 
@@ -51,6 +83,22 @@ Database::Database( const QString& dbname, QObject* parent )
 {
     s_instance = this;
 
+    // register commands
+    registerCommand<DatabaseCommand_AddFiles>();
+    registerCommand<DatabaseCommand_DeleteFiles>();
+    registerCommand<DatabaseCommand_CreatePlaylist>();
+    registerCommand<DatabaseCommand_DeletePlaylist>();
+    registerCommand<DatabaseCommand_LogPlayback>();
+    registerCommand<DatabaseCommand_RenamePlaylist>();
+    registerCommand<DatabaseCommand_SetPlaylistRevision>();
+    registerCommand<DatabaseCommand_CreateDynamicPlaylist>();
+    registerCommand<DatabaseCommand_DeleteDynamicPlaylist>();
+    registerCommand<DatabaseCommand_SetDynamicPlaylistRevision>();
+    registerCommand<DatabaseCommand_SocialAction>();
+    registerCommand<DatabaseCommand_SetCollectionAttributes>();
+    registerCommand<DatabaseCommand_SetTrackAttributes>();
+    registerCommand<DatabaseCommand_ShareTrack>();
+
     if ( MAX_WORKER_THREADS < DEFAULT_WORKER_THREADS )
         m_maxConcurrentThreads = MAX_WORKER_THREADS;
     else
@@ -206,3 +254,69 @@ Database::markAsReady()
     m_ready = true;
     emit ready();
 }
+
+
+void
+Database::registerCommand( DatabaseCommandFactory* commandFactory )
+{
+    // this is ugly, but we don't have virtual static methods in C++ :(
+    QScopedPointer<DatabaseCommand> command( commandFactory->newInstance() );
+
+    const QString commandName = command->commandname();
+    const QString className = command->metaObject()->className();
+
+    tDebug() << "Registering command" << commandName << "from class" << className;
+
+    if( m_commandFactories.keys().contains( commandName ) )
+    {
+        tLog() << commandName << "is already in " << m_commandFactories.keys();
+    }
+    Q_ASSERT( !m_commandFactories.keys().contains( commandName ) );
+
+    m_commandNameClassNameMapping.insert( commandName, className );
+    m_commandFactories.insert( commandName, commandFactory );
+}
+
+
+DatabaseCommandFactory*
+Database::commandFactoryByClassName(const QString& className)
+{
+    const QString commandName = m_commandNameClassNameMapping.key( className );
+    return commandFactoryByCommandName( commandName );
+}
+
+
+DatabaseCommandFactory*
+Database::commandFactoryByCommandName(const QString& commandName )
+{
+    return m_commandFactories.value( commandName );
+}
+
+
+
+DatabaseCommand*
+Database::createCommandInstance( const QString& commandName )
+{
+    DatabaseCommandFactory* factory = commandFactoryByCommandName( commandName );
+
+    if( !factory )
+    {
+         tLog() << "Unknown database command" << commandName;
+         return 0;
+    }
+
+    return factory->newInstance();
+}
+
+
+DatabaseCommand*
+Database::createCommandInstance(const QVariant& op, const source_ptr& source)
+{
+    const QString commandName = op.toMap().value( "command" ).toString();
+
+    DatabaseCommand* command = createCommandInstance( commandName );
+    command->setSource( source );
+    QJson::QObjectHelper::qvariant2qobject( op.toMap(), command );
+    return command;
+}
+

src/libtomahawk/database/Database.h

@@ -35,6 +35,29 @@ class DatabaseWorkerThread;
 class DatabaseWorker;
 class IdThreadWorker;
 
+
+class DLLEXPORT DatabaseCommandFactory : public QObject
+{
+Q_OBJECT
+
+public:
+    virtual ~DatabaseCommandFactory() {};
+    DatabaseCommand* newInstance();
+
+signals:
+    void created( DatabaseCommand* command );
+
+protected:
+    virtual DatabaseCommand* create() const = 0;
+};
+
+template <class COMMAND>
+class DatabaseCommandFactoryImplementation : public DatabaseCommandFactory
+{
+protected:
+  virtual COMMAND* create() const { return new COMMAND(); };
+};
+
 /*
     This class is really a firewall/pimpl - the public functions of LibraryImpl
     are the ones that operate on the database, without any locks.
@@ -60,6 +83,19 @@ public:
 
     DatabaseImpl* impl();
 
+    DatabaseCommand* createCommandInstance( const QVariant& op, const Tomahawk::source_ptr& source );
+
+    // Template implementations need to stay in header!
+    template<typename T> void registerCommand()
+    {
+        registerCommand( new DatabaseCommandFactoryImplementation<T>() );
+    }
+
+    template<typename T> DatabaseCommandFactory* commandFactory()
+    {
+        return commandFactoryByClassName( T::staticMetaObject.className() );
+    }
+
 signals:
     void indexReady(); // search index
     void ready();
@@ -75,6 +111,11 @@ private slots:
     void markAsReady();
 
 private:
+    void registerCommand( DatabaseCommandFactory* commandFactory );
+    DatabaseCommandFactory* commandFactoryByClassName( const QString& className );
+    DatabaseCommandFactory* commandFactoryByCommandName( const QString& commandName );
+    DatabaseCommand* createCommandInstance( const QString& commandName );
+
     bool m_ready;
 
     DatabaseImpl* m_impl;
@@ -83,6 +124,9 @@ private:
     IdThreadWorker* m_idWorker;
     int m_maxConcurrentThreads;
 
+    QHash< QString, DatabaseCommandFactory* > m_commandFactories;
+    QHash< QString, QString> m_commandNameClassNameMapping;
+
     QHash< QThread*, DatabaseImpl* > m_implHash;
     QMutex m_mutex;
 

src/libtomahawk/database/DatabaseCommand.cpp

@@ -20,25 +20,7 @@
 
 #include "utils/Logger.h"
 
-#include "DatabaseCommand_AddFiles.h"
-#include "DatabaseCommand_CreatePlaylist.h"
-#include "DatabaseCommand_DeleteFiles.h"
-#include "DatabaseCommand_DeletePlaylist.h"
-#include "DatabaseCommand_LogPlayback.h"
-#include "DatabaseCommand_RenamePlaylist.h"
-#include "DatabaseCommand_SetPlaylistRevision.h"
-#include "DatabaseCommand_CreateDynamicPlaylist.h"
-#include "DatabaseCommand_DeleteDynamicPlaylist.h"
-#include "DatabaseCommand_SetDynamicPlaylistRevision.h"
-#include "DatabaseCommand_SocialAction.h"
-#include "DatabaseCommand_ShareTrack.h"
-#include "DatabaseCommand_SetCollectionAttributes.h"
-#include "DatabaseCommand_SetTrackAttributes.h"
 
-// Forward Declarations breaking QSharedPointer
-#if QT_VERSION < QT_VERSION_CHECK( 5, 0, 0 )
-    #include "collection/Collection.h"
-#endif
 
 
 DatabaseCommand::DatabaseCommand( QObject* parent )
@@ -49,7 +31,7 @@ DatabaseCommand::DatabaseCommand( QObject* parent )
 }
 
 
-DatabaseCommand::DatabaseCommand( const source_ptr& src, QObject* parent )
+DatabaseCommand::DatabaseCommand( const Tomahawk::source_ptr& src, QObject* parent )
     : QObject( parent )
     , m_state( PENDING )
     , m_source( src )
@@ -73,7 +55,7 @@ DatabaseCommand::_exec( DatabaseImpl* lib )
 {
     //qDebug() << "RUNNING" << thread();
     m_state = RUNNING;
-    emit running();
+    emitRunning();
     exec( lib );
     m_state = FINISHED;
     //qDebug() << "FINISHED" << thread();
@@ -92,114 +74,3 @@ DatabaseCommand::source() const
 {
     return m_source;
 }
-
-
-DatabaseCommand*
-DatabaseCommand::factory( const QVariant& op, const source_ptr& source )
-{
-    const QString name = op.toMap().value( "command" ).toString();
-
-    if( name == "addfiles" )
-    {
-        DatabaseCommand_AddFiles * cmd = new DatabaseCommand_AddFiles;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "deletefiles" )
-    {
-        DatabaseCommand_DeleteFiles * cmd = new DatabaseCommand_DeleteFiles;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "createplaylist" )
-    {
-        DatabaseCommand_CreatePlaylist * cmd = new DatabaseCommand_CreatePlaylist;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "deleteplaylist" )
-    {
-        DatabaseCommand_DeletePlaylist * cmd = new DatabaseCommand_DeletePlaylist;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "logplayback" )
-    {
-        DatabaseCommand_LogPlayback * cmd = new DatabaseCommand_LogPlayback;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "renameplaylist" )
-    {
-        DatabaseCommand_RenamePlaylist * cmd = new DatabaseCommand_RenamePlaylist;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "setplaylistrevision" )
-    {
-        DatabaseCommand_SetPlaylistRevision * cmd = new DatabaseCommand_SetPlaylistRevision;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "createdynamicplaylist" )
-    {
-        DatabaseCommand_CreateDynamicPlaylist * cmd = new DatabaseCommand_CreateDynamicPlaylist;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "deletedynamicplaylist" )
-    {
-        DatabaseCommand_DeleteDynamicPlaylist * cmd = new DatabaseCommand_DeleteDynamicPlaylist;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "setdynamicplaylistrevision" )
-    {
-        qDebug() << "SETDYN CONTENT:" << op;
-        DatabaseCommand_SetDynamicPlaylistRevision * cmd = new DatabaseCommand_SetDynamicPlaylistRevision;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "socialaction" )
-    {
-        DatabaseCommand_SocialAction * cmd = new DatabaseCommand_SocialAction;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "setcollectionattributes" )
-    {
-        DatabaseCommand_SetCollectionAttributes * cmd = new DatabaseCommand_SetCollectionAttributes;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "settrackattributes" )
-    {
-        DatabaseCommand_SetTrackAttributes * cmd = new DatabaseCommand_SetTrackAttributes;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-    else if( name == "sharetrack" )
-    {
-        DatabaseCommand_ShareTrack * cmd = new DatabaseCommand_ShareTrack;
-        cmd->setSource( source );
-        QJson::QObjectHelper::qvariant2qobject( op.toMap(), cmd );
-        return cmd;
-    }
-
-    qDebug() << "Unknown database command" << name;
-//    Q_ASSERT( false );
-    return NULL;
-}

src/libtomahawk/database/DatabaseCommand.h

@@ -64,7 +64,7 @@ public:
 
     // stuff to do once transaction applied ok.
     // Don't change the database from in here, duh.
-    void postCommit() { postCommitHook(); emit committed(); }
+    void postCommit() { postCommitHook(); emitCommitted(); }
     virtual void postCommitHook(){};
 
     void setSource( const Tomahawk::source_ptr& s );
@@ -87,14 +87,19 @@ public:
     }
     void setGuid( const QString& g ) { m_guid = g; }
 
-    void emitFinished() { emit finished(); }
-
-    static DatabaseCommand* factory( const QVariant& op, const Tomahawk::source_ptr& source );
+    void emitFinished() { emit finished(this); emit finished(); }
+    void emitCommitted() { emit committed(this); emit committed(); }
+    void emitRunning() { emit running(this); emit running(); }
 
 signals:
     void running();
+    void running(DatabaseCommand*);
+
     void finished();
+    void finished(DatabaseCommand*);
+
     void committed();
+    void committed(DatabaseCommand*);
 
 private:
     State m_state;

src/libtomahawk/network/DbSyncConnection.cpp

@@ -199,7 +199,7 @@ DBSyncConnection::handleMsg( msg_ptr msg )
     // a db sync op msg
     if ( msg->is( Msg::DBOP ) )
     {
-        DatabaseCommand* cmd = DatabaseCommand::factory( m, m_source );
+        DatabaseCommand* cmd = Database::instance()->createCommandInstance( m, m_source );
         if ( cmd )
         {
             QSharedPointer<DatabaseCommand> cmdsp = QSharedPointer<DatabaseCommand>(cmd);

src/libtomahawk/resolvers/ExternalResolver.h

@@ -79,6 +79,8 @@ public:
     { m_filePath = filePath; }
 
     virtual QString filePath() const { return m_filePath; }
+    virtual QPixmap icon() const { return QPixmap(); }
+    virtual void setIcon( const QPixmap& ) {}
 
     virtual void saveConfig() = 0;
 

src/libtomahawk/resolvers/ExternalResolverGui.h

@@ -44,9 +44,6 @@ public:
     ExternalResolverGui( const QString& filePath );
     virtual AccountConfigWidget* configUI() const = 0;
 
-    virtual QPixmap icon() const { return QPixmap(); }
-    virtual void setIcon( const QPixmap& ) {}
-
 protected:
     AccountConfigWidget* widgetFromData( QByteArray& data, QWidget* parent = 0 );
     QVariant configMsgFromWidget( QWidget* w );

src/libtomahawk/resolvers/ScriptCollection.cpp

@@ -38,18 +38,12 @@ ScriptCollection::ScriptCollection( const source_ptr& source,
     : Collection( source, QString( "scriptcollection:" + resolver->name() + ":" + uuid() ), parent )
     , m_trackCount( -1 ) //null value
 {
-    Q_ASSERT( resolver != 0 );
+    Q_ASSERT( resolver );
     qDebug() << Q_FUNC_INFO << resolver->name() << name();
 
     m_resolver = resolver;
 
     m_servicePrettyName = m_resolver->name();
-
-    ExternalResolverGui* gResolver = qobject_cast< ExternalResolverGui* >( m_resolver );
-    if ( gResolver )
-    {
-        m_icon = gResolver->icon();
-    }
 }
 
 
@@ -93,7 +87,10 @@ ScriptCollection::setIcon( const QIcon& icon )
 QIcon
 ScriptCollection::icon() const
 {
-    return m_icon;
+    if( !m_icon.isNull() )
+        return m_icon;
+
+    return m_resolver->icon();
 }
 
 

tests/CMakeLists.txt

@@ -3,3 +3,4 @@ include(tomahawk_add_test.cmake)
 
 tomahawk_add_test(Result)
 tomahawk_add_test(Query)
+tomahawk_add_test(Database)

tests/TestDatabase.h

@@ -0,0 +1,69 @@
+/* === This file is part of Tomahawk Player - <http://tomahawk-player.org> ===
+ *
+ *   Copyright 2013, 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 3 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/>.
+ */
+
+#ifndef TOMAHAWK_TESTDATABASE_H
+#define TOMAHAWK_TESTDATABASE_H
+
+#include <QtTest>
+
+#include "database/Database.h"
+#include "database/DatabaseCommand_LogPlayback.h"
+
+
+class TestDatabaseCommand : public DatabaseCommand
+{
+Q_OBJECT
+    virtual QString commandname() const { return "TestCommand"; }
+};
+
+class TestDatabase : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testFactories()
+    {
+        Database* db = new Database("test");
+        DatabaseCommand* command = 0;
+
+        // can we check that his ASSERTs?, it's a build in type, one must not register it again
+        // db->registerCommand<DatabaseCommand_LogPlayback>();
+
+        // check that if we request a factory for LogPlayback it really creates a LogPlayback object
+        command = db->commandFactory<DatabaseCommand_LogPlayback>()->newInstance();
+        DatabaseCommand_LogPlayback* lpCmd =  qobject_cast< DatabaseCommand_LogPlayback* >( command );
+        QVERIFY( lpCmd );
+
+        // try to handle a third party database command
+
+        // test no command factory is available until now
+        QVERIFY( !db->commandFactory<TestDatabaseCommand>() );
+
+        // register it
+        db->registerCommand<TestDatabaseCommand>();
+
+        // make sure it's available now
+        command = db->commandFactory<TestDatabaseCommand>()->newInstance();
+        TestDatabaseCommand* tCmd = qobject_cast< TestDatabaseCommand* >( command );
+        QVERIFY( tCmd );
+
+        delete db;
+    }
+};
+
+#endif // TOMAHAWK_TESTDATABASE_H