YDA-6977: fix get user group function stats

This ensures that we don't exceed the maximum GenQuery size
when a non-rodsadmin user is viewing group statistics.

Author: stsnel

stats.py

@@ -14,6 +14,7 @@
 
 import groups
 from util import *
+from util.misc import split_string_list_by_total_length
 
 __all__ = ['api_resource_browse_group_data',
            'api_resource_monthly_category_stats',
@@ -406,7 +407,7 @@ def get_resource_monthly_category_stats(ctx: rule.Context) -> Dict:
             if group not in group_storage:
                 # This can happen if we have a group without a matching group collection
                 log.write(ctx, f"Warning: ignoring group {group} for category statistics, because storage data not found "
-                               + "(possibly a group without a group collection?)")
+                          + "(possibly a group without a group collection?)")
                 continue
             elif len(group_storage[group]) == record_count:
                 group_storage[group].append(0)
@@ -685,7 +686,7 @@ def get_user_groups_for_stats(ctx: rule.Context, search_filter: str = "", user_n
 
     :returns: All groups of current session's user
     """
-    groups_list = []
+    groups = set()
 
     user_name = user.name(ctx) if user_name is None else user_name
     user_zone = user.zone(ctx) if zone_name is None else zone_name
@@ -696,42 +697,33 @@ def get_user_groups_for_stats(ctx: rule.Context, search_filter: str = "", user_n
 
     if user.is_rodsadmin(ctx, f"{user_name}#{user_zone}"):
         # All groups in zone
-        groups_list = list(genquery.Query(ctx,
+        groups.update(list(genquery.Query(ctx,
                                           "ORDER(USER_GROUP_NAME)",
-                                          group_filter + zone_filter + search_filter))
+                                          group_filter + zone_filter + search_filter)))
     else:
         # Groups the user is member of
         user_filter = f"AND USER_NAME = '{user_name}' "
-        group_member = list(genquery.Query(ctx,
-                                           "ORDER(USER_GROUP_NAME)",
-                                           group_filter + user_filter + search_filter))
-
-        for grp in group_member:
-            if grp not in groups_list:
-                groups_list.append(grp)
+        groups.update(list(genquery.Query(ctx,
+                                          "ORDER(USER_GROUP_NAME)",
+                                          group_filter + user_filter + search_filter)))
 
         # Groups the user is datamanager of
         dmgroup_member = list(genquery.Query(ctx,
                                              "ORDER(USER_GROUP_NAME)",
                                              "USER_GROUP_NAME LIKE 'datamanager-%' " + user_filter))
+        categories = [group.replace("datamanager-", "", 1) for group in dmgroup_member]
 
-        categories = []
-        for grp in dmgroup_member:
-            cat = grp.replace("datamanager-", "", 1)
-            categories.append(cat)
-
-        if len(categories) > 0:
-            quoted_categories = [f"'{e}'" for e in categories]
+        # Limit batch size so that it does not exceed maximum GenQuery size. We're using 14000 rather
+        # than 16000 so that we have a bit of slack for other parts of the query, like search filters.
+        category_batches = split_string_list_by_total_length(categories, 14000, add_item_length=3)
+        for category_batch in category_batches:
+            quoted_categories = [f"'{e}'" for e in category_batch]
             categories_string = f"({','.join(quoted_categories)})"
-            group_dm = list(genquery.Query(ctx,
-                                           "ORDER(USER_GROUP_NAME)",
-                                           group_filter + search_filter + f"AND META_USER_ATTR_NAME = 'category' AND META_USER_ATTR_VALUE IN {categories_string}"))
-
-            for grp in group_dm:
-                if grp not in groups_list:
-                    groups_list.append(grp)
+            groups.update(list(genquery.Query(ctx,
+                                              "ORDER(USER_GROUP_NAME)",
+                                              group_filter + search_filter + f"AND META_USER_ATTR_NAME = 'category' AND META_USER_ATTR_VALUE IN {categories_string}")))
 
-    return groups_list
+    return sorted(groups)
 
 
 def rule_resource_research(rule_args, callback, rei):

unit-tests/test_util_misc.py

@@ -10,7 +10,7 @@
 
 sys.path.append('../util')
 
-from misc import check_data_package_system_avus, human_readable_size, last_run_time_acceptable, remove_empty_objects
+from misc import check_data_package_system_avus, human_readable_size, last_run_time_acceptable, remove_empty_objects, split_string_list_by_total_length
 
 # AVs of a successfully published data package, that is the first version of the package
 avs_success_data_package = {
@@ -232,3 +232,23 @@ def test_remove_empty_objects(self):
         self.assertDictEqual(remove_empty_objects(d), OrderedDict({"key1": "value1", "key2": {"key5": "value5"}}))
         d = OrderedDict({"key1": "value1", "key2": [{}]})
         self.assertDictEqual(remove_empty_objects(d), OrderedDict({"key1": "value1"}))
+
+    def test_split_string_list_by_total_length(self):
+        # Items don't exceed maximum length
+        self.assertEqual(split_string_list_by_total_length(["abc", "def", "ghi"], 10),
+                         [["abc", "def", "ghi"]])
+        # Items exceed maximum length
+        self.assertEqual(split_string_list_by_total_length(["abc", "def", "ghi", "jkl"], 10),
+                         [["abc", "def", "ghi"], ["jkl"]])
+        # Items don't exceed maximum length with additional length
+        self.assertEqual(split_string_list_by_total_length(["abc", "def", "ghi"], 15, add_item_length=2),
+                         [["abc", "def", "ghi"]])
+        # Items exceed maximum length with additional length
+        self.assertEqual(split_string_list_by_total_length(["abc", "def", "ghi"], 14, add_item_length=2),
+                         [["abc", "def"], ["ghi"]])
+        # Single item exceeds maximum length
+        self.assertEqual(split_string_list_by_total_length(["abcabcabcabcabcabcabc", "def", "ghi"], 15),
+                         [["abcabcabcabcabcabcabc"], ["def", "ghi"]])
+        # Single item exceeds maximum length and throws Exception
+        with self.assertRaises(Exception):  # noqa B107  / Ruff does not permit asserting exceptions in unit tests
+            split_string_list_by_total_length(["abcabcabcabcabcabcabc", "def", "ghi"], 15, raise_exception_exceed=True)

util/misc.py

@@ -7,7 +7,7 @@
 import time
 import uuid
 from collections import OrderedDict
-from typing import Dict
+from typing import Dict, List
 
 import constants
 
@@ -154,3 +154,63 @@ def is_valid_uuid(uuid_string: str) -> bool:
         return False
 
     return str(uuid_obj) == uuid_string
+
+
+def split_string_list_by_total_length(string_list: List[str], max_length: int, add_item_length: int = 0, raise_exception_exceed: bool = False) -> List[List[str]]:
+    """Split a list of strings into sublists where the total length of all strings
+       in a sublist does not exceed the maximum length. This can be useful when you want
+       to process a list of strings, but need to take into account a maximum length supported
+       by iRODS (e.g. for GenQueries).
+
+       :param string_list: List of strings to process
+       :param max_length: The maximum length of each sublist in the output. By default, if a single string
+                          (along with its additional item length, if applicable) exceeds the maximum length,
+                          if it included in a sublist by itself.
+       :param add_item_length: Additional item length. Increase the length of each string by this number.
+                          This is useful if you need to add additional characters to each string when you
+                          use the sublists (e.g. separator characters or quote characters)
+       :param raise_exception_exceed: Raise an exception if the length of an individual string (along with
+                          its additional item length) exceeds the maximum length, rather than including the
+                          string in its own sublist.
+
+       :raises Exception: if a single string in the input list plus the additional item length exceeds
+                          the maximum length, so that it is not possible to strictly meet the maximum
+                          length requirement. By default, such long strings are included in a sublist
+                          by themselves, and no exception is raised.
+
+       :returns: List of sublists, where each sublist is either a single string, or a list of strings
+                          whose total size does not exceed the maximum length.
+
+    """
+    output: List[List[str]] = []
+    current_sublist: List[str] = []
+    current_length: List[int] = [0]  # In a list so that we can pass it by reference to subfunctions
+
+    def end_of_sublist(current_sublist: List[str], current_length: List[int], output: List[List[str]]) -> None:
+        if len(current_sublist) > 0:
+            output.append(current_sublist.copy())
+            current_sublist.clear()
+            current_length[0] = 0
+
+    def add_to_sublist(item: str, effective_length: int, current_sublist: List[str], current_length: List[int]) -> None:
+        current_sublist.append(item)
+        current_length[0] += effective_length
+
+    for item in string_list:
+        effective_length = len(item) + add_item_length
+        if effective_length > max_length:
+            if raise_exception_exceed:
+                raise Exception(f"Item '{item}' exceeded maximum sublist length.")
+            else:
+                end_of_sublist(current_sublist, current_length, output)
+                add_to_sublist(item, effective_length, current_sublist, current_length)
+                end_of_sublist(current_sublist, current_length, output)
+        elif current_length[0] + effective_length > max_length:
+            end_of_sublist(current_sublist, current_length, output)
+            add_to_sublist(item, effective_length, current_sublist, current_length)
+        else:
+            add_to_sublist(item, effective_length, current_sublist, current_length)
+
+    end_of_sublist(current_sublist, current_length, output)
+
+    return output