Skip to content

Commit

Permalink
Fixed #24701 -- Converted model manager names to unicode in migrations
Browse files Browse the repository at this point in the history
Thanks to Reto Aebersold for reporting the issue and Tim Graham and
Claude Paroz for the review.
  • Loading branch information
MarkusH committed Apr 25, 2015
1 parent 1521861 commit faad607
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
12 changes: 7 additions & 5 deletions django/db/migrations/state.py
Expand Up @@ -436,7 +436,7 @@ def flatten_bases(model):
bases = (models.Model,)

# Constructs all managers on the model
managers = {}
managers_mapping = {}

def reconstruct_manager(mgr):
as_manager, manager_path, qs_path, args, kwargs = mgr.deconstruct()
Expand All @@ -448,16 +448,17 @@ def reconstruct_manager(mgr):
instance = manager_class(*args, **kwargs)
# We rely on the ordering of the creation_counter of the original
# instance
managers[mgr.name] = (mgr.creation_counter, instance)
name = force_text(mgr.name)
managers_mapping[name] = (mgr.creation_counter, instance)

if hasattr(model, "_default_manager"):
default_manager_name = model._default_manager.name
default_manager_name = force_text(model._default_manager.name)
# Make sure the default manager is always the first
if model._default_manager.use_in_migrations:
reconstruct_manager(model._default_manager)
else:
# Force this manager to be the first and thus default
managers[default_manager_name] = (0, models.Manager())
managers_mapping[default_manager_name] = (0, models.Manager())
# Sort all managers by their creation counter
for _, manager, _ in sorted(model._meta.managers):
if manager.name == "_base_manager" or not manager.use_in_migrations:
Expand All @@ -467,7 +468,7 @@ def reconstruct_manager(mgr):
# instance for further processing
managers = [
(name, instance) for name, (cc, instance) in
sorted(managers.items(), key=lambda v: v[1])
sorted(managers_mapping.items(), key=lambda v: v[1])
]
if managers == [(default_manager_name, models.Manager())]:
managers = []
Expand Down Expand Up @@ -513,6 +514,7 @@ def construct_managers(self):
# Sort all managers by their creation counter
sorted_managers = sorted(self.managers, key=lambda v: v[1].creation_counter)
for mgr_name, manager in sorted_managers:
mgr_name = force_text(mgr_name)
as_manager, manager_path, qs_path, args, kwargs = manager.deconstruct()
if as_manager:
qs_class = import_string(qs_path)
Expand Down
3 changes: 3 additions & 0 deletions docs/releases/1.8.1.txt
Expand Up @@ -66,6 +66,9 @@ Bugfixes
* Fixed a migration crash when adding new relations to models
(:ticket:`24573`).

* Fixed a migration crash when applying migrations with model managers on
Python 3 that were generated on Python 2 (:ticket:`24701`).

Optimizations
=============

Expand Down
7 changes: 6 additions & 1 deletion tests/migrations/test_state.py
Expand Up @@ -7,6 +7,7 @@
InvalidBasesError, ModelState, ProjectState, get_related_models_recursive,
)
from django.test import SimpleTestCase, TestCase, override_settings
from django.utils import six

from .models import (
FoodManager, FoodQuerySet, ModelWithCustomBase, NoMigrationFoodManager,
Expand Down Expand Up @@ -144,6 +145,7 @@ class Meta:

# The default manager is used in migrations
self.assertEqual([name for name, mgr in food_state.managers], ['food_mgr'])
self.assertTrue(all(isinstance(name, six.text_type) for name, mgr in food_state.managers))
self.assertEqual(food_state.managers[0][1].args, ('a', 'b', 1, 2))

# No explicit managers defined. Migrations will fall back to the default
Expand All @@ -153,11 +155,13 @@ class Meta:
# default
self.assertEqual([name for name, mgr in food_no_default_manager_state.managers],
['food_no_mgr', 'food_mgr'])
self.assertTrue(all(isinstance(name, six.text_type) for name, mgr in food_no_default_manager_state.managers))
self.assertEqual(food_no_default_manager_state.managers[0][1].__class__, models.Manager)
self.assertIsInstance(food_no_default_manager_state.managers[1][1], FoodManager)

self.assertEqual([name for name, mgr in food_order_manager_state.managers],
['food_mgr1', 'food_mgr2'])
self.assertTrue(all(isinstance(name, six.text_type) for name, mgr in food_order_manager_state.managers))
self.assertEqual([mgr.args for name, mgr in food_order_manager_state.managers],
[('a', 'b', 1, 2), ('x', 'y', 3, 4)])

Expand Down Expand Up @@ -220,7 +224,7 @@ def test_render(self):
# The ordering we really want is objects, mgr1, mgr2
('default', base_mgr),
('food_mgr2', mgr2),
('food_mgr1', mgr1),
(b'food_mgr1', mgr1),
]
))

Expand All @@ -234,6 +238,7 @@ def test_render(self):
managers = sorted(Food._meta.managers)
self.assertEqual([mgr.name for _, mgr, _ in managers],
['default', 'food_mgr1', 'food_mgr2'])
self.assertTrue(all(isinstance(mgr.name, six.text_type) for _, mgr, _ in managers))
self.assertEqual([mgr.__class__ for _, mgr, _ in managers],
[models.Manager, FoodManager, FoodManager])
self.assertIs(managers[0][1], Food._default_manager)
Expand Down

0 comments on commit faad607

Please sign in to comment.