From 829dc3c5a64d3fa203b8cc0055e83cf23addfee3 Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Wed, 20 Mar 2013 10:47:56 +0000 Subject: [PATCH] Fixed #20094 - Be more careful when checking for Iterator Python 2.6 has some different behaviour when checking isinstance(foo, collections.Iterator). --- django/db/models/fields/__init__.py | 4 ++-- django/db/models/sql/where.py | 4 ++-- django/utils/itercompat.py | 15 ++++++++++++++- tests/utils_tests/itercompat.py | 11 +++++++++++ tests/utils_tests/models.py | 14 +++++++++++++- tests/utils_tests/tests.py | 1 + 6 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 tests/utils_tests/itercompat.py diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index c210cfabf6..1f0ce5e4ed 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -import collections import copy import datetime import decimal @@ -18,6 +17,7 @@ from django.core import exceptions, validators from django.utils.datastructures import DictWrapper from django.utils.dateparse import parse_date, parse_datetime, parse_time from django.utils.functional import curry, total_ordering +from django.utils.itercompat import is_iterator from django.utils.text import capfirst from django.utils import timezone from django.utils.translation import ugettext_lazy as _ @@ -488,7 +488,7 @@ class Field(object): return bound_field_class(self, fieldmapping, original) def _get_choices(self): - if isinstance(self._choices, collections.Iterator): + if is_iterator(self._choices): choices, self._choices = tee(self._choices) return choices else: diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index 754e33292d..42682c342e 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -4,7 +4,6 @@ Code to manage the creation and SQL rendering of 'where' constraints. from __future__ import absolute_import -import collections import datetime from itertools import repeat @@ -12,6 +11,7 @@ from django.conf import settings from django.db.models.fields import DateTimeField, Field from django.db.models.sql.datastructures import EmptyResultSet, Empty from django.db.models.sql.aggregates import Aggregate +from django.utils.itercompat import is_iterator from django.utils.six.moves import xrange from django.utils import timezone from django.utils import tree @@ -58,7 +58,7 @@ class WhereNode(tree.Node): if not isinstance(data, (list, tuple)): return data obj, lookup_type, value = data - if isinstance(value, collections.Iterator): + if is_iterator(value): # Consume any generators immediately, so that we can determine # emptiness and transform any non-empty values correctly. value = list(value) diff --git a/django/utils/itercompat.py b/django/utils/itercompat.py index e7ea326536..c50dcfb779 100644 --- a/django/utils/itercompat.py +++ b/django/utils/itercompat.py @@ -4,10 +4,12 @@ Where possible, we try to use the system-native version and only fall back to these implementations if necessary. """ -from django.utils.six.moves import builtins +import collections import itertools +import sys import warnings + def is_iterable(x): "A implementation independent way of checking for iterables" try: @@ -17,6 +19,17 @@ def is_iterable(x): else: return True +def is_iterator(x): + """An implementation independent way of checking for iterators + + Python 2.6 has a different implementation of collections.Iterator which + accepts anything with a `next` method. 2.7+ requires and `__iter__` method + as well. + """ + if sys.version_info >= (2, 7): + return isinstance(x, collections.Iterator) + return isinstance(x, collections.Iterator) and hasattr(x, '__iter__') + def product(*args, **kwds): warnings.warn("django.utils.itercompat.product is deprecated; use the native version instead", DeprecationWarning, stacklevel=2) diff --git a/tests/utils_tests/itercompat.py b/tests/utils_tests/itercompat.py new file mode 100644 index 0000000000..02d3463370 --- /dev/null +++ b/tests/utils_tests/itercompat.py @@ -0,0 +1,11 @@ +from django.test import TestCase + +from .models import Category, Thing + + +class TestIsIterator(TestCase): + def test_regression(self): + """This failed on Django 1.5/Py2.6 because category has a next method.""" + category = Category.objects.create(name='category') + Thing.objects.create(category=category) + Thing.objects.filter(category=category) diff --git a/tests/utils_tests/models.py b/tests/utils_tests/models.py index 97a72bab14..de8f9e3803 100644 --- a/tests/utils_tests/models.py +++ b/tests/utils_tests/models.py @@ -1 +1,13 @@ -# Test runner needs a models.py file. +from django.db import models + + +class Category(models.Model): + name = models.CharField(max_length=100) + + def next(self): + return self + + +class Thing(models.Model): + name = models.CharField(max_length=100) + category = models.ForeignKey(Category) diff --git a/tests/utils_tests/tests.py b/tests/utils_tests/tests.py index 726df3d979..f9c6e55847 100644 --- a/tests/utils_tests/tests.py +++ b/tests/utils_tests/tests.py @@ -18,6 +18,7 @@ from .feedgenerator import FeedgeneratorTest from .functional import FunctionalTestCase from .html import TestUtilsHtml from .http import TestUtilsHttp, ETagProcessingTests, HttpDateProcessingTests +from .itercompat import TestIsIterator from .ipv6 import TestUtilsIPv6 from .jslex import JsToCForGettextTest, JsTokensTest from .module_loading import (CustomLoader, DefaultLoader, EggLoader,