Opened 16 years ago
Closed 16 years ago
#7904 closed (fixed)
Manager 'get' overloading now fails / Accessing single related (parent) objects should not bypass manager
Reported by: | magneto | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Keywords: | ||
Cc: | Maniac@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The fix to Issue #7666 caused Manage overloading for "get" to fail (rather it now fails to use the overloaded method)
from django.db import models class M1Manager(models.Manager): def get(self, *args, **kwargs): """ over load 'get' to allow for more fined tuned actions """ #in this trivial example, all gets return none return None class M1(models.Model): col1 = models.CharField(max_length = 30) objects = M1Manager() __unicode__(self): return col1 class M2(models.Model): m1 = models.ForeignKey(M1) col2 = models.CharField() my_m1 = M1.objects.create(col1="m1_obj") my_m2 = M2.objects.create(m1 = my_m1, col2="my_m2") find_m2 = M2.objects.get(col2 = "my_m2") #prints "m1_obj" and should print None print find_m2.m1
Attachments (2)
Change History (8)
by , 16 years ago
Attachment: | use_for_related_fields.diff added |
---|
comment:1 by , 16 years ago
I take it that little option would go into the Manager
class M1Manager(models.Manager): use_for_related_fields = True def get(self, *args, **kwargs): """ over load 'get' to allow for more fined tuned actions """ #in this trivial example, all gets return none return None
in any case, that does seem to work, but there's still something fishy about the fact that normal object inheritance does not seem to be obeyed
comment:2 by , 16 years ago
Summary: | Re: #7666 -- Manager 'get' overloading now failes → Re: #7666 -- Manager 'get' overloading now fails |
---|
comment:3 by , 16 years ago
Summary: | Re: #7666 -- Manager 'get' overloading now fails → Manager 'get' overloading now fails / Accessing single related (parent) objects should not bypass manager |
---|
comment:4 by , 16 years ago
Ivan opened up #8119 which I closed as a duplicate of this ticket. He had another approach, and I moved the patch and comments to this ticket:
This is a spun-of from http://code.djangoproject.com/ticket/7666#comment:7 with the relevant discussion here: http://groups.google.com/group/django-developers/browse_frm/thread/a2326c32f8860fb0/
The solution is to have a separate queryset factory on a manager get_full_query_set that can be overridden but is required to return all records. Old get_query_set normally just calls get_full_query_set and can be overridden in a usual way. This part is backwards compatible. Also managers acquire a new method get_full which works like get but explicitly calls get_full_query_set. This method is used to access single related (parent) objects. This will require users who was overriding get to override get_full instead if they wish to ensure that default filtering doesn't get in the way. This is as backwards incompatible as was after #7666 that has broken overriding of get but provides a new API to overcome this.
by , 16 years ago
Attachment: | get_full_query_set.diff added |
---|
Patch from Ivan Sagalaev that offers a different approach.
comment:5 by , 16 years ago
Cc: | added |
---|---|
Has patch: | set |
Incidentally, I was also thinking about setting some state to a Manager before calling get. But I've decided against it because:
- when the flag is set permanently to True it binds
get
to behave only in a single fashion, but the point of #7666 is that it should have different behavior depending on if it is called for parent relation or directly withModel.objects.get
- switching the flag dynamically creates a possibility to forget to do it and, worse, introduces a transient state into process which will break things if
Manager.get
calls other functions that also indirectly callManager.get
counting on different state - a stack of states might fix previous issue but this solution is really overcomplicated and ugly
This is why I've just made an explicit full_get
that can be called separately from get
. And two queryset factories are needed because queryset creation in general may be customized in many different ways but filtering is the only type of customization that we care about when accessing parent objects. Thus splitting it into a separate point.
comment:6 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch that looks for use_for_related_fields attribute on managers.