From 80e3444eca045799cc40e50c92609e852a299d38 Mon Sep 17 00:00:00 2001 From: David Szotten Date: Sun, 15 Mar 2015 19:07:39 +0000 Subject: [PATCH] Fixed #24483 -- Prevented keepdb from breaking with generator choices. If Field.choices is provided as an iterator, consume it in __init__ instead of using itertools.tee (which ends up holding everything in memory anyway). Fixes a bug where deconstruct() was consuming the iterator but bypassing the call to `tee`. --- django/db/models/fields/__init__.py | 16 ++++------------ tests/migrations/test_state.py | 20 ++++++++++++++++++++ tests/model_fields/tests.py | 7 ------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 975653d410..6ee1185c90 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -9,7 +9,6 @@ import math import uuid import warnings from base64 import b64decode, b64encode -from itertools import tee from django.apps import apps from django.db import connection @@ -155,7 +154,9 @@ class Field(RegisterLookupMixin): self.unique_for_date = unique_for_date self.unique_for_month = unique_for_month self.unique_for_year = unique_for_year - self._choices = choices or [] + if isinstance(choices, collections.Iterator): + choices = list(choices) + self.choices = choices or [] self.help_text = help_text self.db_index = db_index self.db_column = db_column @@ -405,7 +406,6 @@ class Field(RegisterLookupMixin): } attr_overrides = { "unique": "_unique", - "choices": "_choices", "error_messages": "_error_messages", "validators": "_validators", "verbose_name": "_verbose_name", @@ -553,7 +553,7 @@ class Field(RegisterLookupMixin): # Skip validation for non-editable fields. return - if self._choices and value not in self.empty_values: + if self.choices and value not in self.empty_values: for option_key, option_value in self.choices: if isinstance(option_value, (list, tuple)): # This is an optgroup, so look inside the group for @@ -848,14 +848,6 @@ class Field(RegisterLookupMixin): """ return smart_text(self._get_val_from_obj(obj)) - def _get_choices(self): - if isinstance(self._choices, collections.Iterator): - choices, self._choices = tee(self._choices) - return choices - else: - return self._choices - choices = property(_get_choices) - def _get_flatchoices(self): """Flattened version of choices tuple.""" flat = [] diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 7069018683..c3e0232696 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -595,6 +595,26 @@ class StateTests(TestCase): self.assertIsNot(old_model.food_mgr.model, new_model.food_mgr.model) self.assertIsNot(old_model.food_qs.model, new_model.food_qs.model) + def test_choices_iterator(self): + """ + #24483 - ProjectState.from_apps should not destructively consume + Field.choices iterators. + """ + new_apps = Apps(["migrations"]) + choices = [('a', 'A'), ('b', 'B')] + + class Author(models.Model): + name = models.CharField(max_length=255) + choice = models.CharField(max_length=255, choices=iter(choices)) + + class Meta: + app_label = "migrations" + apps = new_apps + + ProjectState.from_apps(new_apps) + choices_field = Author._meta.get_field('choice') + self.assertEqual(list(choices_field.choices), choices) + class ModelStateTests(TestCase): def test_custom_model_base(self): diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index a8291b6e71..80d35d18e1 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -445,13 +445,6 @@ class ChoicesTests(test.TestCase): self.assertEqual(WhizIterEmpty(c=None).c, None) # Blank value self.assertEqual(WhizIterEmpty(c='').c, '') # Empty value - def test_charfield_get_choices_with_blank_iterator(self): - """ - Check that get_choices works with an empty Iterator - """ - f = models.CharField(choices=(x for x in [])) - self.assertEqual(f.get_choices(include_blank=True), [('', '---------')]) - class SlugFieldTests(test.TestCase): def test_slugfield_max_length(self):