Fix API behavior to match the specification

- Issue:
Some APIs did not work as specified on API's documentation.

- Solution:
This patch adds logic for checking state and parameter and also fix some
function to work as specifedd on API's documentation. Through this
patch, the APIs will properly and expectedly work.

Change-Id: I8a4c0a36806ebfa562d18da13e3673bb776dd4da
Signed-off-by: Suyeon Hwang <stom.hwang@samsung.com>

diff --git a/client/vc.c b/client/vc.c
index 60b67b1..fab3a6a 100644 (file)
--- a/client/vc.c
+++ b/client/vc.c
@@ -41,13 +41,12 @@
 #include "voice_control_command_expand.h"
 
 
-
-static Ecore_Timer* g_connect_timer = NULL;
-
 static Ecore_Event_Handler* g_focus_in_handler = NULL;
 static Ecore_Event_Handler* g_focus_out_handler = NULL;
 
 static Ecore_Thread* g_tts_thread = NULL;
+static Ecore_Thread* g_prepare_thread = NULL;
+static pthread_mutex_t g_prepare_thread_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static int g_pid;
 
@@ -446,11 +445,11 @@ int vc_deinitialize(void)
                __vc_internal_unprepare();
                /* no break. need to next step*/
        case VC_STATE_INITIALIZED:
-               if (NULL != g_connect_timer) {
-                       SLOG(LOG_DEBUG, TAG_VCC, "Connect Timer is deleted"); //LCOV_EXCL_LINE
-                       ecore_timer_del(g_connect_timer);
-                       g_connect_timer = NULL;
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (NULL != g_prepare_thread) {
+                       ecore_thread_cancel(g_prepare_thread);
                }
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
 
                vc_config_mgr_unset_lang_cb(g_pid);
                vc_config_mgr_finalize(g_pid);
@@ -495,8 +494,6 @@ static Eina_Bool __vc_connect_daemon(void *data)
        int mgr_pid = -1;
        int service_state = 0;
 
-       g_connect_timer = NULL;
-
        /* check handle */
        if (true == vc_client_is_valid()) {
                SLOG(LOG_DEBUG, TAG_VCC, "[DEBUG] The current client is valid");
@@ -617,12 +614,21 @@ static void __start_prepare_thread(void *data, Ecore_Thread *thread)
 
        /* Send hello */
        while (0 != ret) {
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (EINA_TRUE == ecore_thread_check(thread)) {
+                       SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
+                       return;
+               }
+
                if (retry_count == VC_TIDL_RETRY_COUNT) { // 200 ms * 20 = 4 sec
                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
                        return;
                }
 
                ret = vc_tidl_request_hello();
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
                if (ret == 0) {
                        SLOG(LOG_DEBUG, TAG_VCC, "Success to request hello. retry count(%d)", retry_count);
                        break;
@@ -635,23 +641,38 @@ static void __start_prepare_thread(void *data, Ecore_Thread *thread)
        ret = -1;
        retry_count = 0;
        while (0 != ret) {
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (EINA_TRUE == ecore_thread_check(thread)) {
+                       SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
+                       return;
+               }
+
                if (retry_count == 10) {
                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to connect daemon !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
                        return;
                }
+
                ret = __vc_connect_daemon(NULL);
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
                if (ret == 0)
                        break;
                else
                        retry_count++;
        }
-
-       return;
 }
 
 static void __end_prepare_thread(void *data, Ecore_Thread *thread)
 {
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ End prepare thread");
+       g_prepare_thread = NULL;
+}
+
+static void __cancel_prepare_thread(void *data, Ecore_Thread *thread)
+{
+       SLOG(LOG_DEBUG, TAG_VCC, "@@@ End prepare thread");
+       g_prepare_thread = NULL;
 }
 
 int vc_prepare(void)
@@ -672,8 +693,7 @@ int vc_prepare(void)
        /* check state */
        RETVM_IF(state != VC_STATE_INITIALIZED, VC_ERROR_INVALID_STATE, TAG_VCC, "[ERROR] Invalid State: Current state(%d) is not 'Initialized'", state);
 
-       ecore_thread_run(__start_prepare_thread, __end_prepare_thread, NULL, NULL);
-
+       g_prepare_thread = ecore_thread_run(__start_prepare_thread, __end_prepare_thread, __cancel_prepare_thread, NULL);
 
        return VC_ERROR_NONE;
 }
@@ -818,24 +838,21 @@ int vc_get_state(vc_state_e* state)
 
        vc_state_e temp;
        if (0 != vc_client_get_client_state(&temp)) {
-               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] handle is not valid");
-               return VC_ERROR_INVALID_STATE;
+               SLOG(LOG_WARN, TAG_VCC, "[ERROR] handle is not valid");
+               temp = VC_STATE_NONE;
        }
 
-
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ [Client] Get State");
-
-       *state = temp;
-
-       switch (*state) {
-       //LCOV_EXCL_START
+       switch (temp) {
        case VC_STATE_NONE:             SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'None'");            break;
        case VC_STATE_INITIALIZED:      SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'Created'");         break;
        case VC_STATE_READY:            SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'Ready'");           break;
-       default:                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid state");
-       //LCOV_EXCL_STOP
+       default:
+               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid state. Return state as 'None'.");
+               temp = VC_STATE_NONE;
        }
 
+       *state = temp;
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ [Client] Get State DONE");
 
        return VC_ERROR_NONE;
@@ -1195,6 +1212,12 @@ int vc_unset_command_list(int type)
        if (VC_ERROR_NONE != ret)
                return ret;
 
+       /* check type */
+       if ((VC_COMMAND_TYPE_FOREGROUND != type) && (VC_COMMAND_TYPE_BACKGROUND != type)) {
+               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid command type: input type is %d", type);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_state_e state;
        if (0 != vc_client_get_client_state(&state)) {
                SLOG(LOG_ERROR, TAG_VCC, "[ERROR] A handle is not available");
@@ -1254,6 +1277,9 @@ int vc_set_command_list_from_file(const char* file_path, int type)
        if (VC_ERROR_NONE != ret)
                return ret;
 
+       RETVM_IF(NULL == file_path, VC_ERROR_INVALID_PARAMETER, TAG_VCM, "[ERROR] Invalid parameter");
+       SLOG(LOG_INFO, TAG_VCC, "@@@ File path: %s", file_path);
+
        vc_state_e state;
        if (0 != vc_client_get_client_state(&state)) {
                SLOG(LOG_ERROR, TAG_VCC, "[ERROR] A handle is not available");

diff --git a/common/vc_cmd_db.c b/common/vc_cmd_db.c
index 154a4fa..2ddbb47 100644 (file)
--- a/common/vc_cmd_db.c
+++ b/common/vc_cmd_db.c
@@ -2490,6 +2490,18 @@ int vc_db_delete_commands(int pid, vc_cmd_type_e type, char* appid)
        return VC_DB_ERROR_NONE;
 }
 
+int vc_db_delete_commands_without_reset_transaction(int pid, vc_cmd_type_e type, char* appid)
+{
+       int ret = 0;
+       ret = __vc_db_delete_commands(g_db_handle, pid, type, appid);
+       if (ret != VC_DB_ERROR_NONE) {
+               __vc_db_rollback_transaction(g_db_handle);
+               return ret;
+       }
+
+       return VC_DB_ERROR_NONE;
+}
+
 static void __free_command_list_element(gpointer data)
 {
        vc_cmd_h temp_cmd = (vc_cmd_h)data;

diff --git a/common/vc_cmd_db.h b/common/vc_cmd_db.h
index 87376e5..040f2af 100644 (file)
--- a/common/vc_cmd_db.h
+++ b/common/vc_cmd_db.h
@@ -86,6 +86,8 @@ int vc_db_select_by_pid(int pid);
 
 int vc_db_delete_commands(int pid, vc_cmd_type_e type, char* appid);
 
+int vc_db_delete_commands_without_reset_transaction(int pid, vc_cmd_type_e type, char* appid);
+
 int vc_db_backup_command();
 
 int vc_db_restore_command();

diff --git a/common/vc_command.c b/common/vc_command.c
index 77e8016..1c63b22 100644 (file)
--- a/common/vc_command.c
+++ b/common/vc_command.c
@@ -356,16 +356,13 @@ int vc_cmd_list_remove(vc_cmd_list_h vc_cmd_list, vc_cmd_h vc_command)
        return VC_ERROR_NONE;
 }
 
-static void __vc_cmd_list_remove_all_foreach(gpointer data, gpointer user_data)
+static void __vc_cmd_list_remove_all_foreach(gpointer data)
 {
-       vc_cmd_s *temp = NULL;
-       temp = data;
+       vc_cmd_s *temp = (vc_cmd_s *)data;
        if (temp) {
-               SLOG(LOG_DEBUG, TAG_VCCMD, "Free command(%p)", temp);
+               SECURE_SLOG(LOG_DEBUG, TAG_VCCMD, "Free command(%p)", temp);
                vc_cmd_destroy((vc_cmd_h)temp);
-               temp = NULL;
        }
-       return;
 }
 
 int vc_cmd_list_remove_all(vc_cmd_list_h vc_cmd_list, bool release_command)
@@ -384,21 +381,25 @@ int vc_cmd_list_remove_all(vc_cmd_list_h vc_cmd_list, bool release_command)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
-       vc_cmd_list_s* list = NULL;
-       list = (vc_cmd_list_s*)vc_cmd_list;
+       vc_cmd_list_s* cmd_list = (vc_cmd_list_s*)vc_cmd_list;
 
        SLOG(LOG_DEBUG, TAG_VCCMD, "[List] list (%p), release command (%s)"
-                , list, release_command ? "true" : "false");
+                , cmd_list, release_command ? "true" : "false");
+
+       if (NULL == cmd_list->list) {
+               SLOG(LOG_DEBUG, TAG_VCCMD, "@@@ List is already empty.");
+               return VC_ERROR_NONE;
+       }
 
        if (true == release_command) {
-               if (list->list) {
-                       g_slist_foreach(list->list, __vc_cmd_list_remove_all_foreach, NULL);
-                       g_slist_free(list->list);
-                       list->list = NULL;
-               }
-               list->index = -1;
+               g_slist_free_full(cmd_list->list, __vc_cmd_list_remove_all_foreach);
+       } else {
+               g_slist_free(cmd_list->list);
        }
 
+       cmd_list->list = NULL;
+       cmd_list->index = -1;
+
        SLOG(LOG_DEBUG, TAG_VCCMD, "@@@");
 
        return VC_ERROR_NONE;
@@ -413,7 +414,7 @@ int vc_cmd_list_foreach_commands(vc_cmd_list_h vc_cmd_list, vc_cmd_list_cb callb
                return VC_ERROR_PERMISSION_DENIED;
        }
 
-       if (NULL == vc_cmd_list) {
+       if (NULL == vc_cmd_list || NULL == callback) {
                SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Input parameter is NULL");
                return VC_ERROR_INVALID_PARAMETER;
        }
@@ -452,7 +453,7 @@ int vc_cmd_list_filter_by_type(vc_cmd_list_h original, int type, vc_cmd_list_h*
                return VC_ERROR_PERMISSION_DENIED;
        }
 
-       if (NULL == original) {
+       if (NULL == original || NULL == filtered) {
                SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Input parameter is NULL");
                return VC_ERROR_INVALID_PARAMETER;
        }
@@ -598,6 +599,10 @@ int vc_cmd_list_next(vc_cmd_list_h vc_cmd_list)
        list = (vc_cmd_list_s*)vc_cmd_list;
 
        int count = g_slist_length(list->list);
+       if (0 == count) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] List is empty");
+               return VC_ERROR_EMPTY;
+       }
 
        if (list->index < count - 1) {
                list->index = list->index + 1;
@@ -627,6 +632,11 @@ int vc_cmd_list_prev(vc_cmd_list_h vc_cmd_list)
        vc_cmd_list_s* list = NULL;
        list = (vc_cmd_list_s*)vc_cmd_list;
 
+       if (0 == g_slist_length(list->list)) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] List is empty");
+               return VC_ERROR_EMPTY;
+       }
+
        if (list->index > 0) {
                list->index = list->index - 1;
                SLOG(LOG_DEBUG, TAG_VCCMD, "[DEBUG] List index : %d", list->index);
@@ -1107,6 +1117,11 @@ int vc_cmd_set_type(vc_cmd_h vc_command, int type)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (VC_COMMAND_TYPE_NONE >= type || VC_COMMAND_TYPE_EXCLUSIVE < type) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid type(%d)", type);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 
@@ -1152,6 +1167,11 @@ int vc_cmd_set_format(vc_cmd_h vc_command, int format)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (VC_COMMAND_FORMAT_FIXED > format || VC_COMMAND_FORMAT_NONFIXED_AND_FIXED < format) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid format(%d)", format);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 
@@ -1197,6 +1217,11 @@ int vc_cmd_set_pid(vc_cmd_h vc_command, int pid)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (0 > pid) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid pid (%d)", pid);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 

diff --git a/common/vc_json_parser.c b/common/vc_json_parser.c
index 63e838b..3fc4eea 100644 (file)
--- a/common/vc_json_parser.c
+++ b/common/vc_json_parser.c
@@ -282,7 +282,7 @@ static int __vc_json_set_commands(JsonObject *root_obj, int type, char* invocati
 
                        if (0 != strncmp(cmd->appid, prev_appid, strlen(cmd->appid))) {
                                //delete background commands with appid and type
-                               ret = vc_db_delete_commands(cmd->pid, type, cmd->appid);
+                               ret = vc_db_delete_commands_without_reset_transaction(cmd->pid, type, cmd->appid);
                                if (VC_ERROR_NONE != ret) {
                                        SLOG(LOG_ERROR, vc_json_tag(), "[ERROR] Fail to insert command into db, ret(%d) pid(%d) type(%d), appid(%s)", ret, cmd->pid, type, cmd->appid);
                                        free(temp_type);