Fixed #26643 -- Prevented unnecessary AlterModelManagers operations caused by…

Fixed #26643 -- Prevented unnecessary AlterModelManagers operations caused by the manager inheritance refactor.

This also makes migrations respect the base_manager_name and
default_manager_name model options.

Thanks Anthony King and Matthew Schinckel for the initial patches.

django/db/migrations/operations/models.py

@@ -665,6 +665,8 @@ class AlterModelOptions(ModelOptionOperation):
 
     # Model options we want to compare and preserve in an AlterModelOptions op
     ALTER_OPTION_KEYS = [
+        "base_manager_name",
+        "default_manager_name",
         "get_latest_by",
         "managed",
         "ordering",

django/db/migrations/state.py

 from __future__ import unicode_literals
 
 import copy
+import warnings
 from collections import OrderedDict
 from contextlib import contextmanager
 
@@ -13,6 +14,7 @@ from django.db.models.fields.related import RECURSIVE_RELATIONSHIP_CONSTANT
 from django.db.models.options import DEFAULT_NAMES, normalize_together
 from django.db.models.utils import make_model_tuple
 from django.utils import six
+from django.utils.deprecation import RemovedInDjango20Warning
 from django.utils.encoding import force_text, smart_text
 from django.utils.functional import cached_property
 from django.utils.module_loading import import_string
@@ -444,28 +446,24 @@ class ModelState(object):
             bases = (models.Model,)
 
         managers = []
-
-        # Make sure the default manager is always first since ordering chooses
-        # the default manager.
-        if not model._default_manager.auto_created:
-            if model._default_manager.use_in_migrations:
-                default_manager = copy.copy(model._default_manager)
-                default_manager._set_creation_counter()
-
-            # If the default manager doesn't have `use_in_migrations = True`,
-            # shim a default manager so another manager isn't promoted in its
-            # place.
+        default_manager_shim = None
+        for manager in model._meta.managers:
+            if manager.use_in_migrations:
+                new_manager = copy.copy(manager)
+                new_manager._set_creation_counter()
+            elif manager is model._base_manager or manager is model._default_manager:
+                new_manager = models.Manager()
+                new_manager.model = manager.model
+                new_manager.name = manager.name
+                if manager is model._default_manager:
+                    default_manager_shim = new_manager
             else:
-                default_manager = models.Manager()
-                default_manager.model = model
-                default_manager.name = model._default_manager.name
-            managers.append((force_text(default_manager.name), default_manager))
+                continue
+            managers.append((force_text(manager.name), new_manager))
 
-        for manager in model._meta.managers:
-            if manager.use_in_migrations and manager is not model._default_manager:
-                manager = copy.copy(manager)
-                manager._set_creation_counter()
-                managers.append((force_text(manager.name), manager))
+        # Ignore a shimmed default manager called objects if it's the only one.
+        if managers == [('objects', default_manager_shim)]:
+            managers = []
 
         # Construct the new ModelState
         return cls(
@@ -541,12 +539,17 @@ class ModelState(object):
         # Restore managers
         body.update(self.construct_managers())
 
-        # Then, make a Model object (apps.register_model is called in __new__)
-        return type(
-            str(self.name),
-            bases,
-            body,
-        )
+        with warnings.catch_warnings():
+            warnings.filterwarnings(
+                "ignore", "Managers from concrete parents will soon qualify as default managers",
+                RemovedInDjango20Warning)
+
+            # Then, make a Model object (apps.register_model is called in __new__)
+            return type(
+                str(self.name),
+                bases,
+                body,
+            )
 
     def get_field_by_name(self, name):
         for fname, field in self.fields:

tests/migrations/test_state.py

@@ -191,6 +191,78 @@ class StateTests(SimpleTestCase):
         author_state = project_state.models['migrations', 'author']
         self.assertEqual(author_state.managers, [('authors', custom_manager)])
 
+    def test_custom_default_manager_named_objects_with_false_migration_flag(self):
+        """
+        When a manager is added with a name of 'objects' but it does not
+        have `use_in_migrations = True`, no migration should be added to the
+        model state (#26643).
+        """
+        new_apps = Apps(['migrations'])
+
+        class Author(models.Model):
+            objects = models.Manager()
+
+            class Meta:
+                app_label = 'migrations'
+                apps = new_apps
+
+        project_state = ProjectState.from_apps(new_apps)
+        author_state = project_state.models['migrations', 'author']
+        self.assertEqual(author_state.managers, [])
+
+    def test_custom_default_manager(self):
+        new_apps = Apps(['migrations'])
+
+        class Author(models.Model):
+            manager1 = models.Manager()
+            manager2 = models.Manager()
+
+            class Meta:
+                app_label = 'migrations'
+                apps = new_apps
+                default_manager_name = 'manager2'
+
+        project_state = ProjectState.from_apps(new_apps)
+        author_state = project_state.models['migrations', 'author']
+        self.assertEqual(author_state.options['default_manager_name'], 'manager2')
+        self.assertEqual(author_state.managers, [('manager2', Author.manager1)])
+
+    def test_custom_base_manager(self):
+        new_apps = Apps(['migrations'])
+
+        class Author(models.Model):
+            manager1 = models.Manager()
+            manager2 = models.Manager()
+
+            class Meta:
+                app_label = 'migrations'
+                apps = new_apps
+                base_manager_name = 'manager2'
+
+        class Author2(models.Model):
+            manager1 = models.Manager()
+            manager2 = models.Manager()
+
+            class Meta:
+                app_label = 'migrations'
+                apps = new_apps
+                base_manager_name = 'manager1'
+
+        project_state = ProjectState.from_apps(new_apps)
+
+        author_state = project_state.models['migrations', 'author']
+        self.assertEqual(author_state.options['base_manager_name'], 'manager2')
+        self.assertEqual(author_state.managers, [
+            ('manager1', Author.manager1),
+            ('manager2', Author.manager2),
+        ])
+
+        author2_state = project_state.models['migrations', 'author2']
+        self.assertEqual(author2_state.options['base_manager_name'], 'manager1')
+        self.assertEqual(author2_state.managers, [
+            ('manager1', Author2.manager1),
+        ])
+
     def test_apps_bulk_update(self):
         """
         StateApps.bulk_update() should update apps.ready to False and reset