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

src/include/conversions.h

@@ -62,9 +62,9 @@ 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);
+PyObject *
+convert_array_of_role_strs_to_py_list(as_error *err, int num_elements,
+                                      char str_array_ptr[][AS_ROLE_SIZE]);
 
 as_status char_double_ptr_to_py_list(as_error *err, int num_elements,
                                      int element_size, char **str_array_ptr,

src/main/conversions.c

@@ -144,12 +144,17 @@ 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_ptr[][AS_ROLE_SIZE])
 {
     as_error_reset(err);
 
+    PyObject *py_list = PyList_New(0);
+    if (!py_list) {
+        goto error;
+    }
+
     char *str;
 
     for (int i = 0; i < num_elements; i++) {
@@ -158,14 +163,24 @@ as_status str_array_of_roles_to_py_list(as_error *err, int num_elements,
         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 build 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,101 @@ as_status as_partitions_status_to_pyobject(
     return err->code;
 }
 
+PyObject *convert_nullable_array_of_uint32_to_py_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) {
+        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_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;
 }
 

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