From dd897e4eeb20cab9ac708c46f34c5f09758205c8 Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Wed, 20 Mar 2013 10:47:56 +0000 Subject: [PATCH] [1.5.x] Fixed #20094 - Be more careful when checking for Iterator Python 2.6 has some different behaviour when checking isinstance(foo, collections.Iterator). Backport of 829dc3c5 from master. --- django/db/models/fields/__init__.py | 4 ++-- django/db/models/sql/where.py | 4 ++-- django/utils/itercompat.py | 15 ++++++++++++++- tests/regressiontests/utils/itercompat.py | 11 +++++++++++ tests/regressiontests/utils/models.py | 14 +++++++++++++- tests/regressiontests/utils/tests.py | 1 + 6 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 tests/regressiontests/utils/itercompat.py diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 67d2531ad4..9949dfa142 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 @@ -16,6 +15,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 _ @@ -444,7 +444,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 47f4ffaba9..2841d130e1 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.utils import tree from django.db.models.fields import Field from django.db.models.sql.datastructures import EmptyResultSet from django.db.models.sql.aggregates import Aggregate +from django.utils.itercompat import is_iterator from django.utils.six.moves import xrange # Connection types @@ -51,7 +51,7 @@ class WhereNode(tree.Node): return 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 aa329c152e..2c555bd5ce 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", PendingDeprecationWarning) diff --git a/tests/regressiontests/utils/itercompat.py b/tests/regressiontests/utils/itercompat.py new file mode 100644 index 0000000000..02d3463370 --- /dev/null +++ b/tests/regressiontests/utils/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/regressiontests/utils/models.py b/tests/regressiontests/utils/models.py index 97a72bab14..de8f9e3803 100644 --- a/tests/regressiontests/utils/models.py +++ b/tests/regressiontests/utils/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/regressiontests/utils/tests.py b/tests/regressiontests/utils/tests.py index 11dd7c320e..2847a7a83a 100644 --- a/tests/regressiontests/utils/tests.py +++ b/tests/regressiontests/utils/tests.py @@ -19,6 +19,7 @@ from .functional import FunctionalTestCase from .html import TestUtilsHtml from .http import TestUtilsHttp, ETagProcessingTests, HttpDateProcessingTests from .ipv6 import TestUtilsIPv6 +from .itercompat import TestIsIterator from .jslex import JsToCForGettextTest, JsTokensTest from .module_loading import CustomLoader, DefaultLoader, EggLoader from .numberformat import TestNumberFormat