Ticket #11670: 11670.field-lookup-collisions.3.diff

django/contrib/admin/options.py

@@ -233 +233 @@
                 if k == lookup and v == value:
                     return True
 
-        parts = lookup.split(LOOKUP_SEP)
+        parts, fields, _ = model._meta.resolve_lookup_path(lookup)
+        num_parts = len(parts)
+        num_fields = len(fields)
 
-        # Last term in lookup is a query term (__exact, __startswith etc)
-        # This term can be ignored.
-        if len(parts) > 1 and parts[-1] in QUERY_TERMS:
+        if not num_fields:
+            # Lookups on non-existants fields are ok, since they're ignored
+            # later.
+            return True
+
+        elif num_parts > 1 and parts[-1] in QUERY_TERMS:
             parts.pop()
+            num_parts += -1
 
         # Special case -- foo__id__exact and foo__id queries are implied
         # if foo has been specificially included in the lookup list; so
         # drop __id if it is the last part. However, first we need to find
         # the pk attribute name.
-        pk_attr_name = None
-        for part in parts[:-1]:
-            field, _, _, _ = model._meta.get_field_by_name(part)
-            if hasattr(field, 'rel'):
-                model = field.rel.to
-                pk_attr_name = model._meta.pk.name
-            elif isinstance(field, RelatedObject):
-                model = field.model
-                pk_attr_name = model._meta.pk.name
-            else:
-                pk_attr_name = None
-        if pk_attr_name and len(parts) > 1 and parts[-1] == pk_attr_name:
+        if (num_fields == num_parts and
+            parts[-1] == fields[-1].model._meta.pk.name):
             parts.pop()
+            num_parts += -1
 
-        try:
-            self.model._meta.get_field_by_name(parts[0])
-        except FieldDoesNotExist:
-            # Lookups on non-existants fields are ok, since they're ignored
-            # later.
+        if num_parts == 1:
             return True
-        else:
-            if len(parts) == 1:
-                return True
-            clean_lookup = LOOKUP_SEP.join(parts)
-            return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
+
+        clean_lookup = LOOKUP_SEP.join(parts)
+        return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
 
     def has_add_permission(self, request):
         """

django/contrib/admin/util.py

@@ -1 +1 @@
 from django.db import models
 from django.db.models.sql.constants import LOOKUP_SEP
 from django.db.models.deletion import Collector
-from django.db.models.related import RelatedObject
+from django.db.models.related import RelatedObject, get_model_from_relation, NotRelationField
 from django.forms.forms import pretty_name
 from django.utils import formats
 from django.utils.html import escape
@@ -302 +302 @@
     else:
         return smart_unicode(value)
 
-
-class NotRelationField(Exception):
-    pass
-
-
-def get_model_from_relation(field):
-    if isinstance(field, models.related.RelatedObject):
-        return field.model
-    elif getattr(field, 'rel'): # or isinstance?
-        return field.rel.to
-    else:
-        raise NotRelationField
-
-
 def reverse_field_path(model, path):
     """ Create a reversed field path.
 
@@ -346 +332 @@
     return (parent, LOOKUP_SEP.join(reversed_path))
 
 
-def get_fields_from_path(model, path):
-    """ Return list of Fields given path relative to model.
-
-    e.g. (ModelX, "user__groups__name") -> [
-        <django.db.models.fields.related.ForeignKey object at 0x...>,
-        <django.db.models.fields.related.ManyToManyField object at 0x...>,
-        <django.db.models.fields.CharField object at 0x...>,
-    ]
-    """
-    pieces = path.split(LOOKUP_SEP)
-    fields = []
-    for piece in pieces:
-        if fields:
-            parent = get_model_from_relation(fields[-1])
-        else:
-            parent = model
-        fields.append(parent._meta.get_field_by_name(piece)[0])
-    return fields
-
-
 def remove_trailing_data_field(fields):
-    """ Discard trailing non-relation field if extant. """
+    """ Discard trailing non-relation field if existant. """
     try:
         get_model_from_relation(fields[-1])
     except NotRelationField:
@@ -381 +347 @@
     If final model in path is linked via a ForeignKey or ManyToManyField which
     has a `limit_choices_to` attribute, return it as a Q object.
     """
-
-    fields = get_fields_from_path(model, path)
+    _, fields, _ = model._meta.resolve_lookup_path(path)
     fields = remove_trailing_data_field(fields)
     limit_choices_to = (
         fields and hasattr(fields[-1], 'rel') and

django/contrib/admin/validation.py

@@ -4 +4 @@
 from django.forms.models import (BaseModelForm, BaseModelFormSet, fields_for_model,
     _get_foreign_key)
 from django.contrib.admin import ListFilter, FieldListFilter
-from django.contrib.admin.util import get_fields_from_path, NotRelationField
 from django.contrib.admin.options import (flatten_fieldsets, BaseModelAdmin,
     HORIZONTAL, VERTICAL)
 
@@ -84 +83 @@
                     # item is option #1
                     field = item
                 # Validate the field string
-                try:
-                    get_fields_from_path(model, field)
-                except (NotRelationField, FieldDoesNotExist):
+                _, _, last_field = model._meta.resolve_lookup_path(field)
+                if not last_field:
                     raise ImproperlyConfigured("'%s.list_filter[%d]' refers to '%s'"
                             " which does not refer to a Field."
                             % (cls.__name__, idx, field))

django/contrib/admin/views/main.py

@@ -10 +10 @@
 
 from django.contrib.admin import FieldListFilter
 from django.contrib.admin.options import IncorrectLookupParameters
-from django.contrib.admin.util import quote, get_fields_from_path
+from django.contrib.admin.util import quote
 
 # Changelist settings
 ALL_VAR = 'all'
@@ -105 +105 @@
                         field, field_list_filter_class = list_filter, FieldListFilter.create
                     if not isinstance(field, models.Field):
                         field_path = field
-                        field = get_fields_from_path(self.model, field_path)[-1]
+                        _, _, field = self.model._meta.resolve_lookup_path(field_path)
                     spec = field_list_filter_class(field, request, cleaned_params,
                         self.model, self.model_admin, field_path=field_path)
                 if spec and spec.has_output():

django/contrib/gis/db/models/sql/where.py

@@ -58 +58 @@
         'address__point'.
 
         If a GeometryField exists according to the given lookup on the model
-        options, it will be returned.  Otherwise returns None.
+        options, it will be returned.  Otherwise returns False.
         """
-        # This takes into account the situation where the lookup is a
-        # lookup to a related geographic field, e.g., 'address__point'.
-        field_list = lookup.split(LOOKUP_SEP)
-
-        # Reversing so list operates like a queue of related lookups,
-        # and popping the top lookup.
-        field_list.reverse()
-        fld_name = field_list.pop()
-
-        try:
-            geo_fld = opts.get_field(fld_name)
-            # If the field list is still around, then it means that the
-            # lookup was for a geometry field across a relationship --
-            # thus we keep on getting the related model options and the
-            # model field associated with the next field in the list
-            # until there's no more left.
-            while len(field_list):
-                opts = geo_fld.rel.to._meta
-                geo_fld = opts.get_field(field_list.pop())
-        except (FieldDoesNotExist, AttributeError):
-            return False
-
-        # Finally, make sure we got a Geographic field and return.
-        if isinstance(geo_fld, GeometryField):
-            return geo_fld
+        _, _, field = opts.resolve_lookup_path(lookup)
+        if field and isinstance(field, GeometryField):
+            return field
         else:
             return False

django/db/models/options.py

@@ +1 @@
+from copy import copy
 import re
 from bisect import bisect
 
+from django.core.exceptions import FieldError
+from django.db.models.sql.constants import LOOKUP_SEP
 from django.conf import settings
-from django.db.models.related import RelatedObject
+from django.db.models.related import RelatedObject, get_model_from_relation, NotRelationField
 from django.db.models.fields.related import ManyToManyRel
 from django.db.models.fields import AutoField, FieldDoesNotExist
 from django.db.models.fields.proxy import OrderWrt
@@ -54 +57 @@
         # from *other* models. Needed for some admin checks. Internal use only.
         self.related_fkey_lookups = []
 
+        self._resolved_lookup_path_cache = {}
+
     def contribute_to_class(self, cls, name):
         from django.db import connection
         from django.db.backends.util import truncate_name
@@ -495 +500 @@
         Returns the index of the primary key field in the self.fields list.
         """
         return self.fields.index(self.pk)
+
+    def resolve_lookup_path(self, path):
+        """
+        Resolves the given lookup path and returns a tuple (parts, fields,
+        last_field) where parts is the lookup path split by LOOKUP_SEP,
+        fields is the list of fields that have been resolved by traversing the
+        relations, and last_field is the last field if the relation traversal
+        reached the end of the lookup or None if it didn't.
+        """
+        # Look in the cache first.
+        if path in self._resolved_lookup_path_cache:
+            # Return a copy of parts and fields, as they may be modified later
+            # by the calling code.
+            cached = self._resolved_lookup_path_cache[path]
+            return copy(cached[0]), copy(cached[1]), cached[2]
+
+        parts = path.split(LOOKUP_SEP)
+        if not parts:
+            raise FieldError("Cannot parse lookup path %r" % path)
+
+        num_parts = len(parts)
+
+        fields = []
+        last_field = None
+        if num_parts == 1:
+            try:
+                # Let's see if the only one lookup provided is a field
+                last_field, _, _, _ = self.get_field_by_name(path)
+                fields.append(last_field)
+            except FieldDoesNotExist:
+                # Not a field, let's move on.
+                pass
+        else:
+            # Traverse the lookup query to distinguish related fields from
+            # lookup types.
+            try:
+                opts = self
+                for counter, field_name in enumerate(parts):
+                    lookup_field, _, _, _ = opts.get_field_by_name(field_name)
+                    fields.append(lookup_field)
+                    if (counter + 1) < num_parts:
+                        # Unless we haven't reached the end of the list of
+                        # lookups yet, then let's attempt to continue
+                        # traversing relations.
+                        related_model = get_model_from_relation(lookup_field)
+                        opts = related_model._meta
+                # We have reached the end of the query and the last lookup
+                # is a field.
+                last_field = fields[-1]
+            except (FieldDoesNotExist, NotRelationField):
+                # The traversing didn't reach the end because at least one of
+                # the lookups wasn't a field.
+                pass
+        self._resolved_lookup_path_cache[path] = (
+            parts, fields, last_field)
+        # Return a copy of parts and fields, as they may be modified later by
+        # the calling code.
+        return copy(parts), copy(fields), last_field

django/db/models/related.py

@@ -36 +36 @@
                 {'%s__isnull' % self.parent_model._meta.module_name: False})
         lst = [(x._get_pk_val(), smart_unicode(x)) for x in queryset]
         return first_choice + lst
-        
+
     def get_db_prep_lookup(self, lookup_type, value, connection, prepared=False):
         # Defer to the actual field definition for db prep
         return self.field.get_db_prep_lookup(lookup_type, value,
@@ -67 +67 @@
 
     def get_cache_name(self):
         return "_%s_cache" % self.get_accessor_name()
+
+class NotRelationField(Exception):
+    pass
+
+def get_model_from_relation(field):
+    if isinstance(field, RelatedObject):
+        return field.model
+    elif getattr(field, 'rel'): # or isinstance?
+        return field.rel.to
+    else:
+        raise NotRelationField
+ No newline at end of file

django/db/models/sql/query.py

@@ -1041 +1041 @@
         during the processing of extra filters to avoid infinite recursion.
         """
         arg, value = filter_expr
-        parts = arg.split(LOOKUP_SEP)
-        if not parts:
-            raise FieldError("Cannot parse keyword query %r" % arg)
 
-        # Work out the lookup type and remove it from 'parts', if necessary.
-        if len(parts) == 1 or parts[-1] not in self.query_terms:
-            lookup_type = 'exact'
-        else:
+        parts, _, last_field = self.model._meta.resolve_lookup_path(arg)
+
+        # Work out the lookup type and remove it from the end of 'parts',
+        # if necessary.
+        if not last_field and len(parts) and parts[-1] in self.query_terms:
             lookup_type = parts.pop()
+        else:
+            lookup_type = 'exact'
 
         # By default, this is a WHERE clause. If an aggregate is referenced
         # in the value, the filter will be promoted to a HAVING

tests/modeltests/lookup/models.py

@@ -26 +26 @@
     name = models.CharField(max_length=100)
     class Meta:
         ordering = ('name', )
+
+class Season(models.Model):
+    year = models.PositiveSmallIntegerField()
+    gt = models.IntegerField(null=True, blank=True)
+
+    def __unicode__(self):
+        return unicode(self.year)
+
+class Game(models.Model):
+    season = models.ForeignKey(Season, related_name='games')
+    home = models.CharField(max_length=100)
+    away = models.CharField(max_length=100)
+
+    def __unicode__(self):
+        return u"%s at %s" % (self.away, self.home)
+
+class Player(models.Model):
+    name = models.CharField(max_length=100)
+    games = models.ManyToManyField(Game, related_name='players')
+
+    def __unicode__(self):
+        return self.name
+ No newline at end of file

tests/modeltests/lookup/tests.py

@@ -1 +1 @@
 from datetime import datetime
 from operator import attrgetter
+
 from django.core.exceptions import FieldError
 from django.test import TestCase, skipUnlessDBFeature
-from models import Author, Article, Tag
+
+from models import Author, Article, Tag, Game, Season, Player
 
 
 class LookupTests(TestCase):
@@ -243 +245 @@
         self.assertQuerysetEqual(Article.objects.filter(id=self.a5.id).values(),
             [{
                 'id': self.a5.id,
-                'author_id': self.au2.id, 
+                'author_id': self.au2.id,
                 'headline': 'Article 5',
                 'pub_date': datetime(2005, 8, 1, 9, 0)
             }], transform=identity)
@@ -606 +608 @@
         a16.save()
         self.assertQuerysetEqual(Article.objects.filter(headline__regex=r'b(.).*b\1'),
             ['<Article: barfoobaz>', '<Article: bazbaRFOO>', '<Article: foobarbaz>'])
+
+class LookupCollisionTests(TestCase):
+
+    def setUp(self):
+        # Here we're using 'gt' as a code number for the year, e.g. 111=>2009.
+        season_2009 = Season.objects.create(year=2009, gt=111)
+        season_2009.games.create(home="Houston Astros", away="St. Louis Cardinals")
+        season_2010 = Season.objects.create(year=2010, gt=222)
+        season_2010.games.create(home="Houston Astros", away="Chicago Cubs")
+        season_2010.games.create(home="Houston Astros", away="Milwaukee Brewers")
+        season_2010.games.create(home="Houston Astros", away="St. Louis Cardinals")
+        season_2011 = Season.objects.create(year=2011, gt=333)
+        season_2011.games.create(home="Houston Astros", away="St. Louis Cardinals")
+        season_2011.games.create(home="Houston Astros", away="Milwaukee Brewers")
+        hunter_pence = Player.objects.create(name="Hunter Pence")
+        hunter_pence.games = Game.objects.filter(season__year__in=[2009, 2010])
+        pudge = Player.objects.create(name="Ivan Rodriquez")
+        pudge.games = Game.objects.filter(season__year=2009)
+        pedro_feliz = Player.objects.create(name="Pedro Feliz")
+        pedro_feliz.games = Game.objects.filter(season__year__in=[2011])
+        johnson = Player.objects.create(name="Johnson")
+        johnson.games = Game.objects.filter(season__year__in=[2011])
+
+    def test_lookup_collision(self):
+        """
+        Ensure that genuine field names don't collide with built-in lookup
+        types ('year', 'gt', 'range', 'in' etc.).
+        Refs #11670.
+        """
+        # Games in 2010
+        self.assertEqual(Game.objects.filter(season__year=2010).count(), 3)
+        self.assertEqual(Game.objects.filter(season__year__exact=2010).count(), 3)
+        self.assertEqual(Game.objects.filter(season__gt=222).count(), 3)
+        self.assertEqual(Game.objects.filter(season__gt__exact=222).count(), 3)
+
+        # Games in 2011
+        self.assertEqual(Game.objects.filter(season__year=2011).count(), 2)
+        self.assertEqual(Game.objects.filter(season__year__exact=2011).count(), 2)
+        self.assertEqual(Game.objects.filter(season__gt=333).count(), 2)
+        self.assertEqual(Game.objects.filter(season__gt__exact=333).count(), 2)
+        self.assertEqual(Game.objects.filter(season__year__gt=2010).count(), 2)
+        self.assertEqual(Game.objects.filter(season__gt__gt=222).count(), 2)
+
+        # Games played in 2010 and 2011
+        self.assertEqual(Game.objects.filter(season__year__in=[2010, 2011]).count(), 5)
+        self.assertEqual(Game.objects.filter(season__year__gt=2009).count(), 5)
+        self.assertEqual(Game.objects.filter(season__gt__in=[222, 333]).count(), 5)
+        self.assertEqual(Game.objects.filter(season__gt__gt=111).count(), 5)
+
+        # Players who played in 2009
+        self.assertEqual(Player.objects.filter(games__season__year=2009).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__year__exact=2009).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__gt=111).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__gt__exact=111).distinct().count(), 2)
+
+        # Players who played in 2010
+        self.assertEqual(Player.objects.filter(games__season__year=2010).distinct().count(), 1)
+        self.assertEqual(Player.objects.filter(games__season__year__exact=2010).distinct().count(), 1)
+        self.assertEqual(Player.objects.filter(games__season__gt=222).distinct().count(), 1)
+        self.assertEqual(Player.objects.filter(games__season__gt__exact=222).distinct().count(), 1)
+
+        # Players who played in 2011
+        self.assertEqual(Player.objects.filter(games__season__year=2011).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__year__exact=2011).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__gt=333).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__year__gt=2010).distinct().count(), 2)
+        self.assertEqual(Player.objects.filter(games__season__gt__gt=222).distinct().count(), 2)