From 4785870bd76bfdd4895969d26c297eb379f65bb2 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 27 Jun 2025 04:52:25 +0930 Subject: [PATCH 1/4] perf: select related objects in container queries to improve performance when fetching lots of children / parents --- .../apps/authoring/publishing/api.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 871df8e25..8966bc128 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -1300,7 +1300,18 @@ def get_entities_in_container( raise ContainerVersion.DoesNotExist # This container has not been published yet, or has been deleted. assert isinstance(container_version, ContainerVersion) entity_list = [] - for row in container_version.entity_list.entitylistrow_set.order_by("order_num"): + for row in container_version.entity_list.entitylistrow_set.select_related( + "entity_version", + "entity_version__componentversion", + "entity__published__version", + "entity__published__version__componentversion", + "entity__published__version__containerversion__unitversion", + "entity__published__version__containerversion__subsectionversion", + "entity__draft__version", + "entity__draft__version__componentversion", + "entity__draft__version__containerversion__unitversion", + "entity__draft__version__containerversion__subsectionversion", + ).order_by("order_num"): entity_version = row.entity_version # This will be set if pinned if not entity_version: # If this entity is "unpinned", use the latest published/draft version: entity_version = row.entity.published.version if published else row.entity.draft.version @@ -1393,7 +1404,10 @@ def get_containers_with_entity( qs = Container.objects.filter( publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk, # pylint: disable=line-too-long # noqa: E501 ) - return qs.order_by("pk").distinct() # Ordering is mostly for consistent test cases. + return qs.select_related( + "publishable_entity__draft__version__containerversion", + "publishable_entity__published__version__containerversion", + ).order_by("pk").distinct() # Ordering is mostly for consistent test cases. def get_container_children_count( From c53a5398c43b9ae4cf72bbbfe65c101ba5cf3983 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 2 Jul 2025 22:24:21 +0930 Subject: [PATCH 2/4] perf: select more related objects in container queries improves performance a little bit more. --- .../apps/authoring/publishing/api.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 8966bc128..2139b50a1 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -1301,16 +1301,15 @@ def get_entities_in_container( assert isinstance(container_version, ContainerVersion) entity_list = [] for row in container_version.entity_list.entitylistrow_set.select_related( - "entity_version", - "entity_version__componentversion", - "entity__published__version", - "entity__published__version__componentversion", - "entity__published__version__containerversion__unitversion", - "entity__published__version__containerversion__subsectionversion", - "entity__draft__version", - "entity__draft__version__componentversion", - "entity__draft__version__containerversion__unitversion", - "entity__draft__version__containerversion__subsectionversion", + "entity_version__componentversion__component", + "entity_version__containerversion__container", + "entity__published__version__containerversion__unitversion__container__unit", + "entity__published__version__containerversion__subsectionversion__container__subsection", + "entity__published__version__containerversion__sectionversion__container__section", + "entity__draft__version__componentversion__component", + "entity__draft__version__containerversion__unitversion__container__unit", + "entity__draft__version__containerversion__subsectionversion__container__subsection", + "entity__draft__version__containerversion__sectionversion__container__section", ).order_by("order_num"): entity_version = row.entity_version # This will be set if pinned if not entity_version: # If this entity is "unpinned", use the latest published/draft version: From 559eee88de636a5ac0e193a701a05d3686a359ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 23 Jul 2025 18:42:45 -0300 Subject: [PATCH 3/4] chore: update version --- openedx_learning/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index bd62127ba..dcc9a2acf 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -2,4 +2,4 @@ Open edX Learning ("Learning Core"). """ -__version__ = "0.27.0" +__version__ = "0.27.1" From 666ae68425dd7b654be9f4cd7371a09f093d7860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 6 Aug 2025 16:22:27 -0300 Subject: [PATCH 4/4] fix: remove container hierarchy information from publish api --- .../apps/authoring/publishing/api.py | 34 +++++++++++++------ .../apps/authoring/sections/api.py | 7 +++- .../apps/authoring/subsections/api.py | 7 +++- openedx_learning/apps/authoring/units/api.py | 7 +++- .../apps/authoring/sections/test_api.py | 26 ++++++++++++++ .../apps/authoring/subsections/test_api.py | 26 ++++++++++++++ .../apps/authoring/units/test_api.py | 26 ++++++++++++++ 7 files changed, 119 insertions(+), 14 deletions(-) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 2139b50a1..159b8aec0 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -1283,6 +1283,7 @@ def get_entities_in_container( container: Container, *, published: bool, + select_related_version: str | None = None, ) -> list[ContainerEntityListEntry]: """ [ 🛑 UNSTABLE ] @@ -1293,23 +1294,34 @@ def get_entities_in_container( container: The Container, e.g. returned by `get_container()` published: `True` if we want the published version of the container, or `False` for the draft version. + select_related_version: An optional optimization; specify a relationship + on ContainerVersion, like `componentversion` or `containerversion__x` + to preload via select_related. """ assert isinstance(container, Container) - container_version = container.versioning.published if published else container.versioning.draft + if published: + # Very minor optimization: reload the container with related 1:1 entities + container = Container.objects.select_related( + "publishable_entity__published__version__containerversion__entity_list").get(pk=container.pk) + container_version = container.versioning.published + select_related = ["entity__published__version"] + if select_related_version: + select_related.append(f"entity__published__version__{select_related_version}") + else: + # Very minor optimization: reload the container with related 1:1 entities + container = Container.objects.select_related( + "publishable_entity__draft__version__containerversion__entity_list").get(pk=container.pk) + container_version = container.versioning.draft + select_related = ["entity__draft__version"] + if select_related_version: + select_related.append(f"entity__draft__version__{select_related_version}") if container_version is None: raise ContainerVersion.DoesNotExist # This container has not been published yet, or has been deleted. assert isinstance(container_version, ContainerVersion) - entity_list = [] + entity_list: list[ContainerEntityListEntry] = [] for row in container_version.entity_list.entitylistrow_set.select_related( - "entity_version__componentversion__component", - "entity_version__containerversion__container", - "entity__published__version__containerversion__unitversion__container__unit", - "entity__published__version__containerversion__subsectionversion__container__subsection", - "entity__published__version__containerversion__sectionversion__container__section", - "entity__draft__version__componentversion__component", - "entity__draft__version__containerversion__unitversion__container__unit", - "entity__draft__version__containerversion__subsectionversion__container__subsection", - "entity__draft__version__containerversion__sectionversion__container__section", + "entity_version", + *select_related, ).order_by("order_num"): entity_version = row.entity_version # This will be set if pinned if not entity_version: # If this entity is "unpinned", use the latest published/draft version: diff --git a/openedx_learning/apps/authoring/sections/api.py b/openedx_learning/apps/authoring/sections/api.py index b5dc9b589..462809a57 100644 --- a/openedx_learning/apps/authoring/sections/api.py +++ b/openedx_learning/apps/authoring/sections/api.py @@ -257,7 +257,12 @@ def get_subsections_in_section( """ assert isinstance(section, Section) subsections = [] - for entry in publishing_api.get_entities_in_container(section, published=published): + entries = publishing_api.get_entities_in_container( + section, + published=published, + select_related_version="containerversion__subsectionversion", + ) + for entry in entries: # Convert from generic PublishableEntityVersion to SubsectionVersion: subsection_version = entry.entity_version.containerversion.subsectionversion assert isinstance(subsection_version, SubsectionVersion) diff --git a/openedx_learning/apps/authoring/subsections/api.py b/openedx_learning/apps/authoring/subsections/api.py index 98f99a761..5e8bd8fbd 100644 --- a/openedx_learning/apps/authoring/subsections/api.py +++ b/openedx_learning/apps/authoring/subsections/api.py @@ -256,7 +256,12 @@ def get_units_in_subsection( """ assert isinstance(subsection, Subsection) units = [] - for entry in publishing_api.get_entities_in_container(subsection, published=published): + entries = publishing_api.get_entities_in_container( + subsection, + published=published, + select_related_version="containerversion__unitversion", + ) + for entry in entries: # Convert from generic PublishableEntityVersion to UnitVersion: unit_version = entry.entity_version.containerversion.unitversion assert isinstance(unit_version, UnitVersion) diff --git a/openedx_learning/apps/authoring/units/api.py b/openedx_learning/apps/authoring/units/api.py index 2ceb6eed0..7256c7060 100644 --- a/openedx_learning/apps/authoring/units/api.py +++ b/openedx_learning/apps/authoring/units/api.py @@ -257,7 +257,12 @@ def get_components_in_unit( """ assert isinstance(unit, Unit) components = [] - for entry in publishing_api.get_entities_in_container(unit, published=published): + entries = publishing_api.get_entities_in_container( + unit, + published=published, + select_related_version="componentversion", + ) + for entry in entries: # Convert from generic PublishableEntityVersion to ComponentVersion: component_version = entry.entity_version.componentversion assert isinstance(component_version, ComponentVersion) diff --git a/tests/openedx_learning/apps/authoring/sections/test_api.py b/tests/openedx_learning/apps/authoring/sections/test_api.py index da0870361..4b269d3ef 100644 --- a/tests/openedx_learning/apps/authoring/sections/test_api.py +++ b/tests/openedx_learning/apps/authoring/sections/test_api.py @@ -993,6 +993,32 @@ def test_sections_containing(self): ] assert result2 == [section4_unpinned] + def test_get_subsections_in_section_queries(self): + """ + Test the query count of get_subsections_in_section() + This also tests the generic method get_entities_in_container() + """ + section = self.create_section_with_subsections([ + self.subsection_1, + self.subsection_2, + self.subsection_2_v1, + ]) + with self.assertNumQueries(4): + result = authoring_api.get_subsections_in_section(section, published=False) + assert result == [ + Entry(self.subsection_1.versioning.draft), + Entry(self.subsection_2.versioning.draft), + Entry(self.subsection_2.versioning.draft, pinned=True), + ] + authoring_api.publish_all_drafts(self.learning_package.id) + with self.assertNumQueries(4): + result = authoring_api.get_subsections_in_section(section, published=True) + assert result == [ + Entry(self.subsection_1.versioning.draft), + Entry(self.subsection_2.versioning.draft), + Entry(self.subsection_2.versioning.draft, pinned=True), + ] + def test_add_remove_container_children(self): """ Test adding and removing children subsections from sections. diff --git a/tests/openedx_learning/apps/authoring/subsections/test_api.py b/tests/openedx_learning/apps/authoring/subsections/test_api.py index 9593d739e..847336611 100644 --- a/tests/openedx_learning/apps/authoring/subsections/test_api.py +++ b/tests/openedx_learning/apps/authoring/subsections/test_api.py @@ -1024,6 +1024,32 @@ def test_subsections_containing(self): ] assert result2 == [subsection4_unpinned] + def test_get_units_in_subsection_queries(self): + """ + Test the query count of get_units_in_subsection() + This also tests the generic method get_entities_in_container() + """ + subsection = self.create_subsection_with_units([ + self.unit_1, + self.unit_2, + self.unit_2_v1, + ]) + with self.assertNumQueries(4): + result = authoring_api.get_units_in_subsection(subsection, published=False) + assert result == [ + Entry(self.unit_1.versioning.draft), + Entry(self.unit_2.versioning.draft), + Entry(self.unit_2.versioning.draft, pinned=True), + ] + authoring_api.publish_all_drafts(self.learning_package.id) + with self.assertNumQueries(4): + result = authoring_api.get_units_in_subsection(subsection, published=True) + assert result == [ + Entry(self.unit_1.versioning.draft), + Entry(self.unit_2.versioning.draft), + Entry(self.unit_2.versioning.draft, pinned=True), + ] + def test_add_remove_container_children(self): """ Test adding and removing children units from subsections. diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index 8ea98d95a..3c3caa7d9 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/tests/openedx_learning/apps/authoring/units/test_api.py @@ -1012,6 +1012,32 @@ def test_units_containing(self): ] assert result2 == [unit4_unpinned, unit7_several] + def test_get_components_in_unit_queries(self): + """ + Test the query count of get_components_in_unit() + This also tests the generic method get_entities_in_container() + """ + unit = self.create_unit_with_components([ + self.component_1, + self.component_2, + self.component_2_v1, + ]) + with self.assertNumQueries(3): + result = authoring_api.get_components_in_unit(unit, published=False) + assert result == [ + Entry(self.component_1.versioning.draft), + Entry(self.component_2.versioning.draft), + Entry(self.component_2.versioning.draft, pinned=True), + ] + authoring_api.publish_all_drafts(self.learning_package.id) + with self.assertNumQueries(3): + result = authoring_api.get_components_in_unit(unit, published=True) + assert result == [ + Entry(self.component_1.versioning.draft), + Entry(self.component_2.versioning.draft), + Entry(self.component_2.versioning.draft, pinned=True), + ] + def test_add_remove_container_children(self): """ Test adding and removing children components from containers.