Change error returns in subsession_register_event_callback and subsession_unregister_...

subsession_register_event will return SUBSESSION_ERROR_ALREADY_EXISTS
if event on the same subsession_uid was previously registered

subsession_unregister_event will return SUBSESSION_ERROR_NOT_AVAILABLE
if event on the subsession wasn't or previously unregistered

to change callback and callbacks data
unregister and register event again is required

To update the calllback and callback's data the call of subsession_unregister_event_callback
and subsession_register_event_callback with new data is required.

Change-Id: I2ad90f7ac04e9e5f8417f5842db920e91c94511c

diff --git a/libsessiond/src/lib.c b/libsessiond/src/lib.c
index 26ede1c..8fefa42 100644 (file)
--- a/libsessiond/src/lib.c
+++ b/libsessiond/src/lib.c
@@ -906,7 +906,6 @@ EXPORT_API int subsession_switch_user(int session_uid, int next_user, subsession
 
 typedef struct {
        signal_client_data_t *signal_client_data;
-       int is_registered;
        int error;
 } registered_signal_t;
 
@@ -925,34 +924,26 @@ registered_signal_t subsession_register_event_in_list(client_callbacks_data_t *c
                return registered_signal;
        }
 
-       signal_client_data_t *signal_client_data = NULL;
 
        GList *found_data = g_list_find_custom( callbacks_data->list , (gconstpointer) params, g_compare_session_uid_params );
 
-       if(found_data == NULL) {
-               registered_signal.is_registered = 0;
-               signal_client_data = make_new_signal_callback_client_data(cb, cb_data, params);
+       if(found_data != NULL) {
+               registered_signal.error = SUBSESSION_ERROR_ALREADY_EXISTS;
+               g_mutex_unlock(&callbacks_data->mutex);
+               return registered_signal;
+       }
 
-               if (signal_client_data == NULL) {
+       signal_client_data_t *signal_client_data = make_new_signal_callback_client_data(cb, cb_data, params);
+
+       if (signal_client_data == NULL) {
                        g_mutex_unlock(&callbacks_data->mutex);
                        registered_signal.error = SUBSESSION_ERROR_OUT_OF_MEMORY;
                        return registered_signal;
-               }
-               callbacks_data->list = g_list_append(callbacks_data->list, signal_client_data);
-       } else {
-               //This will change callback function and data.
-               registered_signal.is_registered = 1;
-               signal_client_data = (signal_client_data_t*)found_data->data;
-               signal_client_data->client_callback = cb;
-
-               signal_client_data_t *data_to_remove = signal_client_data->client_callback_data;
-               free(data_to_remove);
-
-               signal_client_data->client_callback_data = cb_data;
        }
+       callbacks_data->list = g_list_append(callbacks_data->list, signal_client_data);
+
        registered_signal.signal_client_data = signal_client_data;
        registered_signal.error = SUBSESSION_ERROR_NONE;
-
        g_mutex_unlock(&callbacks_data->mutex);
        return registered_signal;
 }
@@ -993,10 +984,6 @@ int register_event_callback(client_callbacks_data_t *callbacks_data, GDBusSignal
                return  registered_signal.error;
        }
 
-       if(registered_signal.is_registered) {
-               return SUBSESSION_ERROR_NONE;
-       }
-
        int method_call_result = subscribe_and_call_method(
                                dbus_data_ptr_(callbacks_data, params),
                                signal_callback,
@@ -1026,10 +1013,6 @@ int subscribe_only_callback(client_callbacks_data_t *callbacks_data, GDBusSignal
                return  registered_signal.error;
        }
 
-       if(registered_signal.is_registered) {
-               return SUBSESSION_ERROR_NONE;
-       }
-
        MAKE_SURE_CONNECTION_IS_NOT_NULL(
                client_data_cleansweep_mt(registered_signal.signal_client_data, callbacks_data);
        );
@@ -1073,10 +1056,13 @@ int unregister_event_callback(client_callbacks_data_t *callbacks_data, int sessi
        GVariant * gv_session_uid = g_variant_new("(i)", session_uid);
 
        signal_client_data_t *signal_data = take_from_callbacks_data_and_remove_from_list(callbacks_data, gv_session_uid, g_compare_session_uid_params);
-       if (signal_data != NULL) {
-               free(signal_data);
+
+       if(signal_data == NULL) {
+               return SUBSESSION_ERROR_NOT_AVAILABLE;
        }
 
+       free(signal_data);
+
        g_mutex_lock(&callbacks_data->mutex);
        if(g_list_length(callbacks_data->list) <= 1) {
                g_dbus_connection_signal_unsubscribe(session_connection_data.connection, callbacks_data->dbus_signal_subscribed);

diff --git a/libsessiond/target_test/CMakeLists.txt b/libsessiond/target_test/CMakeLists.txt
index 282df92..6bcd456 100644 (file)
--- a/libsessiond/target_test/CMakeLists.txt
+++ b/libsessiond/target_test/CMakeLists.txt
@@ -24,3 +24,4 @@ add_libsd_target_test(api_reg_unreg_add_user_wait  "")
 add_libsd_target_test(api_switch_user_completion "")
 add_libsd_target_test(api_get_user_list  "")
 add_libsd_target_test(api_event_wait_done_err "")
+add_libsd_target_test(api_register_event_err "")

diff --git a/libsessiond/target_test/test_api_register_event_err.cpp b/libsessiond/target_test/test_api_register_event_err.cpp
new file mode 100644 (file)
index 0000000..a4cf61e
--- /dev/null
+++ b/libsessiond/target_test/test_api_register_event_err.cpp
@@ -0,0 +1,55 @@
+#include<iostream>
+#include<string>
+#include<algorithm>
+
+#include <gtest/gtest.h>
+#include <gio/gio.h>
+#include <pthread.h>
+#include "sessiond.h"
+#include "test_hlp.hpp"
+
+using tud_ = test_user_data_cb_t;
+using ud_register_event_fail = struct ud_ctrl_t<tud_, tud_, tud_, tud_, tud_>;
+
+struct register_event_fail {
+    enum action {
+               register_bad_session_uid = 0,
+               register_seesion_ok,
+               register_again_error,
+               unregister_session_ok,
+               unregister_session_again_error,
+               action_items,
+       };
+};
+
+TEST(register_event, FailAtRegisterEvent) {
+
+       ud_register_event_fail ud = { .loop = g_main_loop_new(NULL, FALSE),
+               .t = ud_register_event_fail::the_tuple(),
+       };
+
+       std::apply([](auto &... args){( (init_user_s<tud_ &, subsession_5001>(args) ), ...); },  ud.t );
+
+       init_user_s<tud_ &, bad_subsession_5004>(std::get<register_event_fail::register_bad_session_uid>(ud.t));
+
+       std::array<res_t, register_event_fail::action_items> results {
+               res_t{ register_event_wrap<ud_register_event_fail, register_event_fail::register_bad_session_uid, true>(SUBSESSION_ADD_USER_WAIT, test_event_add_user_callback, ud),
+                       SUBSESSION_ERROR_INVALID_PARAMETER, "Check if bad session uid is returns error"},
+
+               res_t {register_event_wrap<ud_register_event_fail, register_event_fail::register_seesion_ok, true>(SUBSESSION_ADD_USER_WAIT, test_event_add_user_callback, ud),
+                       SUBSESSION_ERROR_NONE, "Check if registered session returns no error"},
+
+               res_t { register_event_wrap<ud_register_event_fail, register_event_fail::register_again_error, true>(SUBSESSION_ADD_USER_WAIT, test_event_add_user_callback, ud),
+                       SUBSESSION_ERROR_ALREADY_EXISTS, "Check if registered session returns already exists error"},
+
+               res_t { unregister_event_wrap <ud_register_event_fail, register_event_fail::unregister_session_ok, true>(SUBSESSION_ADD_USER_WAIT, test_event_add_user_callback, ud),
+                       SUBSESSION_ERROR_NONE, "Check if unregistered session returns no error"},
+
+               res_t { unregister_event_wrap <ud_register_event_fail, register_event_fail::unregister_session_again_error, true>(SUBSESSION_ADD_USER_WAIT, test_event_add_user_callback, ud),
+                       SUBSESSION_ERROR_NOT_AVAILABLE, "Check if unregistered the same session returns error"},
+       };
+
+       loop_run_for_test(callback_pending_ud<ud_register_event_fail>,(gpointer*)&ud, ud.loop);
+
+       summarize_results<register_event_fail::action_items>(results);
+}

diff --git a/libsessiond/target_test/test_hlp.hpp b/libsessiond/target_test/test_hlp.hpp
index b7c3dab..f67bceb 100644 (file)
--- a/libsessiond/target_test/test_hlp.hpp
+++ b/libsessiond/target_test/test_hlp.hpp
@@ -155,6 +155,52 @@ void test_reply_callback_default (void *cb_data) {
        g_atomic_int_inc(&user_data->callback_reference);
 }
 
+void test_event_add_user_callback(subsession_event_info info, void *cb_data) {
+
+       test_user_data_cb_t *user_data = (test_user_data_cb_t *)cb_data;
+
+       if (cb_data == NULL)
+               return;
+
+       user_data->session_uid = info.session_uid;
+       user_data->event = info.event;
+       user_data->user_id = info.add_user.user;
+
+       user_data->callback_result = subsession_event_wait_done(info);
+       g_atomic_int_inc(&user_data->callback_reference);
+}
+
+void test_event_remove_user_callback(subsession_event_info info, void *cb_data) {
+
+       test_user_data_cb_t *user_data = (test_user_data_cb_t *)cb_data;
+
+       if (cb_data == NULL)
+               return;
+
+       user_data->session_uid = info.session_uid;
+       user_data->event = info.event;
+       user_data->user_id = info.remove_user.user;
+
+       user_data->callback_result = subsession_event_wait_done(info);
+       g_atomic_int_inc(&user_data->callback_reference);
+}
+
+void test_event_switch_user_callback(subsession_event_info info, void *cb_data) {
+
+       test_user_data_cb_t *user_data = (test_user_data_cb_t *)cb_data;
+
+       if (cb_data == NULL)
+               return;
+
+       user_data->session_uid = info.session_uid;
+       user_data->event = info.event;
+       user_data->user_id = info.switch_user.next_user;
+       user_data->switch_id = info.switch_user.switch_id;
+
+       user_data->callback_result = subsession_event_wait_done(info);
+       g_atomic_int_inc(&user_data->callback_reference);
+}
+
 template<typename T>
 gboolean callback_pending(gpointer data) {
 
@@ -228,7 +274,26 @@ int switch_user_wrap(int user_id, subsession_reply_callback cb, T &ud) {
        return res;
 }
 
-//int ret = subsession_get_user_list(tested_user, &user_list_0, &user_count_0);
+template<typename T, int N, bool cb_pass = false>
+int register_event_wrap(subsession_event_type_e event_bits, subsession_event_callback cb, T &ud) {
+
+       std::get<N>(ud.t).event = event_bits;
+       int res = subsession_register_event_callback(std::get<N>(ud.t).session_uid, std::get<N>(ud.t).event,
+               cb, (void *)&std::get<N>(ud.t));
+       if ( res != SUBSESSION_ERROR_NONE || cb_pass == true ) { g_atomic_int_inc(&std::get<N>(ud.t).callback_reference); }
+       return res;
+}
+
+
+template<typename T, int N, bool cb_pass = false>
+int unregister_event_wrap(subsession_event_type_e event_bits, subsession_event_callback cb, T &ud) {
+
+       std::get<N>(ud.t).event = event_bits;
+       int res = subsession_unregister_event_callback(std::get<N>(ud.t).session_uid, std::get<N>(ud.t).event);
+
+       if ( res != SUBSESSION_ERROR_NONE  || cb_pass == true) { g_atomic_int_inc(&std::get<N>(ud.t).callback_reference); }
+       return res;
+}
 
 template<typename T, int N>
 int get_user_list_wrap(T &ud) {