From 24bff424d970b7b942e118a91ed7d5653b93236a Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 15 Jul 2025 14:48:02 +1000 Subject: [PATCH] MPTT rebuild tweak (#10020) * More efficient call * Tree save fix - Do not rebuild tree when not required - Reduce DB count for API test * Reduce query count even further * Reduce query limit for other tests * Fix for parent getter * Added condition * Remove duplicate check * Don't pre-fill tree values * perform atomic delete * Adjust unit tests --- src/backend/InvenTree/InvenTree/models.py | 15 ++++++++++----- src/backend/InvenTree/build/test_api.py | 7 ++++--- src/backend/InvenTree/build/tests.py | 6 ++++++ src/backend/InvenTree/order/test_api.py | 3 +-- src/backend/InvenTree/part/test_category.py | 8 +++++++- src/backend/InvenTree/plugin/base/event/events.py | 13 +++---------- src/backend/InvenTree/stock/models.py | 2 +- 7 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/models.py b/src/backend/InvenTree/InvenTree/models.py index 554ba1062d..998164b01f 100644 --- a/src/backend/InvenTree/InvenTree/models.py +++ b/src/backend/InvenTree/InvenTree/models.py @@ -718,6 +718,7 @@ class InvenTreeTree(MPTTModel): if parent: # If we have a parent, use the parent's tree_id self.tree_id = parent.tree_id + self.level = parent.level + 1 else: # Otherwise, we need to generate a new tree_id self.tree_id = self.getNextTreeID() @@ -736,23 +737,27 @@ class InvenTreeTree(MPTTModel): trees = set() + parent = getattr(self, self.NODE_PARENT_KEY, None) + if db_instance: # If the tree_id or parent has changed, we need to rebuild the tree - if getattr(db_instance, self.NODE_PARENT_KEY) != getattr( - self, self.NODE_PARENT_KEY - ): + if getattr(db_instance, self.NODE_PARENT_KEY) != parent: trees.add(db_instance.tree_id) if db_instance.tree_id != self.tree_id: trees.add(self.tree_id) trees.add(db_instance.tree_id) - else: - # New instance, so we need to rebuild the tree + elif parent: + # New instance, so we need to rebuild the tree (if it has a parent) trees.add(self.tree_id) for tree_id in trees: if tree_id: self.partial_rebuild(tree_id) + if len(trees) > 0: + # A tree update was performed, so we need to refresh the instance + self.refresh_from_db() + def partial_rebuild(self, tree_id: int) -> bool: """Perform a partial rebuild of the tree structure. diff --git a/src/backend/InvenTree/build/test_api.py b/src/backend/InvenTree/build/test_api.py index 3a19aa6623..1ffc8cd031 100644 --- a/src/backend/InvenTree/build/test_api.py +++ b/src/backend/InvenTree/build/test_api.py @@ -166,6 +166,7 @@ class BuildTest(BuildAPITest): # We shall complete 4 of these outputs outputs = self.build.incomplete_outputs.all() + # TODO: (2025-07-15) Try to optimize this API query to reduce DB hits self.post( self.url, { @@ -174,7 +175,7 @@ class BuildTest(BuildAPITest): 'status': StockStatus.ATTENTION.value, }, expected_code=201, - max_query_count=600, # TODO: Try to optimize this + max_query_count=400, ) self.assertEqual(self.build.incomplete_outputs.count(), 0) @@ -976,7 +977,7 @@ class BuildOverallocationTest(BuildAPITest): self.url, {'accept_overallocated': 'accept'}, expected_code=201, - max_query_count=1000, # TODO: Come back and refactor this + max_query_count=375, ) self.build.refresh_from_db() @@ -995,7 +996,7 @@ class BuildOverallocationTest(BuildAPITest): self.url, {'accept_overallocated': 'trim'}, expected_code=201, - max_query_count=1000, # TODO: Come back and refactor this + max_query_count=375, ) # Note: Large number of queries is due to pricing recalculation for each stock item diff --git a/src/backend/InvenTree/build/tests.py b/src/backend/InvenTree/build/tests.py index 9856927ad2..2d94cb4c13 100644 --- a/src/backend/InvenTree/build/tests.py +++ b/src/backend/InvenTree/build/tests.py @@ -199,6 +199,8 @@ class BuildTreeTest(InvenTreeTestCase): # Test the tree structure for each node for idx, child in enumerate(builds): + child.refresh_from_db() + # Check parent-child relationships expected_parent = builds[idx - 1] if idx > 0 else None self.assertEqual(child.parent, expected_parent) @@ -278,12 +280,16 @@ class BuildTreeTest(InvenTreeTestCase): self.assertEqual(grandchild.tree_id, self.build.tree_id) self.assertEqual(grandchild.level, 2) + child.refresh_from_db() + self.assertEqual(child.get_children().count(), 3) self.assertEqual(child.get_descendants(include_self=False).count(), 3) self.assertEqual(child.level, 1) self.assertEqual(child.tree_id, self.build.tree_id) + self.build.refresh_from_db() + # Basic tests self.assertEqual(Build.objects.count(), 13) self.assertEqual(self.build.get_children().count(), 3) diff --git a/src/backend/InvenTree/order/test_api.py b/src/backend/InvenTree/order/test_api.py index 42f702da19..331ce1e257 100644 --- a/src/backend/InvenTree/order/test_api.py +++ b/src/backend/InvenTree/order/test_api.py @@ -1182,8 +1182,7 @@ class PurchaseOrderReceiveTest(OrderTest): n = StockItem.objects.count() - # TODO: 2024-12-10 - This API query needs to be refactored! - self.post(self.url, data, expected_code=201, max_query_count=500) + self.post(self.url, data, expected_code=201, max_query_count=275) # Check that the expected number of stock items has been created self.assertEqual(n + 11, StockItem.objects.count()) diff --git a/src/backend/InvenTree/part/test_category.py b/src/backend/InvenTree/part/test_category.py index d9f45071e1..c7672245e5 100644 --- a/src/backend/InvenTree/part/test_category.py +++ b/src/backend/InvenTree/part/test_category.py @@ -267,7 +267,13 @@ class CategoryTest(TestCase): and the correct ancestor tree is observed. """ # Clear out any existing parts - Part.objects.all().delete() + for p in Part.objects.all(): + if p.active: + p.refresh_from_db() + p.active = False + p.save() + + p.delete() # First, create a structured tree of part categories A = PartCategory.objects.create(name='A', description='Top level category') diff --git a/src/backend/InvenTree/plugin/base/event/events.py b/src/backend/InvenTree/plugin/base/event/events.py index f59330b754..02e7b42b04 100644 --- a/src/backend/InvenTree/plugin/base/event/events.py +++ b/src/backend/InvenTree/plugin/base/event/events.py @@ -73,19 +73,12 @@ def register_event(event, *args, **kwargs): registry.check_reload() with transaction.atomic(): - for slug, plugin in registry.plugins.items(): - if not plugin.mixin_enabled(PluginMixinEnum.EVENTS): - continue - - # Only allow event registering for 'active' plugins - if not plugin.is_active(): - continue - + for plugin in registry.with_mixin(PluginMixinEnum.EVENTS, active=True): # Let the plugin decide if it wants to process this event if not plugin.wants_process_event(event): continue - logger.debug("Registering callback for plugin '%s'", slug) + logger.debug("Registering callback for plugin '%s'", plugin.slug) # This task *must* be processed by the background worker, # unless we are running CI tests @@ -94,7 +87,7 @@ def register_event(event, *args, **kwargs): # Offload a separate task for each plugin offload_task( - process_event, slug, event, *args, group='plugin', **kwargs + process_event, plugin.slug, event, *args, group='plugin', **kwargs ) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index 80493d5610..5b40aa0aed 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -811,7 +811,7 @@ class StockItem( super().save(*args, **kwargs) # If user information is provided, and no existing note exists, create one! - if user and self.tracking_info.count() == 0: + if user and add_note and self.tracking_info.count() == 0: tracking_info = {'status': self.status} self.add_tracking_entry(