Improve command line parameters dispatching

Encapsulate parameters dispatching in Application class.
Avoid serializing parameters when it is not necessary.

PR #18469.

src/app/application.cpp

@@ -122,6 +122,104 @@ namespace
 #ifndef DISABLE_GUI
     const int PIXMAP_CACHE_SIZE = 64 * 1024 * 1024;  // 64MiB
 #endif
+
+    QString serializeParams(const QBtCommandLineParameters &params)
+    {
+        QStringList result;
+        // Because we're passing a string list to the currently running
+        // qBittorrent process, we need some way of passing along the options
+        // the user has specified. Here we place special strings that are
+        // almost certainly not going to collide with a file path or URL
+        // specified by the user, and placing them at the beginning of the
+        // string list so that they will be processed before the list of
+        // torrent paths or URLs.
+
+        const BitTorrent::AddTorrentParams &addTorrentParams = params.addTorrentParams;
+
+        if (!addTorrentParams.savePath.isEmpty())
+            result.append(u"@savePath=" + addTorrentParams.savePath.data());
+
+        if (addTorrentParams.addPaused.has_value())
+            result.append(*addTorrentParams.addPaused ? u"@addPaused=1"_qs : u"@addPaused=0"_qs);
+
+        if (addTorrentParams.skipChecking)
+            result.append(u"@skipChecking"_qs);
+
+        if (!addTorrentParams.category.isEmpty())
+            result.append(u"@category=" + addTorrentParams.category);
+
+        if (addTorrentParams.sequential)
+            result.append(u"@sequential"_qs);
+
+        if (addTorrentParams.firstLastPiecePriority)
+            result.append(u"@firstLastPiecePriority"_qs);
+
+        if (params.skipDialog.has_value())
+            result.append(*params.skipDialog ? u"@skipDialog=1"_qs : u"@skipDialog=0"_qs);
+
+        result += params.torrentSources;
+
+        return result.join(PARAMS_SEPARATOR);
+    }
+
+    QBtCommandLineParameters parseParams(const QString &str)
+    {
+        QBtCommandLineParameters parsedParams;
+        BitTorrent::AddTorrentParams &addTorrentParams = parsedParams.addTorrentParams;
+
+        for (QString param : asConst(str.split(PARAMS_SEPARATOR, Qt::SkipEmptyParts)))
+        {
+            param = param.trimmed();
+
+            // Process strings indicating options specified by the user.
+
+            if (param.startsWith(u"@savePath="))
+            {
+                addTorrentParams.savePath = Path(param.mid(10));
+                continue;
+            }
+
+            if (param.startsWith(u"@addPaused="))
+            {
+                addTorrentParams.addPaused = (QStringView(param).mid(11).toInt() != 0);
+                continue;
+            }
+
+            if (param == u"@skipChecking")
+            {
+                addTorrentParams.skipChecking = true;
+                continue;
+            }
+
+            if (param.startsWith(u"@category="))
+            {
+                addTorrentParams.category = param.mid(10);
+                continue;
+            }
+
+            if (param == u"@sequential")
+            {
+                addTorrentParams.sequential = true;
+                continue;
+            }
+
+            if (param == u"@firstLastPiecePriority")
+            {
+                addTorrentParams.firstLastPiecePriority = true;
+                continue;
+            }
+
+            if (param.startsWith(u"@skipDialog="))
+            {
+                parsedParams.skipDialog = (QStringView(param).mid(12).toInt() != 0);
+                continue;
+            }
+
+            parsedParams.torrentSources.append(param);
+        }
+
+        return parsedParams;
+    }
 }
 
 Application::Application(int &argc, char **argv)
@@ -390,7 +488,7 @@ void Application::processMessage(const QString &message)
     }
 #endif
 
-    const AddTorrentParams params = parseParams(message.split(PARAMS_SEPARATOR, Qt::SkipEmptyParts));
+    const QBtCommandLineParameters params = parseParams(message);
     // If Application is not allowed to process params immediately
     // (i.e., other components are not ready) store params
     if (m_isProcessingParamsAllowed)
@@ -625,71 +723,12 @@ void Application::allTorrentsFinished()
     exit();
 }
 
-bool Application::sendParams(const QStringList &params)
+bool Application::callMainInstance()
 {
-    return m_instanceManager->sendMessage(params.join(PARAMS_SEPARATOR));
+    return m_instanceManager->sendMessage(serializeParams(commandLineArgs()));
 }
 
-Application::AddTorrentParams Application::parseParams(const QStringList &params) const
+void Application::processParams(const QBtCommandLineParameters &params)
-{
-    AddTorrentParams parsedParams;
-    BitTorrent::AddTorrentParams &torrentParams = parsedParams.torrentParams;
-
-    for (QString param : params)
-    {
-        param = param.trimmed();
-
-        // Process strings indicating options specified by the user.
-
-        if (param.startsWith(u"@savePath="))
-        {
-            torrentParams.savePath = Path(param.mid(10));
-            continue;
-        }
-
-        if (param.startsWith(u"@addPaused="))
-        {
-            torrentParams.addPaused = (QStringView(param).mid(11).toInt() != 0);
-            continue;
-        }
-
-        if (param == u"@skipChecking")
-        {
-            torrentParams.skipChecking = true;
-            continue;
-        }
-
-        if (param.startsWith(u"@category="))
-        {
-            torrentParams.category = param.mid(10);
-            continue;
-        }
-
-        if (param == u"@sequential")
-        {
-            torrentParams.sequential = true;
-            continue;
-        }
-
-        if (param == u"@firstLastPiecePriority")
-        {
-            torrentParams.firstLastPiecePriority = true;
-            continue;
-        }
-
-        if (param.startsWith(u"@skipDialog="))
-        {
-            parsedParams.skipTorrentDialog = (QStringView(param).mid(12).toInt() != 0);
-            continue;
-        }
-
-        parsedParams.torrentSources.append(param);
-    }
-
-    return parsedParams;
-}
-
-void Application::processParams(const AddTorrentParams &params)
 {
 #ifndef DISABLE_GUI
     // There are two circumstances in which we want to show the torrent
@@ -697,21 +736,21 @@ void Application::processParams(const AddTorrentParams &params)
     // be shown and skipTorrentDialog is undefined. The other is when
     // skipTorrentDialog is false, meaning that the application setting
     // should be overridden.
-    const bool showDialogForThisTorrent = !params.skipTorrentDialog.value_or(!AddNewTorrentDialog::isEnabled());
+    const bool showDialog = !params.skipDialog.value_or(!AddNewTorrentDialog::isEnabled());
-    if (showDialogForThisTorrent)
+    if (showDialog)
     {
         for (const QString &torrentSource : params.torrentSources)
-            AddNewTorrentDialog::show(torrentSource, params.torrentParams, m_window);
+            AddNewTorrentDialog::show(torrentSource, params.addTorrentParams, m_window);
     }
     else
 #endif
     {
         for (const QString &torrentSource : params.torrentSources)
-            BitTorrent::Session::instance()->addTorrent(torrentSource, params.torrentParams);
+            BitTorrent::Session::instance()->addTorrent(torrentSource, params.addTorrentParams);
     }
 }
 
-int Application::exec(const QStringList &params)
+int Application::exec()
 try
 {
 #if !defined(DISABLE_WEBUI) && defined(DISABLE_GUI)
@@ -870,13 +909,14 @@ try
 #endif // DISABLE_WEBUI
 
         m_isProcessingParamsAllowed = true;
-        for (const AddTorrentParams &params : m_paramsQueue)
+        for (const QBtCommandLineParameters &params : m_paramsQueue)
             processParams(params);
         m_paramsQueue.clear();
     });
 
-    if (!params.isEmpty())
+    const QBtCommandLineParameters params = commandLineArgs();
-        m_paramsQueue.append(parseParams(params));
+    if (!params.torrentSources.isEmpty())
+        m_paramsQueue.append(params);
 
     return BaseApplication::exec();
 }
@@ -962,7 +1002,8 @@ bool Application::event(QEvent *ev)
             path = static_cast<QFileOpenEvent *>(ev)->url().toString();
         qDebug("Received a mac file open event: %s", qUtf8Printable(path));
 
-        const AddTorrentParams params = parseParams({path});
+        QBtCommandLineParameters params;
+        params.torrentSources.append(path);
         // If Application is not allowed to process params immediately
         // (i.e., other components are not ready) store params
         if (m_isProcessingParamsAllowed)

src/app/application.h

@@ -96,10 +96,10 @@ public:
     Application(int &argc, char **argv);
     ~Application() override;
 
-    int exec(const QStringList &params);
+    int exec();
 
     bool isRunning();
-    bool sendParams(const QStringList &params);
+    bool callMainInstance();
     const QBtCommandLineParameters &commandLineArgs() const;
 
     // FileLogger properties
@@ -149,16 +149,8 @@ private slots:
 #endif
 
 private:
-    struct AddTorrentParams
-    {
-        QStringList torrentSources;
-        BitTorrent::AddTorrentParams torrentParams;
-        std::optional<bool> skipTorrentDialog;
-    };
-
     void initializeTranslation();
-    AddTorrentParams parseParams(const QStringList &params) const;
+    void processParams(const QBtCommandLineParameters &params);
-    void processParams(const AddTorrentParams &params);
     void runExternalProgram(const QString &programTemplate, const BitTorrent::Torrent *torrent) const;
     void sendNotificationEmail(const BitTorrent::Torrent *torrent);
 
@@ -190,7 +182,7 @@ private:
     QTranslator m_qtTranslator;
     QTranslator m_translator;
 
-    QList<AddTorrentParams> m_paramsQueue;
+    QList<QBtCommandLineParameters> m_paramsQueue;
 
     SettingValue<bool> m_storeFileLoggerEnabled;
     SettingValue<bool> m_storeFileLoggerBackup;

src/app/cmdoptions.cpp

@@ -340,14 +340,7 @@ namespace
 }
 
 QBtCommandLineParameters::QBtCommandLineParameters(const QProcessEnvironment &env)
-    : showHelp(false)
+    : relativeFastresumePaths(RELATIVE_FASTRESUME.value(env))
-    , relativeFastresumePaths(RELATIVE_FASTRESUME.value(env))
-    , skipChecking(SKIP_HASH_CHECK_OPTION.value(env))
-    , sequential(SEQUENTIAL_OPTION.value(env))
-    , firstLastPiecePriority(FIRST_AND_LAST_OPTION.value(env))
-#if !defined(Q_OS_WIN) || defined(DISABLE_GUI)
-    , showVersion(false)
-#endif
 #ifndef DISABLE_GUI
     , noSplash(NO_SPLASH_OPTION.value(env))
 #elif !defined(Q_OS_WIN)
@@ -355,49 +348,16 @@ QBtCommandLineParameters::QBtCommandLineParameters(const QProcessEnvironment &en
 #endif
     , webUiPort(WEBUI_PORT_OPTION.value(env, -1))
     , torrentingPort(TORRENTING_PORT_OPTION.value(env, -1))
-    , addPaused(PAUSED_OPTION.value(env))
     , skipDialog(SKIP_DIALOG_OPTION.value(env))
     , profileDir(PROFILE_OPTION.value(env))
     , configurationName(CONFIGURATION_OPTION.value(env))
-    , savePath(SAVE_PATH_OPTION.value(env))
-    , category(CATEGORY_OPTION.value(env))
 {
-}
+    addTorrentParams.savePath = Path(SAVE_PATH_OPTION.value(env));
-
+    addTorrentParams.category = CATEGORY_OPTION.value(env);
-QStringList QBtCommandLineParameters::paramList() const
+    addTorrentParams.skipChecking = SKIP_HASH_CHECK_OPTION.value(env);
-{
+    addTorrentParams.sequential = SEQUENTIAL_OPTION.value(env);
-    QStringList result;
+    addTorrentParams.firstLastPiecePriority = FIRST_AND_LAST_OPTION.value(env);
-    // Because we're passing a string list to the currently running
+    addTorrentParams.addPaused = PAUSED_OPTION.value(env);
-    // qBittorrent process, we need some way of passing along the options
-    // the user has specified. Here we place special strings that are
-    // almost certainly not going to collide with a file path or URL
-    // specified by the user, and placing them at the beginning of the
-    // string list so that they will be processed before the list of
-    // torrent paths or URLs.
-
-    if (!savePath.isEmpty())
-        result.append(u"@savePath=" + savePath.data());
-
-    if (addPaused.has_value())
-        result.append(*addPaused ? u"@addPaused=1"_qs : u"@addPaused=0"_qs);
-
-    if (skipChecking)
-        result.append(u"@skipChecking"_qs);
-
-    if (!category.isEmpty())
-        result.append(u"@category=" + category);
-
-    if (sequential)
-        result.append(u"@sequential"_qs);
-
-    if (firstLastPiecePriority)
-        result.append(u"@firstLastPiecePriority"_qs);
-
-    if (skipDialog.has_value())
-        result.append(*skipDialog ? u"@skipDialog=1"_qs : u"@skipDialog=0"_qs);
-
-    result += torrents;
-    return result;
 }
 
 QBtCommandLineParameters parseCommandLine(const QStringList &args)
@@ -463,27 +423,27 @@ QBtCommandLineParameters parseCommandLine(const QStringList &args)
             }
             else if (arg == SAVE_PATH_OPTION)
             {
-                result.savePath = Path(SAVE_PATH_OPTION.value(arg));
+                result.addTorrentParams.savePath = Path(SAVE_PATH_OPTION.value(arg));
             }
             else if (arg == PAUSED_OPTION)
             {
-                result.addPaused = PAUSED_OPTION.value(arg);
+                result.addTorrentParams.addPaused = PAUSED_OPTION.value(arg);
             }
             else if (arg == SKIP_HASH_CHECK_OPTION)
             {
-                result.skipChecking = true;
+                result.addTorrentParams.skipChecking = true;
             }
             else if (arg == CATEGORY_OPTION)
             {
-                result.category = CATEGORY_OPTION.value(arg);
+                result.addTorrentParams.category = CATEGORY_OPTION.value(arg);
             }
             else if (arg == SEQUENTIAL_OPTION)
             {
-                result.sequential = true;
+                result.addTorrentParams.sequential = true;
             }
             else if (arg == FIRST_AND_LAST_OPTION)
             {
-                result.firstLastPiecePriority = true;
+                result.addTorrentParams.firstLastPiecePriority = true;
             }
             else if (arg == SKIP_DIALOG_OPTION)
             {
@@ -502,9 +462,9 @@ QBtCommandLineParameters parseCommandLine(const QStringList &args)
             torrentPath.setFile(arg);
 
             if (torrentPath.exists())
-                result.torrents += torrentPath.absoluteFilePath();
+                result.torrentSources += torrentPath.absoluteFilePath();
             else
-                result.torrents += arg;
+                result.torrentSources += arg;
         }
     }
 

src/app/cmdoptions.h

@@ -35,6 +35,7 @@
 #include <QString>
 #include <QStringList>
 
+#include "base/bittorrent/addtorrentparams.h"
 #include "base/exceptions.h"
 #include "base/path.h"
 
@@ -42,32 +43,29 @@ class QProcessEnvironment;
 
 struct QBtCommandLineParameters
 {
-    bool showHelp;
+    bool showHelp = false;
-    bool relativeFastresumePaths;
+    bool relativeFastresumePaths = false;
-    bool skipChecking;
-    bool sequential;
-    bool firstLastPiecePriority;
 #if !defined(Q_OS_WIN) || defined(DISABLE_GUI)
-    bool showVersion;
+    bool showVersion = false;
 #endif
 #ifndef DISABLE_GUI
-    bool noSplash;
+    bool noSplash = false;
 #elif !defined(Q_OS_WIN)
-    bool shouldDaemonize;
+    bool shouldDaemonize = false;
 #endif
-    int webUiPort;
+    int webUiPort = -1;
-    int torrentingPort;
+    int torrentingPort = -1;
-    std::optional<bool> addPaused;
     std::optional<bool> skipDialog;
-    QStringList torrents;
     Path profileDir;
     QString configurationName;
-    Path savePath;
+
-    QString category;
+    QStringList torrentSources;
+    BitTorrent::AddTorrentParams addTorrentParams;
+
     QString unknownParameter;
 
+    QBtCommandLineParameters() = default;
     explicit QBtCommandLineParameters(const QProcessEnvironment &);
-    QStringList paramList() const;
 };
 
 class CommandLineParameterError : public RuntimeError

src/app/main.cpp

@@ -193,7 +193,7 @@ int main(int argc, char *argv[])
 #endif
 
             QThread::msleep(300);
-            app->sendParams(params.paramList());
+            app->callMainInstance();
 
             return EXIT_SUCCESS;
         }
@@ -258,7 +258,7 @@ int main(int argc, char *argv[])
 
         registerSignalHandlers();
 
-        return app->exec(params.paramList());
+        return app->exec();
     }
     catch (const CommandLineParameterError &er)
     {