django-seal
django-seal copied to clipboard
Errors are not raised on prefetched table with different queries
Hello,
I notice that the library is not raising the UnsealedAttributeAccess
errors if you try to access a prefetched table with a different query. Take this model for example
class Company(SealableModel):
users = models.ManyToManyField(settings.AUTH_USER_MODEL, blank=True)
If I try to access the company from the User it raises the error as expected:
User.objects.all().seal().first().company_set.all()
UnsealedAttributeAccess: Attempt to fetch many-to-many field "company" on sealed <User instance>.
Now, if I prefetch the company_set
the error disappears as expected:
for u in User.objects.prefetch_related('company_set').all().seal():
for c in u.company_set.all():
print(c) # no errors and 2 queries
The problem is that django-seal cannot detect if the user changes the queryset:
for u in User.objects.prefetch_related('company_set').all().seal():
for c in u.company_set.order_by('-pk'):
print(c) # no errors and 4 queries
This is currently done by design. As soon as a queryset clone is created due a queryset operation the queryset is unsealed
https://github.com/charettes/django-seal/blob/75c1697af996f29dd9e3f9571de18efc5436b15f/seal/descriptors.py#L31-L40
I guess we could emit a different kind of warning when this happens (e.g. RelatedQuerysetUnsealed
) so users could decide how they want to deal with this scenario?
Hi @charettes
I am not sure if it's related, but I was able to "bypass" the seal even without the prefetch:
This one raises the error:
[u.company_set.all() for u in User.objects.all().seal()]
This one doesn't
[u.company_set.order_by('-pk') for u in User.objects.all().seal()]
Personally I would prefer that those cases were at least logged by the library. My use case is that I have a DRF endpoint that displays a nested structure with 3 models deep. I want to make sure that the sealed queryset defined at the view is enough for the whole endpoint, but right now if someone changes any reverse lookup it would invalidate the prefetch related silently.
@canassa I spent some time drafting a solution in #66. Would that solve your usecase?
@charettes Yes! I think so!
I got a question.
When will the warning be emitted exactly? Is it issued when the pretech call is executed or when an unwanted extra query is executed?