From 5339b807433c79ffcbfdc3c7f447273f44024819 Mon Sep 17 00:00:00 2001 From: Johnny Date: Fri, 10 Jan 2020 20:56:44 +0000 Subject: [PATCH 1/7] Makes CmdFind querying use iterators to minimize memory ballooning. Refactors some redundant rendering code. --- evennia/commands/default/building.py | 100 ++++++++++++++++----------- evennia/commands/default/tests.py | 12 ++++ 2 files changed, 73 insertions(+), 39 deletions(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index 0d2612fe89..9db74cebe8 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -3,7 +3,7 @@ Building and world design commands """ import re from django.conf import settings -from django.db.models import Q +from django.db.models import Q, Min, Max from evennia.objects.models import ObjectDB from evennia.locks.lockhandler import LockException from evennia.commands.cmdhandler import get_and_merge_cmdsets @@ -2778,8 +2778,35 @@ class CmdFind(COMMAND_DEFAULT_CLASS): switches.append("loc") searchstring = self.lhs - low, high = 1, ObjectDB.objects.all().order_by("-id")[0].id + + try: + # Try grabbing the actual min/max id values by database aggregation + qs = ObjectDB.objects.values('id').aggregate(low=Min('id'), high=Max('id')) + low, high = sorted(qs.values()) + if not (low and high): + raise ValueError(f"{self.__class__.__name__}: Min and max ID not returned by aggregation; falling back to queryset slicing.") + except Exception as e: + logger.log_trace(e) + # If that doesn't work for some reason (empty DB?), guess the lower + # bound and do a less-efficient query to find the upper. + low, high = 1, ObjectDB.objects.all().order_by("-id")[0].id + if self.rhs: + # Check that rhs is either a valid dbref or dbref range + try: + # Get rid of # signs, split on hyphen or space and cast all to int. + # Then sort by number to get the lowest and highest values + # comprising the bounds. + bounds = sorted(int(x) for x in re.split('[-\s]+', self.rhs.strip().replace('#', ''))) + except ValueError: + caller.msg("Invalid dbref range provided (not a number).") + return + + low = bounds[0] + if len(bounds) > 1: + high = bounds[-1] + + """ if "-" in self.rhs: # also support low-high syntax limlist = [part.lstrip("#").strip() for part in self.rhs.split("-", 1)] @@ -2790,6 +2817,7 @@ class CmdFind(COMMAND_DEFAULT_CLASS): low = max(low, int(limlist[0])) if len(limlist) > 1 and limlist[1].isdigit(): high = min(high, int(limlist[1])) + """ low = min(low, high) high = max(low, high) @@ -2868,51 +2896,45 @@ class CmdFind(COMMAND_DEFAULT_CLASS): results = ObjectDB.objects.filter(keyquery | aliasquery).distinct() nresults = results.count() - + + # Use iterator to minimize memory ballooning on large result sets + results = results.iterator() + if nresults: - # convert result to typeclasses. - results = [result for result in results] + # filter results by typeclasses, if requested + obj_ids = [] if "room" in switches: - results = [ - obj for obj in results if inherits_from(obj, ROOM_TYPECLASS) - ] + obj_ids.extend([ + obj.id for obj in results if inherits_from(obj, ROOM_TYPECLASS) + ]) if "exit" in switches: - results = [ - obj for obj in results if inherits_from(obj, EXIT_TYPECLASS) - ] + obj_ids.extend([ + obj.id for obj in results if inherits_from(obj, EXIT_TYPECLASS) + ]) if "char" in switches: - results = [ - obj for obj in results if inherits_from(obj, CHAR_TYPECLASS) - ] - nresults = len(results) + obj_ids.extend([ + obj.id for obj in results if inherits_from(obj, CHAR_TYPECLASS) + ]) + if obj_ids: + filtered_result_qs = ObjectDB.objects.filter(id__in=set(obj_ids)).distinct() + nresults = filtered_result_qs.count() + + # Keep using iterator to minimize memory ballooning + results = filtered_result_qs.iterator() # still results after type filtering? if nresults: - if nresults > 1: - string = "|w%i Matches|n(#%i-#%i%s):" % ( - nresults, - low, - high, - restrictions, - ) - for res in results: - string += "\n |g%s - %s|n" % ( - res.get_display_name(caller), - res.path, - ) - else: - string = "|wOne Match|n(#%i-#%i%s):" % (low, high, restrictions) - string += "\n |g%s - %s|n" % ( - results[0].get_display_name(caller), - results[0].path, - ) - if "loc" in self.switches and nresults == 1 and results[0].location: - string += " (|wlocation|n: |g{}|n)".format( - results[0].location.get_display_name(caller) - ) + if nresults > 1: header = f'{nresults} Matches' + else: header = 'One Match' + + string = f"|w{header}|n(#{low}-#{high}{restrictions}):" + for res in results: + string += f"\n |g{res.get_display_name(caller)} - {res.path}|n" + if "loc" in self.switches and nresults == 1 and res and res.location: + string += f" (|wlocation|n: |g{res.location.get_display_name(caller)}|n)" else: - string = "|wMatch|n(#%i-#%i%s):" % (low, high, restrictions) - string += "\n |RNo matches found for '%s'|n" % searchstring + string = "|wMatch|n(#{low}-#{high}{restrictions}):" + string += "\n |RNo matches found for '{searchstring}'|n" # send result caller.msg(string.strip()) diff --git a/evennia/commands/default/tests.py b/evennia/commands/default/tests.py index 2751771fdb..d2373adf2f 100644 --- a/evennia/commands/default/tests.py +++ b/evennia/commands/default/tests.py @@ -1214,6 +1214,18 @@ class TestBuilding(CommandTest): self.call(building.CmdFind(), "/room Obj") self.call(building.CmdFind(), "/exit Obj") self.call(building.CmdFind(), "/exact Obj", "One Match") + + # Test bogus dbref range with no search term + self.call(building.CmdFind(), "= obj", "Invalid dbref range provided (not a number).") + self.call(building.CmdFind(), "= #1a", "Invalid dbref range provided (not a number).") + + # Test valid dbref ranges with no search term + self.call(building.CmdFind(), "=#1", "7 Matches(#1-#7)") + self.call(building.CmdFind(), "=1-2", "2 Matches(#1-#2):") + self.call(building.CmdFind(), "=1 - 2", "2 Matches(#1-#2):") + self.call(building.CmdFind(), "=1- #2", "2 Matches(#1-#2):") + self.call(building.CmdFind(), "=1-#2", "2 Matches(#1-#2):") + self.call(building.CmdFind(), "=#1-2", "2 Matches(#1-#2):") def test_script(self): self.call(building.CmdScript(), "Obj = ", "No scripts defined on Obj") From 54b25725387bd13f2adc6c54e02e05398d4f6642 Mon Sep 17 00:00:00 2001 From: Johnny Date: Fri, 10 Jan 2020 21:32:21 +0000 Subject: [PATCH 2/7] Adds checks for null search terms and dbref range. --- evennia/commands/default/building.py | 2 +- evennia/commands/default/tests.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index 9db74cebe8..6605f6a0cf 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -2768,7 +2768,7 @@ class CmdFind(COMMAND_DEFAULT_CLASS): caller = self.caller switches = self.switches - if not self.args: + if not self.args or (not self.lhs and not self.rhs): caller.msg("Usage: find [= low [-high]]") return diff --git a/evennia/commands/default/tests.py b/evennia/commands/default/tests.py index d2373adf2f..333e1e12fc 100644 --- a/evennia/commands/default/tests.py +++ b/evennia/commands/default/tests.py @@ -1215,6 +1215,9 @@ class TestBuilding(CommandTest): self.call(building.CmdFind(), "/exit Obj") self.call(building.CmdFind(), "/exact Obj", "One Match") + # Test null search + self.call(building.CmdFind(), "=", "Usage: ") + # Test bogus dbref range with no search term self.call(building.CmdFind(), "= obj", "Invalid dbref range provided (not a number).") self.call(building.CmdFind(), "= #1a", "Invalid dbref range provided (not a number).") From 3725d94e8855fd3a187be7607a258ca2341a55d1 Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 13 Jan 2020 19:01:33 +0000 Subject: [PATCH 3/7] Removes extraneous codeblock, fixes generator reuse, and addresses an unbound local error with rendering. --- evennia/commands/default/building.py | 58 ++++++++++------------------ 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index 6605f6a0cf..d14d8d5ca3 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -2806,18 +2806,6 @@ class CmdFind(COMMAND_DEFAULT_CLASS): if len(bounds) > 1: high = bounds[-1] - """ - if "-" in self.rhs: - # also support low-high syntax - limlist = [part.lstrip("#").strip() for part in self.rhs.split("-", 1)] - else: - # otherwise split by space - limlist = [part.lstrip("#") for part in self.rhs.split(None, 1)] - if limlist and limlist[0].isdigit(): - low = max(low, int(limlist[0])) - if len(limlist) > 1 and limlist[1].isdigit(): - high = min(high, int(limlist[1])) - """ low = min(low, high) high = max(low, high) @@ -2894,33 +2882,28 @@ class CmdFind(COMMAND_DEFAULT_CLASS): id__lte=high, ) - results = ObjectDB.objects.filter(keyquery | aliasquery).distinct() - nresults = results.count() + # Keep the initial queryset handy for later reuse + result_qs = ObjectDB.objects.filter(keyquery | aliasquery).distinct() + nresults = result_qs.count() # Use iterator to minimize memory ballooning on large result sets - results = results.iterator() + results = result_qs.iterator() - if nresults: - # filter results by typeclasses, if requested - obj_ids = [] - if "room" in switches: - obj_ids.extend([ - obj.id for obj in results if inherits_from(obj, ROOM_TYPECLASS) - ]) - if "exit" in switches: - obj_ids.extend([ - obj.id for obj in results if inherits_from(obj, EXIT_TYPECLASS) - ]) - if "char" in switches: - obj_ids.extend([ - obj.id for obj in results if inherits_from(obj, CHAR_TYPECLASS) - ]) - if obj_ids: - filtered_result_qs = ObjectDB.objects.filter(id__in=set(obj_ids)).distinct() - nresults = filtered_result_qs.count() - - # Keep using iterator to minimize memory ballooning - results = filtered_result_qs.iterator() + # Check and see if type filtering was requested; skip it if not + if any(x in switches for x in ("room", "exit", "char")): + obj_ids = set() + for obj in results: + if ("room" in switches and inherits_from(obj, ROOM_TYPECLASS)) \ + or ("exit" in switches and inherits_from(obj, EXIT_TYPECLASS)) \ + or ("char" in switches and inherits_from(obj, CHAR_TYPECLASS)): + obj_ids.add(obj.id) + + # Filter previous queryset instead of requesting another + filtered_qs = result_qs.filter(id__in=obj_ids).distinct() + nresults = filtered_qs.count() + + # Keep using iterator to minimize memory ballooning + results = filtered_qs.iterator() # still results after type filtering? if nresults: @@ -2928,9 +2911,10 @@ class CmdFind(COMMAND_DEFAULT_CLASS): else: header = 'One Match' string = f"|w{header}|n(#{low}-#{high}{restrictions}):" + res = None for res in results: string += f"\n |g{res.get_display_name(caller)} - {res.path}|n" - if "loc" in self.switches and nresults == 1 and res and res.location: + if "loc" in self.switches and nresults == 1 and res and getattr(res, 'location', None): string += f" (|wlocation|n: |g{res.location.get_display_name(caller)}|n)" else: string = "|wMatch|n(#{low}-#{high}{restrictions}):" From e03d4ccaf765c17b7426fefdf409e46243998d9f Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 13 Jan 2020 20:10:51 +0000 Subject: [PATCH 4/7] Cleans up some verbiage and syntax for fallback idrange. --- evennia/commands/default/building.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index d14d8d5ca3..fc2512a421 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -2789,7 +2789,7 @@ class CmdFind(COMMAND_DEFAULT_CLASS): logger.log_trace(e) # If that doesn't work for some reason (empty DB?), guess the lower # bound and do a less-efficient query to find the upper. - low, high = 1, ObjectDB.objects.all().order_by("-id")[0].id + low, high = 1, ObjectDB.objects.all().order_by("-id").first().id if self.rhs: # Check that rhs is either a valid dbref or dbref range @@ -2817,7 +2817,6 @@ class CmdFind(COMMAND_DEFAULT_CLASS): restrictions = ", %s" % (", ".join(self.switches)) if is_dbref or is_account: - if is_dbref: # a dbref search result = caller.search(searchstring, global_search=True, quiet=True) @@ -2854,7 +2853,7 @@ class CmdFind(COMMAND_DEFAULT_CLASS): ) else: # Not an account/dbref search but a wider search; build a queryset. - # Searchs for key and aliases + # Searches for key and aliases if "exact" in switches: keyquery = Q(db_key__iexact=searchstring, id__gte=low, id__lte=high) aliasquery = Q( @@ -2917,8 +2916,8 @@ class CmdFind(COMMAND_DEFAULT_CLASS): if "loc" in self.switches and nresults == 1 and res and getattr(res, 'location', None): string += f" (|wlocation|n: |g{res.location.get_display_name(caller)}|n)" else: - string = "|wMatch|n(#{low}-#{high}{restrictions}):" - string += "\n |RNo matches found for '{searchstring}'|n" + string = f"|wNo Matches|n(#{low}-#{high}{restrictions}):" + string += f"\n |RNo matches found for '{searchstring}'|n" # send result caller.msg(string.strip()) From 39fa8dacdf4f1d7f527107ccc9114c6475a0f7fe Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 13 Jan 2020 20:12:09 +0000 Subject: [PATCH 5/7] Improves existing and implements additional tests for multitype filtering. --- evennia/commands/default/tests.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/evennia/commands/default/tests.py b/evennia/commands/default/tests.py index 333e1e12fc..ca2c185ba8 100644 --- a/evennia/commands/default/tests.py +++ b/evennia/commands/default/tests.py @@ -1210,11 +1210,17 @@ class TestBuilding(CommandTest): self.call(building.CmdFind(), self.char1.dbref, "Exact dbref match") self.call(building.CmdFind(), "*TestAccount", "Match") - self.call(building.CmdFind(), "/char Obj") - self.call(building.CmdFind(), "/room Obj") - self.call(building.CmdFind(), "/exit Obj") + self.call(building.CmdFind(), "/char Obj", "No Matches") + self.call(building.CmdFind(), "/room Obj", "No Matches") + self.call(building.CmdFind(), "/exit Obj", "No Matches") self.call(building.CmdFind(), "/exact Obj", "One Match") + # Test multitype filtering + with mock.patch('evennia.commands.default.building.CHAR_TYPECLASS', 'evennia.objects.objects.DefaultCharacter'): + self.call(building.CmdFind(), "/char/room Obj", "No Matches") + self.call(building.CmdFind(), "/char/room/exit Char", "2 Matches") + self.call(building.CmdFind(), "/char/room/exit/startswith Cha", "2 Matches") + # Test null search self.call(building.CmdFind(), "=", "Usage: ") @@ -1229,7 +1235,7 @@ class TestBuilding(CommandTest): self.call(building.CmdFind(), "=1- #2", "2 Matches(#1-#2):") self.call(building.CmdFind(), "=1-#2", "2 Matches(#1-#2):") self.call(building.CmdFind(), "=#1-2", "2 Matches(#1-#2):") - + def test_script(self): self.call(building.CmdScript(), "Obj = ", "No scripts defined on Obj") self.call( From 0b38de00573fb016c6539b29ff8388f778b4fece Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 13 Jan 2020 20:40:46 +0000 Subject: [PATCH 6/7] Uses utils.dbref() for dbref validation. --- evennia/commands/default/building.py | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index fc2512a421..fe19ffdb96 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -13,6 +13,7 @@ from evennia.utils.utils import ( class_from_module, get_all_typeclasses, variable_from_module, + dbref, ) from evennia.utils.eveditor import EvEditor from evennia.utils.evmore import EvMore @@ -2792,19 +2793,25 @@ class CmdFind(COMMAND_DEFAULT_CLASS): low, high = 1, ObjectDB.objects.all().order_by("-id").first().id if self.rhs: - # Check that rhs is either a valid dbref or dbref range try: - # Get rid of # signs, split on hyphen or space and cast all to int. - # Then sort by number to get the lowest and highest values - # comprising the bounds. - bounds = sorted(int(x) for x in re.split('[-\s]+', self.rhs.strip().replace('#', ''))) - except ValueError: + # Check that rhs is either a valid dbref or dbref range + bounds = tuple(sorted(dbref(x, False) for x in re.split('[-\s]+', self.rhs.strip()))) + + # dbref() will return either a valid int or None + assert bounds + # None should not exist in the bounds list + assert None not in bounds + + low = bounds[0] + if len(bounds) > 1: + high = bounds[-1] + + except AssertionError: caller.msg("Invalid dbref range provided (not a number).") return - - low = bounds[0] - if len(bounds) > 1: - high = bounds[-1] + except IndexError as e: + logger.log_err(f"{self.__class__.__name__}: Error parsing upper and lower bounds of query.") + logger.log_trace(e) low = min(low, high) high = max(low, high) @@ -2901,7 +2908,7 @@ class CmdFind(COMMAND_DEFAULT_CLASS): filtered_qs = result_qs.filter(id__in=obj_ids).distinct() nresults = filtered_qs.count() - # Keep using iterator to minimize memory ballooning + # Use iterator again to minimize memory ballooning results = filtered_qs.iterator() # still results after type filtering? From 69c6f3b44323034d811efcff50ff2bd06eb4f86c Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 13 Jan 2020 20:46:42 +0000 Subject: [PATCH 7/7] Resolves trailing whitespace. --- evennia/commands/default/building.py | 2 +- evennia/commands/default/tests.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/evennia/commands/default/building.py b/evennia/commands/default/building.py index fe19ffdb96..5c83f437fa 100644 --- a/evennia/commands/default/building.py +++ b/evennia/commands/default/building.py @@ -3707,4 +3707,4 @@ class CmdSpawn(COMMAND_DEFAULT_CLASS): for obj in spawner.spawn(prototype): self.caller.msg("Spawned %s." % obj.get_display_name(self.caller)) except RuntimeError as err: - caller.msg(err) + caller.msg(err) \ No newline at end of file diff --git a/evennia/commands/default/tests.py b/evennia/commands/default/tests.py index ca2c185ba8..779ed580e5 100644 --- a/evennia/commands/default/tests.py +++ b/evennia/commands/default/tests.py @@ -1700,4 +1700,4 @@ class TestSystemCommands(CommandTest): mock_channeldb.objects.get_channel = mock.MagicMock(return_value=channel) self.call(syscommands.SystemSendToChannel(), "public:Hello") - channel.msg.assert_called() + channel.msg.assert_called() \ No newline at end of file