On Monday 13 February 2006 19:44, Luke Plant wrote: > The attached patch does so - the .all() call is now optional on > related objects (it's still there due to inheritance, but I've fixed > it so it does the right thing).
I listened to the code a bit more carefully, and couldn't let this lie - the attached patch is a much cleaner implementation than the first, and doesn't allow .all() on the related objects, since it no longer inherits from Manager. As before, it's probably still rough around the edges. All tests still passing though. Luke -- Life is complex. It has both real and imaginary components. Luke Plant || L.Plant.98 (at) cantab.net || http://lukeplant.me.uk/
Index: django/db/models/base.py =================================================================== --- django/db/models/base.py (revision 2306) +++ django/db/models/base.py (working copy) @@ -209,7 +209,7 @@ else: sub_obj.__collect_sub_objects(seen_objs) else: - for sub_obj in getattr(self, rel_opts_name).all(): + for sub_obj in getattr(self, rel_opts_name): sub_obj.__collect_sub_objects(seen_objs) def delete(self): Index: django/db/models/fields/related.py =================================================================== --- django/db/models/fields/related.py (revision 2306) +++ django/db/models/fields/related.py (working copy) @@ -7,6 +7,7 @@ from django.core import validators from django import forms from django.dispatch import dispatcher +from django.db.models.query import QuerySet, Q # For Python 2.3 if not hasattr(__builtins__, 'set'): @@ -94,7 +95,7 @@ setattr(instance, cache_name, rel_obj) return rel_obj -def _add_m2m_items(rel_manager_inst, managerclass, rel_model, join_table, this_col_name, +def _add_m2m_items(rel_manager, rel_model, join_table, this_col_name, rel_col_name, this_pk_val, *objs, **kwargs): # Utility function used by the ManyRelatedObjectsDescriptors # to do addition to a many-to-many field. @@ -112,7 +113,7 @@ # Create the related object. if kwargs: assert len(objs) == 0, "add() can't be passed both positional and keyword arguments" - objs = [managerclass.add(rel_manager_inst, **kwargs)] + objs = [rel_manager.add(**kwargs)] else: assert len(objs) > 0, "add() must be passed either positional or keyword arguments" for obj in objs: @@ -203,47 +204,53 @@ this_col_name = this_opts.object_name.lower() + '_id' rel_col_name = rel_opts.object_name.lower() + '_id' - # Dynamically create a class that subclasses the related - # model's default manager. - superclass = self.related.model._default_manager.__class__ + # Dynamically create a class that subclasses QuerySet + rel_manager = self.related.model._default_manager - class RelatedManager(superclass): - def get_query_set(self): - return superclass.get_query_set(self).filter(**(self.core_filters)) + class RelatedManager(QuerySet): if rel_type == "o2m": def add(self, **kwargs): kwargs.update({rel_field.name: instance}) - return superclass.add(self, **kwargs) + self._result_cache = None + return rel_manager.add(**kwargs) else: def add(self, *objs, **kwargs): - _add_m2m_items(self, superclass, rel_model, join_table, this_col_name, + self._result_cache = None + _add_m2m_items(rel_manager, rel_model, join_table, this_col_name, rel_col_name, instance._get_pk_val(), *objs, **kwargs) add.alters_data = True if rel_type == "o2m": def remove(self, *objs): + self._result_cache = None pass # TODO else: def remove(self, *objs): + self._result_cache = None _remove_m2m_items(rel_model, join_table, this_col_name, rel_col_name, instance._get_pk_val(), *objs) remove.alters_data = True if rel_type == "o2m": def clear(self): + self._result_cache = None pass # TODO else: def clear(self): + self._result_cache = None _clear_m2m_items(join_table, this_col_name, instance._get_pk_val()) clear.alters_data = True + if self.rel_type == 'o2m': + core_filters = {'%s__%s__exact' % (rel_field.name, rel_field.rel.to._meta.pk.name): getattr(instance, rel_field.rel.get_related_field().attname)} + else: + core_filters = {'%s__%s__exact' % (rel_field.name, instance_type._meta.pk.name): instance._get_pk_val()} + + def __init__(self, model=None): + QuerySet.__init__(self, model) + self._filters = Q(**self.core_filters) + manager = RelatedManager() - - if self.rel_type == 'o2m': - manager.core_filters = {'%s__%s__exact' % (rel_field.name, rel_field.rel.to._meta.pk.name): getattr(instance, rel_field.rel.get_related_field().attname)} - else: - manager.core_filters = {'%s__%s__exact' % (rel_field.name, instance_type._meta.pk.name): instance._get_pk_val()} - manager.model = self.related.model return manager @@ -273,29 +280,32 @@ # Dynamically create a class that subclasses the related # model's default manager. - superclass = self.rel_model._default_manager.__class__ + rel_manager = self.rel_model._default_manager - class RelatedManager(superclass): - def get_query_set(self): - return superclass.get_query_set(self).extra( - tables=(join_table,), - where=[ + class RelatedManager(QuerySet): + def __init__(self, model=None): + QuerySet.__init__(self, model) + self._tables = (join_table,) + self._where=[ '%s.%s = %s.%s' % (qn(rel_opts.db_table), qn(rel_opts.pk.column), join_table, rel_col_name), '%s.%s = %%s' % (join_table, this_col_name) - ], - params = [instance._get_pk_val()] - ) + ] + self._params = [instance._get_pk_val()] + def add(self, *objs, **kwargs): - _add_m2m_items(self, superclass, rel_model, join_table, this_col_name, + self._result_cache = None + _add_m2m_items(rel_manager, rel_model, join_table, this_col_name, rel_col_name, instance._get_pk_val(), *objs, **kwargs) add.alters_data = True def remove(self, *objs): + self._result_cache = None _remove_m2m_items(rel_model, join_table, this_col_name, rel_col_name, instance._get_pk_val(), *objs) remove.alters_data = True def clear(self): + self._result_cache = None _clear_m2m_items(join_table, this_col_name, instance._get_pk_val()) clear.alters_data = True @@ -489,7 +499,7 @@ def flatten_data(self, follow, obj = None): new_data = {} if obj: - instance_ids = [instance._get_pk_val() for instance in getattr(obj, self.name).all()] + instance_ids = [instance._get_pk_val() for instance in getattr(obj, self.name)] if self.rel.raw_id_admin: new_data[self.name] = ",".join([str(id) for id in instance_ids]) else: Index: django/contrib/auth/models.py =================================================================== --- django/contrib/auth/models.py (revision 2306) +++ django/contrib/auth/models.py (working copy) @@ -156,7 +156,7 @@ def get_all_permissions(self): if not hasattr(self, '_perm_cache'): import sets - self._perm_cache = sets.Set(["%s.%s" % (p.package_id, p.codename) for p in self.user_permissions.all()]) + self._perm_cache = sets.Set(["%s.%s" % (p.package_id, p.codename) for p in self.user_permissions]) self._perm_cache.update(self.get_group_permissions()) return self._perm_cache @@ -183,7 +183,7 @@ def get_and_delete_messages(self): messages = [] - for m in self.message_set.all(): + for m in self.message_set: messages.append(m.message) m.delete() return messages Index: django/contrib/admin/views/main.py =================================================================== --- django/contrib/admin/views/main.py (revision 2306) +++ django/contrib/admin/views/main.py (working copy) @@ -387,7 +387,7 @@ _get_deleted_objects(deleted_objects, perms_needed, user, sub_obj, related.opts, current_depth+2) else: has_related_objs = False - for sub_obj in getattr(obj, rel_opts_name).all(): + for sub_obj in getattr(obj, rel_opts_name): has_related_objs = True if related.field.rel.edit_inline or not related.opts.admin: # Don't display link to edit, because it either has no @@ -410,7 +410,7 @@ opts_seen.append(related.opts) rel_opts_name = related.get_accessor_name() has_related_objs = False - for sub_obj in getattr(obj, rel_opts_name).all(): + for sub_obj in getattr(obj, rel_opts_name): has_related_objs = True if related.field.rel.edit_inline or not related.opts.admin: # Don't display link to edit, because it either has no Index: tests/modeltests/m2o_recursive/models.py =================================================================== --- tests/modeltests/m2o_recursive/models.py (revision 2306) +++ tests/modeltests/m2o_recursive/models.py (working copy) @@ -26,7 +26,7 @@ >>> c = Category(id=None, name='Child category', parent=r) >>> c.save() ->>> r.child_set.all() +>>> r.child_set [Child category] >>> r.child_set.get(name__startswith='Child') Child category @@ -35,7 +35,7 @@ ... DoesNotExist ->>> c.child_set.all() +>>> c.child_set [] >>> c.parent Root category Index: tests/modeltests/many_to_many/models.py =================================================================== --- tests/modeltests/many_to_many/models.py (revision 2306) +++ tests/modeltests/many_to_many/models.py (working copy) @@ -51,17 +51,17 @@ >>> a2.publications.add(title='Highlights for Children') # Article objects have access to their related Publication objects. ->>> a1.publications.all() +>>> a1.publications [The Python Journal] ->>> a2.publications.all() +>>> a2.publications [The Python Journal, Science News, Science Weekly, Highlights for Children] # Publication objects have access to their related Article objects. ->>> p2.article_set.all() +>>> p2.article_set [NASA uses Python] >>> p1.article_set.order_by('headline') [Django lets you build Web apps easily, NASA uses Python] ->>> Publication.objects.get(id=4).article_set.all() +>>> Publication.objects.get(id=4).article_set [NASA uses Python] # We can perform kwarg queries across m2m relationships @@ -97,7 +97,7 @@ >>> Publication.objects.all() [Science News, Science Weekly, Highlights for Children] >>> a1 = Article.objects.get(pk=1) ->>> a1.publications.all() +>>> a1.publications [] # If we delete an Article, its Publications won't be able to access it. @@ -111,31 +111,31 @@ >>> a4 = Article(headline='NASA finds intelligent life on Earth') >>> a4.save() >>> p2.article_set.add(a4) ->>> p2.article_set.all() +>>> p2.article_set [NASA finds intelligent life on Earth] ->>> a4.publications.all() +>>> a4.publications [Science News] # Adding via the other end using keywords >>> p2.article_set.add(headline='Oxygen-free diet works wonders') ->>> p2.article_set.all().order_by('headline') +>>> p2.article_set.order_by('headline') [NASA finds intelligent life on Earth, Oxygen-free diet works wonders] ->>> a5 = p2.article_set.all().order_by('headline')[1] ->>> a5.publications.all() +>>> a5 = p2.article_set.order_by('headline')[1] +>>> a5.publications [Science News] # Removing publication from an article: >>> a4.publications.remove(p2) ->>> p2.article_set.all().order_by('headline') +>>> p2.article_set.order_by('headline') [Oxygen-free diet works wonders] ->>> a4.publications.all() +>>> a4.publications [] # And from the other end >>> p2.article_set.remove(a5) >>> p2.article_set.order_by('headline') [] ->>> a5.publications.all() +>>> a5.publications [] # You can clear the whole lot: @@ -145,21 +145,21 @@ >>> a4.publications.order_by('title') [Science News, Science Weekly] >>> p2.article_set.clear() ->>> p2.article_set.all() +>>> p2.article_set [] ->>> a4.publications.all() +>>> a4.publications [Science Weekly] # And you can clear from the other end >>> p2.article_set.add(a4, a5) ->>> p2.article_set.all().order_by('headline') +>>> p2.article_set.order_by('headline') [NASA finds intelligent life on Earth, Oxygen-free diet works wonders] >>> a4.publications.order_by('title') [Science News, Science Weekly] >>> a4.publications.clear() ->>> a4.publications.all() +>>> a4.publications [] ->>> p2.article_set.all().order_by('headline') +>>> p2.article_set.order_by('headline') [Oxygen-free diet works wonders] Index: tests/modeltests/custom_pk/models.py =================================================================== --- tests/modeltests/custom_pk/models.py (revision 2306) +++ tests/modeltests/custom_pk/models.py (working copy) @@ -62,9 +62,9 @@ >>> b = Business(name='Sears') >>> b.save() >>> b.employees.add(dan, fran) ->>> b.employees.all() +>>> b.employees [Dan Jones, Fran Jones] ->>> fran.business_set.all() +>>> fran.business_set [Sears] >>> Business.objects.in_bulk(['Sears']) {'Sears': Sears} Index: tests/modeltests/m2o_recursive2/models.py =================================================================== --- tests/modeltests/m2o_recursive2/models.py (revision 2306) +++ tests/modeltests/m2o_recursive2/models.py (working copy) @@ -32,12 +32,12 @@ Jane Smith >>> kid.father John Smith Senior ->>> dad.fathers_child_set.all() +>>> dad.fathers_child_set [John Smith Junior] ->>> mom.mothers_child_set.all() +>>> mom.mothers_child_set [John Smith Junior] ->>> kid.mothers_child_set.all() +>>> kid.mothers_child_set [] ->>> kid.fathers_child_set.all() +>>> kid.fathers_child_set [] """ Index: tests/modeltests/m2m_multiple/models.py =================================================================== --- tests/modeltests/m2m_multiple/models.py (revision 2306) +++ tests/modeltests/m2m_multiple/models.py (working copy) @@ -50,30 +50,30 @@ >>> a2.primary_categories.add(c1, c2) >>> a2.secondary_categories.add(c4) ->>> a1.primary_categories.all() +>>> a1.primary_categories [Crime, News] ->>> a2.primary_categories.all() +>>> a2.primary_categories [News, Sports] ->>> a1.secondary_categories.all() +>>> a1.secondary_categories [Life] ->>> c1.primary_article_set.all() +>>> c1.primary_article_set [Area man runs] ->>> c1.secondary_article_set.all() +>>> c1.secondary_article_set [] ->>> c2.primary_article_set.all() +>>> c2.primary_article_set [Area man steals, Area man runs] ->>> c2.secondary_article_set.all() +>>> c2.secondary_article_set [] ->>> c3.primary_article_set.all() +>>> c3.primary_article_set [Area man steals] ->>> c3.secondary_article_set.all() +>>> c3.secondary_article_set [] ->>> c4.primary_article_set.all() +>>> c4.primary_article_set [] ->>> c4.secondary_article_set.all() +>>> c4.secondary_article_set [Area man steals, Area man runs] """ Index: tests/modeltests/m2m_intermediary/models.py =================================================================== --- tests/modeltests/m2m_intermediary/models.py (revision 2306) +++ tests/modeltests/m2m_intermediary/models.py (working copy) @@ -63,6 +63,6 @@ This is a test >>> w2.article This is a test ->>> r1.writer_set.all() +>>> r1.writer_set [John Smith (Main writer)] """