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)]
 """

Reply via email to