Skip to content

Commit

Permalink
Fix slow annotation value query
Browse files Browse the repository at this point in the history
  • Loading branch information
gregorjerse committed Nov 21, 2024
1 parent 475b64b commit 4229a4e
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 11 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ This project adheres to `Semantic Versioning <http://semver.org/>`_.
Unreleased
==========

Fixed
-----
- Filter out deleted annotation values in viewset

Changed
-------
- **BACKWARD INCOMPATIBLE:** Return all version of processess on process
Expand Down
13 changes: 8 additions & 5 deletions resolwe/flow/models/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,14 @@ class AnnotationValueManager(BaseManager["AnnotationValue", PermissionQuerySet])
version_field = "created"
QuerySet = PermissionQuerySet

def get_queryset(self) -> PermissionQuerySet:
"""Return the latest version for every value."""
queryset = super().get_queryset()
# The old value is deleted so make sure it is never returned.
return queryset.filter(pk__in=queryset).exclude(_value__isnull=True)
def remove_delete_markers(self) -> PermissionQuerySet:
"""Remote delete markers from the queryset.
Due to the nature of the queryset use the method only on small querysets.
"""
return self.filter(pk__in=list(self.values_list("pk", flat=True))).exclude(
_value__isnull=True
)


def _slug_for_annotation_value(instance: "AnnotationValue") -> str:
Expand Down
10 changes: 5 additions & 5 deletions resolwe/flow/tests/test_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,13 +692,13 @@ def test_update_annotation_value(self):
entity=created_value.entity, field=created_value.field
)
expected = AnnotationValueSerializer([updated_value], many=True).data
print("Got response", response.data)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertCountEqual(response.data, expected)
with self.assertRaises(AnnotationValue.DoesNotExist):
AnnotationValue.objects.get(
entity=self.annotation_value1.entity, field=self.annotation_value1.field
)
AnnotationValue.objects.filter(
entity=self.annotation_value1.entity,
field=self.annotation_value1.field,
).remove_delete_markers().get()
created_value = AnnotationValue.objects.get(
entity=created_value.entity, field=created_value.field
)
Expand Down Expand Up @@ -1451,7 +1451,7 @@ def has_value(entity, field_id, value):
force_authenticate(request, self.contributor)
response = viewset(request, pk=self.entity1.pk)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(self.entity1.annotations.count(), 1)
self.assertEqual(self.entity1.annotations.remove_delete_markers().count(), 1)
has_value(self.entity1, self.annotation_field2.pk, 2)

# Re-create deleted annotation value.
Expand Down
9 changes: 9 additions & 0 deletions resolwe/flow/views/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ class AnnotationValueViewSet(
permission_classes = (get_permissions_class(),)
ordering_fields = ("created", "id")

def list(self, request, *args, **kwargs):
"""List objects."""
queryset = self.filter_queryset(self.get_queryset())
# Remove the delete markers from the result.
serializer = self.get_serializer(
(entry for entry in queryset if entry.value is not None), many=True
)
return response.Response(serializer.data)

def get_serializer(self, *args: Any, **kwargs: Any) -> BaseSerializer:
"""Get serializer instance depending on the request type."""
kwargs_many = kwargs.get("many", False)
Expand Down
9 changes: 9 additions & 0 deletions resolwe/permissions/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,15 @@ def _check_permissions(self):
f"Permissions are not supported on model `{self.model._meta.label}`."
)

def remove_delete_markers(self) -> "PermissionQuerySet":
"""Remote delete markers from the queryset.
Due to the nature of the queryset use the method only on small querysets.
"""
return self.filter(pk__in=list(self.values_list("pk", flat=True))).exclude(
_value__isnull=True
)

def _filter_by_permission(
self,
user: Optional[User],
Expand Down
2 changes: 1 addition & 1 deletion resolwe/process/tests/test_python_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def test_annotation_v2(self):
# Try bulk setting.
data = self.run_process("test-python-process-annotate-entity-v2-bulk-set")
self.assertIsNotNone(data.entity)
self.assertEqual(data.entity.annotations.count(), 1)
self.assertEqual(data.entity.annotations.remove_delete_markers().count(), 1)
self.assertAnnotation(data.entity, "general.species", "Human Bulk Set")

# Now try reading and updating existing annotations.
Expand Down

0 comments on commit 4229a4e

Please sign in to comment.