[CLIENT-4845] Fix user dictionary returning only the first entry of read_info and write_info array of integers

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);

src/main/conversions.c

@@ -144,30 +144,6 @@ 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)
-{
-    as_error_reset(err);
-
-    char *str;
-
-    for (int i = 0; i < num_elements; i++) {
-        str = str_array_ptr[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;
-        }
-
-        PyList_Append(py_list, py_str);
-        Py_DECREF(py_str);
-    }
-
-    return err->code;
-}
-
 as_status as_user_info_array_to_pyobject(as_error *err, as_user **users,
                                          PyObject **py_as_users, int users_size)
 {
@@ -415,62 +391,132 @@ as_status as_partitions_status_to_pyobject(
     return err->code;
 }
 
+// format_specifier: type casts each array element and converts it to the right Python type
+// This method makes certain assumptions if format_specifier is for converting a string:
+// 1. The array is a 2 dimensional array with the strings allocated in one long buffer.
+// 2. Each string is AS_ROLE_SIZE chars long.
+// TODO - Just refactor later when this helper function needs to handle more cases.
+static inline PyObject *convert_nullable_array_to_py_optional_list(
+    as_error *err, void *array, int array_size, char format_specifier)
+{
+    if (array == NULL) {
+        Py_RETURN_NONE;
+    }
+
+    PyObject *py_list = PyList_New(0);
+    if (!py_list) {
+        as_error_update(err, AEROSPIKE_ERR_CLIENT,
+                        "Failed to create python list");
+        goto error;
+    }
+
+    for (int i = 0; i < array_size; i++) {
+        PyObject *py_element = NULL;
+        char format_str[2];
+        sprintf(format_str, "%c", format_specifier);
+        switch (format_specifier) {
+        case 'k': {
+            uint32_t element = ((uint32_t *)array)[i];
+            py_element = Py_BuildValue(format_str, (unsigned long)element);
+            break;
+        }
+        case 's': {
+            const char *element = (const char *)array + i * AS_ROLE_SIZE;
+            py_element = Py_BuildValue(format_str, element);
+            break;
+        }
+        }
+
+        if (!py_element) {
+            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_element);
+        Py_DECREF(py_element);
+        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;
+}
+
+#define USER_DICTIONARY_FAILED_TO_SET "Failed to set %s in user dictionary"
+
 as_status as_user_info_to_pyobject(as_error *err, as_user *user,
-                                   PyObject **py_as_user)
+                                   PyObject **py_user_dict_ref)
 {
     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_list_of_roles = convert_nullable_array_to_py_optional_list(
+        err, user->roles, user->roles_size, 's');
+    if (!py_list_of_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_list_of_roles);
+    Py_DECREF(py_list_of_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;
+                        USER_DICTIONARY_FAILED_TO_SET, "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_to_py_optional_list(err, arrays[i],
+                                                       array_sizes[i], 'k');
+        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,
+                            USER_DICTIONARY_FAILED_TO_SET, array_names[i]);
+            goto CLEANUP_ON_ERROR;
+        }
     }
-    if (PyDict_SetItemString(py_info, "conns_in_use",
-                             Py_BuildValue("i", user->conns_in_use)) == -1) {
+
+    PyObject *py_conns_in_use = Py_BuildValue("i", user->conns_in_use);
+    if (!py_conns_in_use) {
         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;
+                        "Failed to convert conns_in_use in user dictionary.");
+        goto CLEANUP_ON_ERROR;
     }
-    if (PyDict_SetItemString(py_info, "roles", py_roles) == -1) {
+
+    retval =
+        PyDict_SetItemString(py_user_dict, "conns_in_use", py_conns_in_use);
+    Py_DECREF(py_conns_in_use);
+    if (retval == -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;
+                        USER_DICTIONARY_FAILED_TO_SET, "conns_in_use");
+        goto CLEANUP_ON_ERROR;
     }
 
-    Py_DECREF(py_roles);
+    *py_user_dict_ref = 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;
 }
 

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

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}

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):