Fix errors found while adding tests for CPackageUpdateMonitor

While adding new tests for CPackageUpdateMonitor,
unexpected errors were found and fixed the main cause
that the 'ret' variable was used for both application
filter result and the function invocation result.

Since all the errors found while adding new tests are fixed,
the existing package related code is now replaced with the
CPackageUpdateMonitor class implementation.

Change-Id: Ia3af5644ac41aaae26248456103d042a76031fe2

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3d2eee2..0bb5279 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -74,6 +74,7 @@ SET(SRCS
                src/application_manager_aul.cpp
                src/preference_manager_vconf.cpp
                src/client_manager.cpp
+               src/package_update_monitor.cpp
                src/service_main.cpp
                src/service_plugin.cpp
                src/service_ipc_dbus.cpp

diff --git a/inc/service_main.h b/inc/service_main.h
index fd3c521..cf947e6 100644 (file)
--- a/inc/service_main.h
+++ b/inc/service_main.h
@@ -34,6 +34,7 @@
 #include "service_ipc_dbus.h"
 #include "application_manager.h"
 #include "preference_manager.h"
+#include "package_update_monitor.h"
 
 typedef enum {
        CLIENT_LAUNCH_MODE_ACTIVATION,
@@ -63,6 +64,15 @@ typedef enum {
 #define WAKEUP_SETTINGS_KEY_PREPROCESSING_ASSISTANT_APPID "db/multi-assistant/preprocessing_assistant_appid"
 #define WAKEUP_SETTINGS_KEY_PRELAUNCH_MODE "db/multi-assistant/prelaunch_mode"
 
+class CServiceMain;
+
+class CPackageUpdateEventObserver : public IPackageUpdateEventObserver {
+public:
+       void OnUpdateStarted() override;
+       void OnUpdateFinished() override;
+       void OnRestartRequired() override;
+};
+
 class CServiceMain : public IServiceIpcObserver {
 public:
        CServiceMain(IApplicationManager& applicationManager, IPreferenceManager& preferenceManager) :
@@ -166,7 +176,9 @@ private:
        int mCurrentClientInfo{0};
        int mCurrentPreprocessingClientInfo{-1};
        std::string mWakeupClientAppId;
-       package_manager_h mPackageManagerHandle{NULL};
+
+       CPackageUpdateEventObserver mPackageUpdateEventObserver;
+       CPackageUpdateMonitor mPackageUpdateMonitor{mPackageUpdateEventObserver, mClientInfo};
 
        PREPROCESSING_STATE mCurrentPreprocessingState{PREPROCESSING_STATE_NONE};
        ma_service_state_e mCurrentServiceState{MA_SERVICE_STATE_INACTIVE};

diff --git a/src/package_update_monitor.cpp b/src/package_update_monitor.cpp
index b06f753..1a57bd8 100644 (file)
--- a/src/package_update_monitor.cpp
+++ b/src/package_update_monitor.cpp
@@ -20,7 +20,7 @@
 #include <pkgmgr-info.h>
 
 typedef struct {
-    int *return_value{nullptr};
+    int *filter_result{nullptr};
     CPackageUpdateMonitor *monitor{nullptr};
 } PackageListCallbackParams;
 
@@ -105,7 +105,7 @@ static int pkg_app_list_cb(const pkgmgrinfo_appinfo_h handle, void *user_data)
        PackageListCallbackParams *params = (PackageListCallbackParams*)user_data;
        if (!params) return 0;
 
-       int *result = params->return_value;
+       int *result = params->filter_result;
        if (result) {
                if (is_voice_assistant(handle, params->monitor)) {
                        *result = 1;
@@ -178,9 +178,10 @@ void CPackageUpdateMonitor::package_manager_event_cb(const char *type, const cha
                }
        }
 
+       int filter_result = 0;
        if (pkginfo_found) {
                PackageListCallbackParams params;
-               params.return_value = &ret;
+               params.filter_result = &filter_result;
                params.monitor =  monitor;
 
                if (user) {
@@ -203,11 +204,11 @@ void CPackageUpdateMonitor::package_manager_event_cb(const char *type, const cha
                        if (PACKAGE_MANAGER_EVENT_TYPE_UNINSTALL == event_type &&
                                (PACKAGE_MANAGER_EVENT_STATE_COMPLETED == event_state ||
                                PACKAGE_MANAGER_EVENT_STATE_FAILED == event_state)) {
-                               ret = 1;
+                               filter_result = 1;
                        }
                }
        }
-       if (1 == ret) {
+       if (1 == filter_result) {
                if (PACKAGE_MANAGER_EVENT_STATE_STARTED == event_state) {
                        LOGI("processing PACKAGE_MANAGER_EVENT_STATE_STARTED event : %d", event_type);
                        if (false == in_progress) {

diff --git a/src/service_main.cpp b/src/service_main.cpp
index 8f84b20..b37b422 100644 (file)
--- a/src/service_main.cpp
+++ b/src/service_main.cpp
@@ -35,6 +35,24 @@
 
 static CServiceMain* g_service_main = nullptr;
 
+void CPackageUpdateEventObserver::OnUpdateStarted()
+{
+       if (g_service_main) {
+               g_service_main->deinitialize_service_plugin();
+       } else {
+               MAS_LOGE("service_main is NULL");
+       }
+}
+
+void CPackageUpdateEventObserver::OnUpdateFinished()
+{
+}
+
+void CPackageUpdateEventObserver::OnRestartRequired()
+{
+       service_app_exit();
+}
+
 bool CServiceMain::check_preprocessing_assistant_exists()
 {
        bool ret = false;
@@ -1235,188 +1253,6 @@ bool CServiceMain::is_valid_wakeup_engine(const char* appid)
        return false;
 }
 
-bool CServiceMain::is_wakeup_engine(const pkgmgrinfo_appinfo_h handle)
-{
-       if (nullptr == g_service_main) return false;
-       bool is_wakeup_engine = false;
-
-       char *appid = NULL;
-
-       int ret = pkgmgrinfo_appinfo_get_appid(handle, &appid);
-       if (PMINFO_R_OK == ret && NULL != appid) {
-               for (int loop = 0; loop < MAX_MACLIENT_INFO_NUM;loop++) {
-                       ClientInfoItems *items = mClientInfo.getItems();
-                       if (items[loop].used) {
-                               LOGD("comparing appid : %s %s", items[loop].wakeup_engine, appid);
-                               if (0 == strncmp(items[loop].wakeup_engine, appid, MAX_APPID_LEN)) {
-                                       is_wakeup_engine = true;
-                               }
-                       }
-               }
-       } else {
-               LOGE("pkgmgrinfo_appinfo_get_appid failed! error code=%d", ret);
-       }
-
-       return is_wakeup_engine;
-}
-
-bool CServiceMain::is_voice_assistant(const pkgmgrinfo_appinfo_h handle)
-{
-       bool is_voice_assistant = false;
-       char* metadata_value = NULL;
-       const char* voice_assistant_metadata = "http://tizen.org/metadata/multi-assistant/name";
-       int ret = pkgmgrinfo_appinfo_get_metadata_value(handle, voice_assistant_metadata, &metadata_value);
-       if (PMINFO_R_OK == ret && NULL != metadata_value) {
-               is_voice_assistant = true;
-       } else {
-               LOGE("pkgmgrinfo_appinfo_get_metadata_value failed! error code=%d", ret);
-       }
-       return is_voice_assistant;
-}
-
-static int pkg_app_list_cb(const pkgmgrinfo_appinfo_h handle, void *user_data)
-{
-       if (!g_service_main) return 0;
-
-       int *result = (int*)user_data;
-       if (result) {
-               if (g_service_main->is_voice_assistant(handle)) {
-                       *result = 1;
-               } else if (g_service_main->is_wakeup_engine(handle)) {
-                       *result = 1;
-               }
-       }
-       return 0;
-}
-
-/*
-INFO: Package install/update/uninstall scenario
-Install and Uninstall are obviously simple.
-   Install: just INSTALL
-   Uninstall: just UNINSTALL
-Update package (change the source codes and Run As again), there are four scenarios:
-1. UPDATE
-   Source code change
-2. UNINSTALL -> INSTALL
-   This happens when Tizen IDE Property > Tizen SDK > Rapid Development Support > Check "Enable Project specific settings"
-   and change Application ID in tizen-manifest.xml file and Run As.
-3. UPDATE -> INSTALL
-   This happens when Tizen IDE Property > Tizen SDK > Rapid Development Support > Uncheck "Enable Project specific settings"
-   and change Application ID in tizen-manifest.xml file and Run As.
-   At UPDATE event, pkgid (package parameter) is invalid...
-4. UPDATE
-   Exceptionally, only UPDATE can be called when Application ID in tizen-manifest.xml file is changed.
-   At UPDATE event, pkgid (package parameter) is valid, and only appid is changed; the previous appid is invalid.
-*/
-static void _package_manager_event_cb(const char *type, const char *package, package_manager_event_type_e event_type, package_manager_event_state_e event_state, int progress, package_manager_error_e error, void *user_data)
-{
-       CServiceMain* service_main = static_cast<CServiceMain*>(user_data);
-
-       int ret = 0;
-       uid_t uid = getuid();
-       pkgmgrinfo_pkginfo_h handle = NULL;
-       static bool in_progress = false;
-       bool should_exit = false;
-       bool pkginfo_found = true;
-
-       if (!package || !type)
-               return;
-
-       if (PACKAGE_MANAGER_EVENT_TYPE_UPDATE != event_type &&
-               PACKAGE_MANAGER_EVENT_TYPE_INSTALL != event_type &&
-               PACKAGE_MANAGER_EVENT_TYPE_UNINSTALL != event_type)
-               return;
-
-       if (PACKAGE_MANAGER_EVENT_STATE_STARTED != event_state &&
-               PACKAGE_MANAGER_EVENT_STATE_COMPLETED != event_state &&
-               PACKAGE_MANAGER_EVENT_STATE_FAILED != event_state)
-               return;
-
-       bool user = false;
-       MAS_LOGD("type=%s package=%s event_type=%d event_state=%d progress=%d error=%d",
-               type, package, event_type, event_state, progress, error);
-       ret = pkgmgrinfo_pkginfo_get_pkginfo(package, &handle);
-       if (ret != PMINFO_R_OK || NULL == handle) {
-               MAS_LOGW("Failed to call pkgmgrinfo_pkginfo_get_pkginfo(\"%s\",~) returned %d", package, ret);
-               /* Try to get in user packages */
-               user = true;
-               ret = pkgmgrinfo_pkginfo_get_usr_pkginfo(package, uid, &handle);
-               if (ret != PMINFO_R_OK || NULL == handle) {
-                       MAS_LOGW("Failed to call pkgmgrinfo_pkginfo_get_pkginfo & get_usr_pkginfo(\"%s\",~) returned %d, uid : %d", package, ret, getuid());
-                       pkginfo_found = false;
-               }
-       }
-
-       if (pkginfo_found) {
-               if (user) {
-                       /* Try to get in user packages */
-                       ret = pkgmgrinfo_appinfo_get_usr_list(handle, PMINFO_ALL_APP, pkg_app_list_cb, (void *)&ret, uid);
-                       if (ret != PMINFO_R_OK) {
-                               MAS_LOGW("Failed to get usr pkg list from pkginfo, returned %d", ret);
-                       }
-               }  else {
-                       /* Try to get in global packages */
-                       ret = pkgmgrinfo_appinfo_get_list(handle, PMINFO_ALL_APP, pkg_app_list_cb, (void *)&ret);
-                       if (ret != PMINFO_R_OK) {
-                               MAS_LOGW("Failed to get pkg list from pkginfo, returned %d", ret);
-                       }
-               }
-       } else {
-               /* Even if we failed acquiring the pkginfo, proceed if we're uninstalling
-                       since at the time of uninstall completion, pkginfo would not exist */
-               if (in_progress) {
-                       if (PACKAGE_MANAGER_EVENT_TYPE_UNINSTALL == event_type &&
-                               (PACKAGE_MANAGER_EVENT_STATE_COMPLETED == event_state ||
-                               PACKAGE_MANAGER_EVENT_STATE_FAILED == event_state)) {
-                               ret = 1;
-                       }
-               }
-       }
-       if (1 == ret) {
-               if (PACKAGE_MANAGER_EVENT_STATE_STARTED == event_state) {
-                       MAS_LOGI("processing PACKAGE_MANAGER_EVENT_STATE_STARTED event : %d", event_type);
-                       if (false == in_progress) {
-                               in_progress = true;
-                               if (service_main) {
-                                       service_main->deinitialize_service_plugin();
-                               } else {
-                                       MAS_LOGE("service_main is NULL");
-                               }
-                       }
-               } else if (PACKAGE_MANAGER_EVENT_STATE_COMPLETED == event_state ||
-                       PACKAGE_MANAGER_EVENT_STATE_FAILED == event_state) {
-                       MAS_LOGI("processing PACKAGE_MANAGER_EVENT_STATE_COMPLETED/FAILED event : %d %d",
-                               event_state, event_type);
-                       if (false == in_progress) {
-                               if (service_main) {
-                                       service_main->deinitialize_service_plugin();
-                               } else {
-                                       MAS_LOGE("service_main is NULL");
-                               }
-                       }
-                       /*
-                       if (service_main) {
-                               service_main->initialize_service_plugin();
-                               service_main->process_activated_setting();
-                       } else {
-                               MAS_LOGE("service_main is NULL");
-                       }
-                       */
-                       should_exit = true;
-                       in_progress = false;
-               }
-       }
-
-       if (handle) pkgmgrinfo_pkginfo_destroy_pkginfo(handle);
-
-       if (should_exit) {
-               LOGI("Now restarting multi-assistant-service for reloading updated modules");
-               service_app_exit();
-       }
-
-       return;
-}
-
 bool CServiceMain::app_create(void *data)
 {
        // Todo: add your code here.
@@ -1453,19 +1289,7 @@ bool CServiceMain::app_create(void *data)
                launch_client_by_appid((*preprocessing_assistant).c_str(), CLIENT_LAUNCH_MODE_PRELAUNCH);
        }
 
-       if (!mPackageManagerHandle) {
-               int ret = package_manager_create(&mPackageManagerHandle);
-               if (ret == PACKAGE_MANAGER_ERROR_NONE) {
-                       ret = package_manager_set_event_cb(mPackageManagerHandle, _package_manager_event_cb, this);
-                       if (ret == PACKAGE_MANAGER_ERROR_NONE) {
-                               LOGD("package_manager_set_event_cb succeeded.");
-                       } else {
-                               LOGE("package_manager_set_event_cb failed(%d)", ret);
-                       }
-               } else {
-                       LOGE("package_manager_create failed(%d)", ret);
-               }
-       }
+       mPackageUpdateMonitor.initialize();
 
        MAS_LOGI("[END] Service app create");
        return true;
@@ -1474,11 +1298,8 @@ bool CServiceMain::app_create(void *data)
 void CServiceMain::app_terminate(void *data)
 {
        MAS_LOGI("[ENTER]");
-       if (mPackageManagerHandle) {
-               package_manager_unset_event_cb(mPackageManagerHandle);
-               package_manager_destroy(mPackageManagerHandle);
-               mPackageManagerHandle = NULL;
-       }
+
+       mPackageUpdateMonitor.deinitialize();
 
        deinitialize_service_plugin();
 

diff --git a/tests/utc/package-update-monitor/test_package_update_monitor.cpp b/tests/utc/package-update-monitor/test_package_update_monitor.cpp
index a4fc299..9bce7cd 100644 (file)
--- a/tests/utc/package-update-monitor/test_package_update_monitor.cpp
+++ b/tests/utc/package-update-monitor/test_package_update_monitor.cpp
@@ -42,6 +42,16 @@ public:
                        PACKAGE_MANAGER_EVENT_STATE_COMPLETED,
                        100, PACKAGE_MANAGER_ERROR_NONE, this);
        }
+       void GenerateFakeUpdatePackageEvent(std::string package) {
+               package_manager_event_cb("", package.c_str(),
+                       PACKAGE_MANAGER_EVENT_TYPE_UPDATE,
+                       PACKAGE_MANAGER_EVENT_STATE_STARTED,
+                       0, PACKAGE_MANAGER_ERROR_NONE, this);
+               package_manager_event_cb("", package.c_str(),
+                       PACKAGE_MANAGER_EVENT_TYPE_UPDATE,
+                       PACKAGE_MANAGER_EVENT_STATE_COMPLETED,
+                       100, PACKAGE_MANAGER_ERROR_NONE, this);
+       }
 };
 
 class CPackageUpdateEventObserver : public IPackageUpdateEventObserver {
@@ -86,21 +96,96 @@ int main(int argc, char** argv) {
        return ret;
 }
 
-TEST_F(DefaultFixture, NoUpdateEventsForNonExistingApp) {
+TEST_F(DefaultFixture, NoUpdateEventsForNonExistingAppInstall) {
        observer.Reset();
 
        monitor.GenerateFakeInstallPackageEvent("org.tizen.non-existing-app");
 
        ASSERT_TRUE(observer.events.empty());
 }
-TEST_F(DefaultFixture, UpdateStartsAndFinishesForVoiceAssistant) {
+
+TEST_F(DefaultFixture, NoUpdateEventsForNonMonitoringAppInstall) {
+       observer.Reset();
+
+       monitor.GenerateFakeInstallPackageEvent("org.tizen.multi-assistant-service");
+
+       ASSERT_TRUE(observer.events.empty());
+}
+
+TEST_F(DefaultFixture, UpdateStartsAndRestartsForVoiceAssistantInstall) {
        observer.Reset();
 
        monitor.GenerateFakeInstallPackageEvent("org.tizen.sample-assistant");
 
        std::vector<ObserverEvent> comparand{
                ObserverEvent::UpdateStarted,
-               ObserverEvent::UpdateFinished
+               ObserverEvent::RestartRequired
+       };
+       bool equality = (observer.events == comparand);
+
+       ASSERT_TRUE(equality);
+}
+
+TEST_F(DefaultFixture, UpdateStartsAndRestartsForWakeupEngineInstall) {
+       observer.Reset();
+       std::string wakeup_engine_appid{"wakeup-engine-default"};
+
+       clientInfo.getItems()[0].used = true;
+       strcpy(clientInfo.getItems()[0].wakeup_engine, wakeup_engine_appid.c_str());
+
+       monitor.GenerateFakeInstallPackageEvent(wakeup_engine_appid);
+
+       std::vector<ObserverEvent> comparand{
+               ObserverEvent::UpdateStarted,
+               ObserverEvent::RestartRequired
+       };
+       bool equality = (observer.events == comparand);
+
+       ASSERT_TRUE(equality);
+}
+
+TEST_F(DefaultFixture, NoUpdateEventsForNonExistingAppUpdate) {
+       observer.Reset();
+
+       monitor.GenerateFakeUpdatePackageEvent("org.tizen.non-existing-app");
+
+       ASSERT_TRUE(observer.events.empty());
+}
+
+TEST_F(DefaultFixture, NoUpdateEventsForNonMonitoringAppUpdate) {
+       observer.Reset();
+
+       monitor.GenerateFakeUpdatePackageEvent("org.tizen.multi-assistant-service");
+
+       ASSERT_TRUE(observer.events.empty());
+}
+
+TEST_F(DefaultFixture, UpdateStartsAndRestartsForVoiceAssistantUpdate) {
+       observer.Reset();
+
+       monitor.GenerateFakeUpdatePackageEvent("org.tizen.sample-assistant");
+
+       std::vector<ObserverEvent> comparand{
+               ObserverEvent::UpdateStarted,
+               ObserverEvent::RestartRequired
+       };
+       bool equality = (observer.events == comparand);
+
+       ASSERT_TRUE(equality);
+}
+
+TEST_F(DefaultFixture, UpdateStartsAndRestartsForWakeupEngineUpdate) {
+       observer.Reset();
+       std::string wakeup_engine_appid{"wakeup-engine-default"};
+
+       clientInfo.getItems()[0].used = true;
+       strcpy(clientInfo.getItems()[0].wakeup_engine, wakeup_engine_appid.c_str());
+
+       monitor.GenerateFakeUpdatePackageEvent(wakeup_engine_appid);
+
+       std::vector<ObserverEvent> comparand{
+               ObserverEvent::UpdateStarted,
+               ObserverEvent::RestartRequired
        };
        bool equality = (observer.events == comparand);
 

diff --git a/tests/utc/service-main/CMakeLists.txt b/tests/utc/service-main/CMakeLists.txt
index 69a4d83..1bf4890 100644 (file)
--- a/tests/utc/service-main/CMakeLists.txt
+++ b/tests/utc/service-main/CMakeLists.txt
@@ -15,6 +15,7 @@ ADD_DEFINITIONS("-DFULLVER=\"${FULLVER}\"")
 
 SET(TEST_SOURCES
        test_service_main.cpp
+       ${CMAKE_SOURCE_DIR}/src/package_update_monitor.cpp
        ${CMAKE_SOURCE_DIR}/src/service_main.cpp
        ${CMAKE_SOURCE_DIR}/src/service_ipc_dbus.cpp
        ${CMAKE_SOURCE_DIR}/src/service_ipc_dbus_dispatcher.cpp