fix: improve the commu between gig module and core

4 files changed, 130 insertions, 88 deletions

diff --git a/src/core/function/gpg/GpgAdvancedOperator.cpp b/src/core/function/gpg/GpgAdvancedOperator.cpp
index 9f8de193..8bb558c1 100644
--- a/src/core/function/gpg/GpgAdvancedOperator.cpp
+++ b/src/core/function/gpg/GpgAdvancedOperator.cpp
@@ -46,6 +46,11 @@ bool GpgFrontend::GpgAdvancedOperator::ClearGpgPasswordCache() {
       "core", "gpgme.ctx.gpgconf_path", std::string{});
   SPDLOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
 
+  if (gpgconf_path.empty()) {
+    SPDLOG_ERROR("cannot get valid gpgconf path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {gpgconf_path,
        {"--reload", "gpg-agent"},
@@ -65,6 +70,11 @@ bool GpgFrontend::GpgAdvancedOperator::ReloadGpgComponents() {
       "core", "gpgme.ctx.gpgconf_path", std::string{});
   SPDLOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
 
+  if (gpgconf_path.empty()) {
+    SPDLOG_ERROR("cannot get valid gpgconf path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {gpgconf_path,
        {"--reload"},
@@ -86,6 +96,11 @@ void GpgFrontend::GpgAdvancedOperator::RestartGpgComponents() {
       "core", "gpgme.ctx.gpgconf_path", std::string{});
   SPDLOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
 
+  if (gpgconf_path.empty()) {
+    SPDLOG_ERROR("cannot get valid gpgconf path from rt, abort.");
+    return;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {gpgconf_path,
        {"--verbose", "--kill", "all"},
@@ -138,6 +153,11 @@ bool GpgFrontend::GpgAdvancedOperator::ResetConfigures() {
       "core", "gpgme.ctx.gpgconf_path", std::string{});
   SPDLOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
 
+  if (gpgconf_path.empty()) {
+    SPDLOG_ERROR("cannot get valid gpgconf path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {gpgconf_path,
        {"--apply-defaults"},
@@ -170,6 +190,11 @@ bool GpgFrontend::GpgAdvancedOperator::StartGpgAgent() {
       "gnupg.home_path", std::string{});
   SPDLOG_DEBUG("got gnupg home path from rt: {}", home_path);
 
+  if (gpg_agent_path.empty()) {
+    SPDLOG_ERROR("cannot get valid gpg agent path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {gpg_agent_path,
        {"--homedir", home_path, "--daemon"},
@@ -207,6 +232,11 @@ bool GpgFrontend::GpgAdvancedOperator::StartDirmngr() {
       "gnupg.home_path", std::string{});
   SPDLOG_DEBUG("got gnupg home path from rt: {}", home_path);
 
+  if (dirmngr_path.empty()) {
+    SPDLOG_ERROR("cannot get valid dirmngr path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {dirmngr_path,
        {"--homedir", home_path, "--daemon"},
@@ -243,6 +273,11 @@ bool GpgFrontend::GpgAdvancedOperator::StartKeyBoxd() {
       "gnupg.home_path", std::string{});
   SPDLOG_DEBUG("got gnupg home path from rt: {}", home_path);
 
+  if (keyboxd_path.empty()) {
+    SPDLOG_ERROR("cannot get valid keyboxd path from rt, abort.");
+    return false;
+  }
+
   GpgFrontend::GpgCommandExecutor::GetInstance().ExecuteSync(
       {keyboxd_path,
        {"--homedir", home_path, "--daemon"},
diff --git a/src/core/function/gpg/GpgCommandExecutor.cpp b/src/core/function/gpg/GpgCommandExecutor.cpp
index 582f8afa..3fd56d35 100644
--- a/src/core/function/gpg/GpgCommandExecutor.cpp
+++ b/src/core/function/gpg/GpgCommandExecutor.cpp
@@ -28,18 +28,26 @@
 #include "GpgCommandExecutor.h"
 
 #include <boost/format.hpp>
+#include <string>
 
 #include "GpgFunctionObject.h"
 #include "core/thread/DataObject.h"
 #include "core/thread/TaskRunnerGetter.h"
+#include "spdlog/spdlog.h"
 
 namespace GpgFrontend {
 
+GpgCommandExecutor::ExecuteContext::ExecuteContext(
+    std::string cmd, std::vector<std::string> arguments,
+    GpgCommandExecutorCallback callback, GpgCommandExecutorInteractor int_func)
+    : cmd(cmd), arguments(arguments), cb_func(callback), int_func(int_func) {}
+
 GpgCommandExecutor::GpgCommandExecutor(int channel)
     : SingletonFunctionObject<GpgCommandExecutor>(channel) {}
 
 void GpgCommandExecutor::ExecuteSync(ExecuteContext context) {
-  Thread::Task *task = build_task(context);
+  Thread::Task *task = build_task_from_exec_ctx(context);
+
   QEventLoop looper;
   QObject::connect(task, &Thread::Task::SignalTaskEnd, &looper,
                    &QEventLoop::quit);
@@ -58,7 +66,7 @@ void GpgCommandExecutor::ExecuteConcurrentlyAsync(ExecuteContexts contexts) {
     auto &cmd = context.cmd;
     SPDLOG_INFO("gpg concurrently called cmd {}", cmd);
 
-    Thread::Task *task = build_task(context);
+    Thread::Task *task = build_task_from_exec_ctx(context);
     GpgFrontend::Thread::TaskRunnerGetter::GetInstance()
         .GetTaskRunner(
             Thread::TaskRunnerGetter::kTaskRunnerType_External_Process)
@@ -75,7 +83,7 @@ void GpgCommandExecutor::ExecuteConcurrentlySync(
     auto &cmd = context.cmd;
     SPDLOG_INFO("gpg concurrently called cmd {}", cmd);
 
-    Thread::Task *task = build_task(context);
+    Thread::Task *task = build_task_from_exec_ctx(context);
 
     QObject::connect(task, &Thread::Task::SignalTaskEnd, [&]() {
       --remainingTasks;
@@ -93,11 +101,12 @@ void GpgCommandExecutor::ExecuteConcurrentlySync(
   looper.exec();
 }
 
-Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
+Thread::Task *GpgCommandExecutor::build_task_from_exec_ctx(
+    const ExecuteContext &context) {
   auto &cmd = context.cmd;
   auto &arguments = context.arguments;
-  auto &interact_function = context.interact_func;
-  auto &callback = context.callback;
+  auto &interact_function = context.int_func;
+  auto &cmd_executor_callback = context.cb_func;
 
   const std::string joined_argument = std::accumulate(
       std::begin(arguments), std::end(arguments), std::string(),
@@ -109,9 +118,10 @@ Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
                arguments.size());
 
   Thread::Task::TaskCallback result_callback =
-      [](int rtn, Thread::DataObjectPtr data_object) {
-        SPDLOG_DEBUG("data object args count: {}",
-                     data_object->GetObjectSize());
+      [cmd, joined_argument](int rtn, Thread::DataObjectPtr data_object) {
+        SPDLOG_DEBUG(
+            "data object args count of cmd executor result callback: {}",
+            data_object->GetObjectSize());
         if (!data_object->Check<int, std::string, std::string,
                                 GpgCommandExecutorCallback>())
           throw std::runtime_error("invalid data object size");
@@ -124,8 +134,11 @@ Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
         auto callback =
             Thread::ExtractParams<GpgCommandExecutorCallback>(data_object, 3);
 
-        SPDLOG_DEBUG("data object args got, exit_code: {}", exit_code);
         // call callback
+        SPDLOG_DEBUG(
+            "calling custom callback from caller of cmd {} {}, "
+            "exit_code: {}",
+            cmd, joined_argument, exit_code);
         callback(exit_code, process_stdout, process_stderr);
       };
 
@@ -151,52 +164,33 @@ Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
     auto *cmd_process = new QProcess();
     cmd_process->moveToThread(QThread::currentThread());
     cmd_process->setProcessChannelMode(QProcess::MergedChannels);
+    cmd_process->setProgram(QString::fromStdString(cmd));
+    QStringList q_arguments;
+    for (const auto &argument : arguments)
+      q_arguments.append(QString::fromStdString(argument));
+    cmd_process->setArguments(q_arguments);
 
-    QObject::connect(cmd_process, &QProcess::started,
-                     []() -> void { SPDLOG_DEBUG("process started"); });
+    QObject::connect(
+        cmd_process, &QProcess::started, [cmd, joined_argument]() -> void {
+          SPDLOG_DEBUG(
+              "\n== Process Execute Started ==\nCommand: {}\nArguments: "
+              "{}\n========================",
+              cmd, joined_argument);
+        });
     QObject::connect(
         cmd_process, &QProcess::readyReadStandardOutput,
         [interact_func, cmd_process]() { interact_func(cmd_process); });
-    QObject::connect(cmd_process, &QProcess::errorOccurred,
-                     [=](QProcess::ProcessError error) {
-                       SPDLOG_ERROR("error in executing command: {} error: {}",
-                                    cmd, error);
-                     });
     QObject::connect(
-        cmd_process, qOverload<int, QProcess::ExitStatus>(&QProcess::finished),
-        [=](int, QProcess::ExitStatus status) {
-          int exit_code = cmd_process->exitCode();
-          if (status == QProcess::NormalExit)
-            SPDLOG_DEBUG(
-                "proceess finished, succeed in executing command: {} {}, exit "
-                "code: {}",
-                cmd, joined_argument, exit_code);
-          else
-            SPDLOG_ERROR(
-                "proceess finished, error in executing command: {} {}, exit "
-                "code: {}",
-                cmd, joined_argument, exit_code);
-          std::string process_stdout =
-                          cmd_process->readAllStandardOutput().toStdString(),
-                      process_stderr =
-                          cmd_process->readAllStandardError().toStdString();
-
-          cmd_process->close();
-          cmd_process->deleteLater();
-
-          data_object->Swap(
-              {exit_code, process_stdout, process_stderr, callback});
+        cmd_process, &QProcess::errorOccurred,
+        [=](QProcess::ProcessError error) {
+          SPDLOG_ERROR("caught error while executing command: {} {}, error: {}",
+                       cmd, joined_argument, error);
         });
 
-    cmd_process->setProgram(QString::fromStdString(cmd));
-
-    QStringList q_arguments;
-    for (const auto &argument : arguments)
-      q_arguments.append(QString::fromStdString(argument));
-    cmd_process->setArguments(q_arguments);
-
-    SPDLOG_DEBUG("process execute ready, cmd: {} {}", cmd,
-                 q_arguments.join(" ").toStdString());
+    SPDLOG_DEBUG(
+        "\n== Process Execute Ready ==\nCommand: {}\nArguments: "
+        "{}\n========================",
+        cmd, joined_argument);
 
     cmd_process->start();
     cmd_process->waitForFinished();
@@ -207,6 +201,18 @@ Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
                     cmd_process->readAllStandardError().toStdString();
     int exit_code = cmd_process->exitCode();
 
+    SPDLOG_DEBUG(
+        "\n==== Process Execution Summary ====\n"
+        "Command: {}\n"
+        "Arguments: {}\n"
+        "Exit Code: {}\n"
+        "---- Standard Output ----\n"
+        "{}\n"
+        "---- Standard Error ----\n"
+        "{}\n"
+        "===============================",
+        cmd, joined_argument, exit_code, process_stdout, process_stderr);
+
     cmd_process->close();
     cmd_process->deleteLater();
 
@@ -216,8 +222,10 @@ Thread::Task *GpgCommandExecutor::build_task(const ExecuteContext &context) {
 
   return new Thread::Task(
       std::move(runner),
-      (boost::format("Execute(%1%){%2%}") % cmd % joined_argument).str(),
-      Thread::TransferParams(cmd, arguments, interact_function, callback),
+      (boost::format("GpgCommamdExecutor(%1%){%2%}") % cmd % joined_argument)
+          .str(),
+      Thread::TransferParams(cmd, arguments, interact_function,
+                             cmd_executor_callback),
       std::move(result_callback));
 }
 
diff --git a/src/core/function/gpg/GpgCommandExecutor.h b/src/core/function/gpg/GpgCommandExecutor.h
index f8dc3a9c..bd356b8b 100644
--- a/src/core/function/gpg/GpgCommandExecutor.h
+++ b/src/core/function/gpg/GpgCommandExecutor.h
@@ -53,18 +53,14 @@ class GPGFRONTEND_CORE_EXPORT GpgCommandExecutor
   struct ExecuteContext {
     const std::string cmd;
     const std::vector<std::string> arguments;
-    const GpgCommandExecutorCallback callback;
-    const GpgCommandExecutorInteractor interact_func;
+    const GpgCommandExecutorCallback cb_func;
+    const GpgCommandExecutorInteractor int_func;
 
     ExecuteContext(
         std::string cmd, std::vector<std::string> arguments,
         GpgCommandExecutorCallback callback = [](int, std::string,
                                                  std::string) {},
-        GpgCommandExecutorInteractor interact_func = [](QProcess *) {})
-        : cmd(cmd),
-          arguments(arguments),
-          callback(callback),
-          interact_func(interact_func) {}
+        GpgCommandExecutorInteractor int_func = [](QProcess *) {});
   };
 
   using ExecuteContexts = std::vector<ExecuteContext>;
@@ -93,7 +89,7 @@ class GPGFRONTEND_CORE_EXPORT GpgCommandExecutor
   GpgContext &ctx_ = GpgContext::GetInstance(
       SingletonFunctionObject::GetChannel());  ///< Corresponding context
 
-  Thread::Task *build_task(const ExecuteContext &);
+  Thread::Task *build_task_from_exec_ctx(const ExecuteContext &);
 };
 
 }  // namespace GpgFrontend
diff --git a/src/module/integrated/gnupg_info_gathering_module/GnuPGInfoGatheringModule.cpp b/src/module/integrated/gnupg_info_gathering_module/GnuPGInfoGatheringModule.cpp
index e8c4cf34..9046d3b1 100644
--- a/src/module/integrated/gnupg_info_gathering_module/GnuPGInfoGatheringModule.cpp
+++ b/src/module/integrated/gnupg_info_gathering_module/GnuPGInfoGatheringModule.cpp
@@ -38,16 +38,16 @@ std::optional<std::string> check_binary_chacksum(std::filesystem::path path) {
   // check file info and access rights
   QFileInfo info(QString::fromStdString(path.u8string()));
   if (!info.exists() || !info.isFile() || !info.isReadable()) {
-    SPDLOG_ERROR("get info for file {} error, exists: {}",
-                 info.filePath().toStdString(), info.exists());
+    MODULE_LOG_ERROR("get info for file {} error, exists: {}",
+                     info.filePath().toStdString(), info.exists());
     return {};
   }
 
   // open and read file
   QFile f(info.filePath());
   if (!f.open(QIODevice::ReadOnly)) {
-    SPDLOG_ERROR("open {} to calculate check sum error: {}", path.u8string(),
-                 f.errorString().toStdString());
+    MODULE_LOG_ERROR("open {} to calculate check sum error: {}",
+                     path.u8string(), f.errorString().toStdString());
     return {};
   }
 
@@ -59,7 +59,7 @@ std::optional<std::string> check_binary_chacksum(std::filesystem::path path) {
   // md5
   hash_sha.addData(buffer);
   auto sha = hash_sha.result().toHex().toStdString();
-  SPDLOG_DEBUG("checksum for file {} is {}", path.u8string(), sha);
+  MODULE_LOG_DEBUG("checksum for file {} is {}", path.u8string(), sha);
 
   return sha.substr(0, 6);
 }
@@ -89,13 +89,13 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
 
   const auto gpgme_version = RetrieveRTValueTypedOrDefault<>(
       "core", "gpgme.version", std::string{"2.0.0"});
-  SPDLOG_DEBUG("got gpgme version from rt: {}", gpgme_version);
+  MODULE_LOG_DEBUG("got gpgme version from rt: {}", gpgme_version);
 
   const auto gpgconf_path = RetrieveRTValueTypedOrDefault<>(
       "core", "gpgme.ctx.gpgconf_path", std::string{});
-  SPDLOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
+  MODULE_LOG_DEBUG("got gpgconf path from rt: {}", gpgconf_path);
 
-  SPDLOG_DEBUG("start to load extra info...");
+  MODULE_LOG_DEBUG("start to load extra info at module gnupginfogathering...");
 
   // get all components
   GpgCommandExecutor::GetInstance().ExecuteSync(
@@ -103,12 +103,12 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
        {"--list-components"},
        [this, gpgme_version, gpgconf_path](
            int exit_code, const std::string &p_out, const std::string &p_err) {
-         SPDLOG_DEBUG(
+         MODULE_LOG_DEBUG(
              "gpgconf components exit_code: {} process stdout size: {}",
              exit_code, p_out.size());
 
          if (exit_code != 0) {
-           SPDLOG_ERROR(
+           MODULE_LOG_ERROR(
                "gpgconf execute error, process stderr: {} ,process stdout: "
                "{}",
                p_err, p_out);
@@ -149,7 +149,7 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
 
            auto binary_checksum = check_binary_chacksum(component_path);
 
-           SPDLOG_DEBUG(
+           MODULE_LOG_DEBUG(
                "gnupg component name: {} desc: {} checksum: {} path: {} ",
                component_name, component_desc,
                binary_checksum.has_value() ? binary_checksum.value() : "/",
@@ -175,17 +175,18 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
            }
 
            {
-             // try lock
-             std::unique_lock lock(info_.Lock);
              // add component info to list
              components_info[component_name] = {
                  component_desc, version, component_path,
                  binary_checksum.has_value() ? binary_checksum.value() : "/"};
            }
+
+           MODULE_LOG_DEBUG(
+               "gathering module have loaded extra info from gpgconf.");
          }
        }});
 
-  SPDLOG_DEBUG("start to get dirs info");
+  MODULE_LOG_DEBUG("start to get dirs info");
 
   GpgCommandExecutor::ExecuteContexts exec_contexts;
 
@@ -194,12 +195,12 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
       {"--list-dirs"},
       [this](int exit_code, const std::string &p_out,
              const std::string &p_err) {
-        SPDLOG_DEBUG(
+        MODULE_LOG_DEBUG(
             "gpgconf configurations exit_code: {} process stdout size: {}",
             exit_code, p_out.size());
 
         if (exit_code != 0) {
-          SPDLOG_ERROR(
+          MODULE_LOG_ERROR(
               "gpgconf execute error, process stderr: {} process stdout: "
               "{}",
               p_err, p_out);
@@ -214,8 +215,8 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
         for (const auto &line : line_split_list) {
           std::vector<std::string> info_split_list;
           boost::split(info_split_list, line, boost::is_any_of(":"));
-          SPDLOG_DEBUG("gpgconf info line: {} info size: {}", line,
-                       info_split_list.size());
+          MODULE_LOG_DEBUG("gpgconf info line: {} info size: {}", line,
+                           info_split_list.size());
 
           if (info_split_list.size() != 2) continue;
 
@@ -242,10 +243,11 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
         }
       }});
 
-  SPDLOG_DEBUG("start to get components info");
+  MODULE_LOG_DEBUG("start to get components info");
 
   for (const auto &component : info_.ComponentsInfo) {
-    SPDLOG_DEBUG("gpgconf check options ready", "component", component.first);
+    MODULE_LOG_DEBUG("gpgconf check options ready", "component",
+                     component.first);
 
     if (component.first == "gpgme" || component.first == "gpgconf") continue;
 
@@ -254,13 +256,13 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
         {"--check-options", component.first},
         [this, component](int exit_code, const std::string &p_out,
                           const std::string &p_err) {
-          SPDLOG_DEBUG(
+          MODULE_LOG_DEBUG(
               "gpgconf {} options exit_code: {} process stdout "
               "size: {} ",
               component.first, exit_code, p_out.size());
 
           if (exit_code != 0) {
-            SPDLOG_ERROR(
+            MODULE_LOG_ERROR(
                 "gpgconf {} options execute error, process "
                 "stderr: {} , process stdout:",
                 component.first, p_err, p_out);
@@ -276,8 +278,8 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
             std::vector<std::string> info_split_list;
             boost::split(info_split_list, line, boost::is_any_of(":"));
 
-            SPDLOG_DEBUG("component {} options line: {} info size: {}",
-                         component.first, line, info_split_list.size());
+            MODULE_LOG_DEBUG("component {} options line: {} info size: {}",
+                             component.first, line, info_split_list.size());
 
             if (info_split_list.size() != 6) continue;
 
@@ -300,10 +302,11 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
         }});
   }
 
-  SPDLOG_DEBUG("start to get avaliable component options info");
+  MODULE_LOG_DEBUG("start to get avaliable component options info");
 
   for (const auto &component : info_.ComponentsInfo) {
-    SPDLOG_DEBUG("gpgconf list options ready", "component", component.first);
+    MODULE_LOG_DEBUG("gpgconf list options ready", "component",
+                     component.first);
 
     if (component.first == "gpgme" || component.first == "gpgconf") continue;
 
@@ -312,13 +315,13 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
         {"--list-options", component.first},
         [this, component](int exit_code, const std::string &p_out,
                           const std::string &p_err) {
-          SPDLOG_DEBUG(
+          MODULE_LOG_DEBUG(
               "gpgconf {} avaliable options exit_code: {} process stdout "
               "size: {} ",
               component.first, exit_code, p_out.size());
 
           if (exit_code != 0) {
-            SPDLOG_ERROR(
+            MODULE_LOG_ERROR(
                 "gpgconf {} avaliable options execute error, process stderr: "
                 "{} , process stdout:",
                 component.first, p_err, p_out);
@@ -334,7 +337,7 @@ int GnuPGInfoGatheringModule::Exec(EventRefrernce event) {
             std::vector<std::string> info_split_list;
             boost::split(info_split_list, line, boost::is_any_of(":"));
 
-            SPDLOG_DEBUG(
+            MODULE_LOG_DEBUG(
                 "component {} avaliable options line: {} info size: {}",
                 component.first, line, info_split_list.size());