From 3b175b3fb3457e2191ecb505adb86c18477eb034 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 5 Jul 2013 21:43:38 +0200 Subject: [PATCH] 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 | 160 +++++++------------ src/libtomahawk/database/Database.h | 24 ++- src/libtomahawk/network/DbSyncConnection.cpp | 2 +- 3 files changed, 79 insertions(+), 107 deletions(-) diff --git a/src/libtomahawk/database/Database.cpp b/src/libtomahawk/database/Database.cpp index a3291e378..4e72a1145 100644 --- a/src/libtomahawk/database/Database.cpp +++ b/src/libtomahawk/database/Database.cpp @@ -71,6 +71,22 @@ Database::Database( const QString& dbname, QObject* parent ) { s_instance = this; + // register commands + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + registerCommand(); + if ( MAX_WORKER_THREADS < DEFAULT_WORKER_THREADS ) m_maxConcurrentThreads = MAX_WORKER_THREADS; else @@ -228,113 +244,47 @@ Database::markAsReady() } -DatabaseCommand* -Database::commandFactory( const QVariant& op, const Tomahawk::source_ptr& source ) +void +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 command( commandFactory->create() ); - 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; - } + const QString commandName = command->commandname(); - qDebug() << "Unknown database command" << name; -// Q_ASSERT( false ); - return NULL; + if( m_commandFactories.keys().contains( commandName ) ) + { + 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; } diff --git a/src/libtomahawk/database/Database.h b/src/libtomahawk/database/Database.h index 9adbc1803..1fc1b77be 100644 --- a/src/libtomahawk/database/Database.h +++ b/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 +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 void registerCommand() + { + registerCommand( new DatabaseCommandFactoryImplementation() ); + } 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; diff --git a/src/libtomahawk/network/DbSyncConnection.cpp b/src/libtomahawk/network/DbSyncConnection.cpp index 3b84c0e79..909d603bc 100644 --- a/src/libtomahawk/network/DbSyncConnection.cpp +++ b/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 cmdsp = QSharedPointer(cmd);