diff --git a/src/include/conversions.h b/src/include/conversions.h index 91504bf629..2af657e6de 100644 --- a/src/include/conversions.h +++ b/src/include/conversions.h @@ -62,10 +62,6 @@ as_status as_udf_file_to_pyobject(as_error *err, as_udf_file *entry, as_status as_udf_files_to_pyobject(as_error *err, as_udf_files *files, PyObject **py_files); -as_status str_array_of_roles_to_py_list(as_error *err, int num_elements, - char str_array_ptr[][AS_ROLE_SIZE], - PyObject *py_list); - as_status char_double_ptr_to_py_list(as_error *err, int num_elements, int element_size, char **str_array_ptr, PyObject *py_list); diff --git a/src/main/conversions.c b/src/main/conversions.c index 6576252d5c..4cb3701beb 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -144,28 +144,43 @@ as_status char_double_ptr_to_py_list(as_error *err, int num_elements, return err->code; } -as_status str_array_of_roles_to_py_list(as_error *err, int num_elements, - char str_array_ptr[][AS_ROLE_SIZE], - PyObject *py_list) +PyObject *convert_array_of_role_strs_to_py_list(as_error *err, int num_elements, + char str_array[][AS_ROLE_SIZE]) { as_error_reset(err); + PyObject *py_list = PyList_New(0); + if (!py_list) { + as_error_update("Failed to create python list"); + goto error; + } + char *str; for (int i = 0; i < num_elements; i++) { - str = str_array_ptr[i]; + str = str_array[i]; PyObject *py_str = Py_BuildValue("s", str); if (py_str == NULL) { as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to build string value from %s.", str); - break; + goto CLEANUP_ON_ERROR; } - PyList_Append(py_list, py_str); + int retval = PyList_Append(py_list, py_str); Py_DECREF(py_str); + if (retval == -1) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to append string %s to list.", str); + goto CLEANUP_ON_ERROR; + } } - return err->code; + return py_list; + +CLEANUP_ON_ERROR: + Py_DECREF(py_list); +error: + return NULL; } as_status as_user_info_array_to_pyobject(as_error *err, as_user **users, @@ -415,62 +430,102 @@ as_status as_partitions_status_to_pyobject( return err->code; } +PyObject *convert_nullable_array_of_uint32_to_py_optional_list(as_error *err, + uint32_t *array, + int array_size) +{ + if (array == NULL) { + Py_RETURN_NONE; + } + + PyObject *py_list = PyList_New(0); + if (!py_list) { + as_error_update("Failed to create python list"); + goto error; + } + + for (int i = 0; i < array_size; i++) { + PyObject *py_long_value = PyLong_FromUInt32(array[i]); + if (!py_long_value) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to get list item at index %" PRIu32, i); + goto CLEANUP_ON_ERROR; + } + + int retval = PyList_Append(py_list, py_long_value); + Py_DECREF(py_long_value); + if (retval == -1) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to append list item at index %" PRIu32, i); + goto CLEANUP_ON_ERROR; + } + } + + return py_list; + +CLEANUP_ON_ERROR: + Py_DECREF(py_list); +error: + return NULL; +} + as_status as_user_info_to_pyobject(as_error *err, as_user *user, PyObject **py_as_user) { as_error_reset(err); - PyObject *py_info = PyDict_New(); - PyObject *py_roles = PyList_New(0); + PyObject *py_user_dict = PyDict_New(); - str_array_of_roles_to_py_list(err, user->roles_size, user->roles, py_roles); - if (err->code != AEROSPIKE_OK) { - Py_DECREF(py_roles); - Py_DECREF(py_info); - goto END; + PyObject *py_roles = convert_array_of_role_strs_to_py_list( + err, user->roles_size, user->roles); + if (!py_roles) { + goto CLEANUP_ON_ERROR; } - - if (PyDict_SetItemString( - py_info, "read_info", - Py_BuildValue("i", (user->read_info ? *(user->read_info) : 0))) == - -1) { + int retval = PyDict_SetItemString(py_user_dict, "roles", py_roles); + Py_DECREF(py_roles); + if (retval == -1) { as_error_update(err, AEROSPIKE_ERR_CLIENT, - "Failed to set %s in py_info.", "read_info"); - Py_DECREF(py_roles); - Py_DECREF(py_info); - goto END; + "Failed to set %s in user dictionary.", "roles"); + goto CLEANUP_ON_ERROR; } - if (PyDict_SetItemString( - py_info, "write_info", - Py_BuildValue("i", (user->write_info ? *(user->write_info) : 0))) == - -1) { - as_error_update(err, AEROSPIKE_ERR_CLIENT, - "Failed to set %s in py_info.", "write_info"); - Py_DECREF(py_roles); - Py_DECREF(py_info); - goto END; + + uint32_t *arrays[] = {user->read_info, user->write_info}; + const char *array_names[] = {"read_info", "write_info"}; + int array_sizes[] = {user->read_info_size, user->write_info_size}; + + for (unsigned long i = 0; i < sizeof(arrays) / sizeof(arrays[0]); i++) { + PyObject *py_optional_list_of_ints = + convert_nullable_array_of_uint32_to_py_optional_list( + err, arrays[i], array_sizes[i]); + if (!py_optional_list_of_ints) { + goto CLEANUP_ON_ERROR; + } + + int retval = PyDict_SetItemString(py_user_dict, array_names[i], + py_optional_list_of_ints); + Py_DECREF(py_optional_list_of_ints); + if (retval == -1) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Failed to set %s in user dictionary.", + array_names[i]); + goto CLEANUP_ON_ERROR; + } } - if (PyDict_SetItemString(py_info, "conns_in_use", + + if (PyDict_SetItemString(py_user_dict, "conns_in_use", Py_BuildValue("i", user->conns_in_use)) == -1) { as_error_update(err, AEROSPIKE_ERR_CLIENT, - "Failed to set %s in py_info.", "conns_in_use"); - Py_DECREF(py_roles); - Py_DECREF(py_info); - goto END; - } - if (PyDict_SetItemString(py_info, "roles", py_roles) == -1) { - as_error_update(err, AEROSPIKE_ERR_CLIENT, - "Failed to set %s in py_info.", "roles"); - Py_DECREF(py_roles); - Py_DECREF(py_info); - goto END; + "Failed to set %s in py_user_dict.", "conns_in_use"); + goto CLEANUP_ON_ERROR; } - Py_DECREF(py_roles); + *py_as_user = py_user_dict; - *py_as_user = py_info; +CLEANUP_ON_ERROR: + if (err->code != AEROSPIKE_OK) { + Py_DECREF(py_user_dict); + } -END: return err->code; } diff --git a/test/new_tests/conftest.py b/test/new_tests/conftest.py index 6d3fe87523..876336330d 100644 --- a/test/new_tests/conftest.py +++ b/test/new_tests/conftest.py @@ -322,3 +322,27 @@ def expect_earlier_than_server_version_to_fail(as_connection, request): else: # InvalidRequest, BinIncompatibleTypes are exceptions that have been raised request.cls.expected_context_for_pos_tests = pytest.raises(e.ServerError) + +def check_user_dictionary(user: dict): + assert set(user["roles"]) == set([ + "read", + "read-write", + "sys-admin" + ]) + + + # The user has not read or written anything, so all r/w stats should be 0 + # NOTE: we don't test the scenario where read_info / write_info is not 0 + # because it takes time and a lot of transactions for the server to actually record non-zero values + dict_keys = [ + "read_info", + "write_info" + ] + for key in dict_keys: + assert type(user[key]) == list + assert len(user[key]) == 4 + for element in user[key]: + assert element == 0 + + # We assume no clients were logged in as this user + assert user.get("conns_in_use") == 0 diff --git a/test/new_tests/test_admin_query_user_info.py b/test/new_tests/test_admin_query_user_info.py index 0138ee4dd7..282a91e65c 100644 --- a/test/new_tests/test_admin_query_user_info.py +++ b/test/new_tests/test_admin_query_user_info.py @@ -4,6 +4,7 @@ import time from .test_base_class import TestBaseClass from aerospike import exception as e +from .conftest import check_user_dictionary import aerospike @@ -59,18 +60,8 @@ def test_query_user_info_with_proper_parameters(self): time.sleep(2) user_details = self.client.admin_query_user_info(self.user) - assert user_details.get("roles") == [ - "read", - "read-write", - "sys-admin" - ] - # The user has not read or written anything, so all r/w stats should be 0 - # NOTE: we don't test the scenario where read_info / write_info is not 0 - # because it takes time and a lot of transactions for the server to actually record non-zero values - assert user_details.get("read_info") == 0 - assert user_details.get("write_info") == 0 - # No clients were logged in as this user - assert user_details.get("conns_in_use") == 0 + + check_user_dictionary(user_details) def test_query_user_info_with_invalid_timeout_policy_value(self): policy = {"timeout": 0.1} diff --git a/test/new_tests/test_admin_query_users_info.py b/test/new_tests/test_admin_query_users_info.py index 7cb61d97e5..3dd3489564 100644 --- a/test/new_tests/test_admin_query_users_info.py +++ b/test/new_tests/test_admin_query_users_info.py @@ -4,6 +4,7 @@ import time from .test_base_class import TestBaseClass from aerospike import exception as e +from .conftest import check_user_dictionary import aerospike @@ -56,10 +57,7 @@ def test_query_users_info_with_proper_parameters(self): # Usage test; doesn't actually test if the server records user data user_details = user_details.get("example-test") - assert user_details.get("roles") == ["read", "read-write", "sys-admin"] - assert user_details.get("read_info") == 0 - assert user_details.get("write_info") == 0 - assert user_details.get("conns_in_use") == 0 + check_user_dictionary(user_details) def test_query_users_info_with_invalid_timeout_policy_value(self):