Use factories for creatig databasecommands in database

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/database/Database.cpp

@@ -71,6 +71,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
@@ -228,113 +244,47 @@ Database::markAsReady()
 }
 
 
-DatabaseCommand*
+void
-Database::commandFactory( const QVariant& op, const Tomahawk::source_ptr& source )
+Database::registerCommand( DatabaseCommandFactory* commandFactory )
 {
-    const QString name = op.toMap().value( "command" ).toString();
+    // this is ugly, but we don't have virtual static methods in C++ :(
+    QScopedPointer<DatabaseCommand> command( commandFactory->create() );
 
-    if( name == "addfiles" )
+    const QString commandName = command->commandname();
-    {
-        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;
+    if( m_commandFactories.keys().contains( commandName ) )
-//    Q_ASSERT( false );
+    {
-    return NULL;
+        tLog() << commandName << "is already in " << m_commandFactories.keys();
+    }
+    Q_ASSERT( !m_commandFactories.keys().contains( commandName ) );
+
+    m_commandFactories.insert( commandName, commandFactory );
+}
+
+
+DatabaseCommand*
+Database::createCommandInstance( const QString& commandName )
+{
+    DatabaseCommandFactory* factory = m_commandFactories.value( commandName );
+
+    if( !factory )
+    {
+         tLog() << "Unknown database command" << commandName;
+         return 0;
+    }
+
+    return factory->create();
+}
+
+
+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,18 @@ class DatabaseWorkerThread;
 class DatabaseWorker;
 class IdThreadWorker;
 
+
+struct DLLEXPORT DatabaseCommandFactory {
+  virtual ~DatabaseCommandFactory() {};
+  virtual DatabaseCommand* create() const = 0;
+};
+
+template <class COMMAND>
+struct DatabaseCommandFactoryImplementation : public DatabaseCommandFactory
+{
+  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,7 +72,13 @@ public:
 
     DatabaseImpl* impl();
 
-    static DatabaseCommand* commandFactory( const QVariant& op, const Tomahawk::source_ptr& source );
+    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>() );
+    }
 
 signals:
     void indexReady(); // search index
@@ -77,6 +95,9 @@ private slots:
     void markAsReady();
 
 private:
+    void registerCommand( DatabaseCommandFactory* commandFactory );
+    DatabaseCommand* createCommandInstance( const QString& commandName );
+
     bool m_ready;
 
     DatabaseImpl* m_impl;
@@ -84,6 +105,7 @@ private:
     QList< QPointer< DatabaseWorkerThread > > m_workerThreads;
     IdThreadWorker* m_idWorker;
     int m_maxConcurrentThreads;
+    QHash< QString, DatabaseCommandFactory* > m_commandFactories;
 
     QHash< QThread*, DatabaseImpl* > m_implHash;
     QMutex m_mutex;

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 = Database::commandFactory( m, m_source );
+        DatabaseCommand* cmd = Database::instance()->createCommandInstance( m, m_source );
         if ( cmd )
         {
             QSharedPointer<DatabaseCommand> cmdsp = QSharedPointer<DatabaseCommand>(cmd);