Skip to content
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.6.1"
__version__ = "0.6.2"
6 changes: 6 additions & 0 deletions openedx_tagging/core/tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,12 @@ def copy(self, taxonomy: Taxonomy) -> Taxonomy:
self.visible_to_authors = taxonomy.visible_to_authors
self.export_id = taxonomy.export_id
self._taxonomy_class = taxonomy._taxonomy_class # pylint: disable=protected-access

# Copy Django's internal prefetch_related cache to reduce queries required on the casted taxonomy.
if hasattr(taxonomy, '_prefetched_objects_cache'):
# pylint: disable=protected-access,attribute-defined-outside-init
self._prefetched_objects_cache: dict = taxonomy._prefetched_objects_cache

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I used code from this blog to make this change. That blog post also copies the caches for one-to-many fields that were cached using select_related, but since Taxonomy doesn't have any one-to-many fields, I omitted that part.

return self

def get_filtered_tags(
Expand Down
22 changes: 21 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import rules # type: ignore[import]
from rest_framework.permissions import DjangoObjectPermissions

from ...models import Tag
from ...models import Tag, Taxonomy


class TaxonomyObjectPermissions(DjangoObjectPermissions):
Expand Down Expand Up @@ -58,3 +58,23 @@ def has_object_permission(self, request, view, obj):
"""
obj = obj.taxonomy if isinstance(obj, Tag) else obj
return rules.has_perm("oel_tagging.list_tag", request.user, obj)

def has_permission(self, request, view):
"""
Returns True if the request user is allowed the given view on the Taxonomy model.

We override this method to avoid calling our view's get_queryset(), which performs database queries.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, do you know why this is happening?

I didn't find a place where we are actually evaluating the query. Were the queries from the Taxonomy model or some other tables?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's being called by Django Rest Framework: APIView.initial calls DjangoModelPermissions.has_permission()

"""
# Workaround to ensure DjangoModelPermissions are not applied
# to the root view when using DefaultRouter.
Comment thread
bradenmacdonald marked this conversation as resolved.
Outdated
if getattr(view, '_ignore_model_permissions', False):
return True

if not request.user or (
not request.user.is_authenticated and self.authenticated_users_only):
return False

queryset = Taxonomy.objects
perms = self.get_required_permissions(request.method, queryset.model)

return request.user.has_perms(perms)
7 changes: 7 additions & 0 deletions openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ def to_representation(self, instance):
return super().to_representation(instance)

def get_tags_count(self, instance):
"""
Return the "tags_count" annotation if present.

Or just count the taxonomy's tags.
"""
if hasattr(instance, 'tags_count'):
return instance.tags_count
Comment on lines +108 to +109

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this help reduce the query count? The reason I ask is because on the commit that this was added in, the query counts in the tests did not change, so I was just wondering what effect this may have had.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query counts dropped here when this branch was used: openedx/openedx-platform@016f2a5

return instance.tag_set.count()

def get_can_tag_object(self, instance) -> bool | None:
Expand Down
4 changes: 3 additions & 1 deletion openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ def get_queryset(self) -> models.QuerySet:
query_params.is_valid(raise_exception=True)
enabled = query_params.data.get("enabled", None)

return get_taxonomies(enabled)
qs = get_taxonomies(enabled)
qs = qs.annotate(tags_count=models.Count("tag", distinct=True))
return qs

def perform_create(self, serializer) -> None:
"""
Expand Down
4 changes: 2 additions & 2 deletions tests/openedx_tagging/core/tagging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def test_list_taxonomy_query_count(self):
url = TAXONOMY_LIST_URL

self.client.force_authenticate(user=self.user)
with self.assertNumQueries(5):
with self.assertNumQueries(3):
response = self.client.get(url)

assert response.status_code == 200
Expand Down Expand Up @@ -1461,7 +1461,7 @@ def test_small_query_count(self):
url = f"{self.small_taxonomy_url}?search_term=eU"

self.client.force_authenticate(user=self.staff)
with self.assertNumQueries(5):
with self.assertNumQueries(4):
response = self.client.get(url)

assert response.status_code == status.HTTP_200_OK
Expand Down