Fixed #13724: Corrected routing of write queries involving managers. (9595183d) · Commits · Dom Sekotill / django

django/db/models/fields/related.py

+26 −14

Original line number	Diff line number	Diff line
		@@ -159,9 +159,8 @@ class SingleRelatedObjectDescriptor(six.with_metaclass(RenameRelatedObjectDescri
		def is_cached(self, instance):
		return hasattr(instance, self.cache_name)

		def get_queryset(self, **db_hints):
		db = router.db_for_read(self.related.model, **db_hints)
		return self.related.model._base_manager.using(db)
		def get_queryset(self, **hints):
		return self.related.model._base_manager.db_manager(hints=hints)

		def get_prefetch_queryset(self, instances):
		rel_obj_attr = attrgetter(self.related.field.attname)
		@@ -256,15 +255,14 @@ class ReverseSingleRelatedObjectDescriptor(six.with_metaclass(RenameRelatedObjec
		def is_cached(self, instance):
		return hasattr(instance, self.cache_name)

		def get_queryset(self, **db_hints):
		db = router.db_for_read(self.field.rel.to, **db_hints)
		rel_mgr = self.field.rel.to._default_manager
		def get_queryset(self, **hints):
		rel_mgr = self.field.rel.to._default_manager.db_manager(hints=hints)
		# If the related manager indicates that it should be used for
		# related fields, respect that.
		if getattr(rel_mgr, 'use_for_related_fields', False):
		return rel_mgr.using(db)
		return rel_mgr
		else:
		return QuerySet(self.field.rel.to).using(db)
		return QuerySet(self.field.rel.to, hints=hints)

		def get_prefetch_queryset(self, instances):
		rel_obj_attr = self.field.get_foreign_related_value
		@@ -385,8 +383,12 @@ def create_foreign_related_manager(superclass, rel_field, rel_model):
		return self.instance._prefetched_objects_cache[rel_field.related_query_name()]
		except (AttributeError, KeyError):
		db = self._db or router.db_for_read(self.model, instance=self.instance)
		qs = super(RelatedManager, self).get_queryset().using(db).filter(**self.core_filters)
		empty_strings_as_null = connections[db].features.interprets_empty_strings_as_nulls
		qs = super(RelatedManager, self).get_queryset()
		qs._add_hints(instance=self.instance)
		if self._db:
		qs = qs.using(self._db)
		qs = qs.filter(**self.core_filters)
		for field in rel_field.foreign_related_fields:
		val = getattr(self.instance, field.attname)
		if val is None or (val == '' and empty_strings_as_null):
		@@ -398,9 +400,12 @@ def create_foreign_related_manager(superclass, rel_field, rel_model):
		rel_obj_attr = rel_field.get_local_related_value
		instance_attr = rel_field.get_foreign_related_value
		instances_dict = dict((instance_attr(inst), inst) for inst in instances)
		db = self._db or router.db_for_read(self.model, instance=instances[0])
		query = {'%s__in' % rel_field.name: instances}
		qs = super(RelatedManager, self).get_queryset().using(db).filter(**query)
		qs = super(RelatedManager, self).get_queryset()
		qs._add_hints(instance=instances[0])
		if self._db:
		qs = qs.using(self._db)
		qs = qs.filter(**query)
		# Since we just bypassed this class' get_queryset(), we must manage
		# the reverse relation manually.
		for rel_obj in qs:
		@@ -545,14 +550,21 @@ def create_many_related_manager(superclass, rel):
		try:
		return self.instance._prefetched_objects_cache[self.prefetch_cache_name]
		except (AttributeError, KeyError):
		db = self._db or router.db_for_read(self.instance.__class__, instance=self.instance)
		return super(ManyRelatedManager, self).get_queryset().using(db)._next_is_sticky().filter(**self.core_filters)
		qs = super(ManyRelatedManager, self).get_queryset()
		qs._add_hints(instance=self.instance)
		if self._db:
		qs = qs.using(self._db)
		return qs._next_is_sticky().filter(**self.core_filters)

		def get_prefetch_queryset(self, instances):
		instance = instances[0]
		db = self._db or router.db_for_read(instance.__class__, instance=instance)
		query = {'%s__in' % self.query_field_name: instances}
		qs = super(ManyRelatedManager, self).get_queryset().using(db)._next_is_sticky().filter(**query)
		qs = super(ManyRelatedManager, self).get_queryset()
		qs._add_hints(instance=instance)
		if self._db:
		qs = qs.using(db)
		qs = qs._next_is_sticky().filter(**query)

		# M2M: need to annotate the query in order to get the primary model
		# that the secondary model was actually related to. We know that

django/db/models/manager.py

+6 −4

Original line number	Diff line number	Diff line
		@@ -68,6 +68,7 @@ class BaseManager(six.with_metaclass(RenameManagerMethods)):
		self.model = None
		self._inherited = False
		self._db = None
		self._hints = {}

		@classmethod
		def _get_queryset_methods(cls, queryset_class):
		@@ -144,21 +145,22 @@ class BaseManager(six.with_metaclass(RenameManagerMethods)):
		mgr._inherited = True
		return mgr

		def db_manager(self, using):
		def db_manager(self, using=None, hints=None):
		obj = copy.copy(self)
		obj._db = using
		obj._db = using or self._db
		obj._hints = hints or self._hints
		return obj

		@property
		def db(self):
		return self._db or router.db_for_read(self.model)
		return self._db or router.db_for_read(self.model, **self._hints)

		def get_queryset(self):
		"""
		Returns a new QuerySet object. Subclasses can override this method to
		easily customize the behavior of the Manager.
		"""
		return self._queryset_class(self.model, using=self._db)
		return self._queryset_class(self.model, using=self._db, hints=self._hints)

		def all(self):
		# We can't proxy this method through the `QuerySet` like we do for the

django/db/models/query.py

+16 −6

Original line number	Diff line number	Diff line
		@@ -47,9 +47,10 @@ class QuerySet(object):
		Represents a lazy database lookup for a set of objects.
		"""

		def __init__(self, model=None, query=None, using=None):
		def __init__(self, model=None, query=None, using=None, hints=None):
		self.model = model
		self._db = using
		self._hints = hints or {}
		self.query = query or sql.Query(self.model)
		self._result_cache = None
		self._sticky_filter = False
		@@ -904,8 +905,8 @@ class QuerySet(object):
		def db(self):
		"Return the database that will be used if this query is executed now"
		if self._for_write:
		return self._db or router.db_for_write(self.model)
		return self._db or router.db_for_read(self.model)
		return self._db or router.db_for_write(self.model, **self._hints)
		return self._db or router.db_for_read(self.model, **self._hints)

		###################
		# PRIVATE METHODS #
		@@ -955,7 +956,7 @@ class QuerySet(object):
		query = self.query.clone()
		if self._sticky_filter:
		query.filter_is_sticky = True
		c = klass(model=self.model, query=query, using=self._db)
		c = klass(model=self.model, query=query, using=self._db, hints=self._hints)
		c._for_write = self._for_write
		c._prefetch_related_lookups = self._prefetch_related_lookups[:]
		c._known_related_objects = self._known_related_objects
		@@ -1025,6 +1026,14 @@ class QuerySet(object):
		# empty" result.
		value_annotation = True

		def _add_hints(self, **hints):
		"""
		Update hinting information for later use by Routers
		"""
		# If there is any hinting information, add it to what we already know.
		# If we have a new hint for an existing key, overwrite with the new value.
		self._hints.update(hints)


		class InstanceCheckMeta(type):
		def __instancecheck__(self, instance):
		@@ -1485,10 +1494,11 @@ class RawQuerySet(object):
		annotated model instances.
		"""
		def __init__(self, raw_query, model=None, query=None, params=None,
		translations=None, using=None):
		translations=None, using=None, hints=None):
		self.raw_query = raw_query
		self.model = model
		self._db = using
		self._hints = hints or {}
		self.query = query or sql.RawQuery(sql=raw_query, using=self.db, params=params)
		self.params = params or ()
		self.translations = translations or {}
		@@ -1572,7 +1582,7 @@ class RawQuerySet(object):
		@property
		def db(self):
		"Return the database that will be used if this query is executed now"
		return self._db or router.db_for_read(self.model)
		return self._db or router.db_for_read(self.model, **self._hints)

		def using(self, alias):
		"""

docs/releases/1.7.txt

+14 −0

Original line number	Diff line number	Diff line
		@@ -368,6 +368,20 @@ For apps with migrations, ``allow_migrate`` will now get passed
		without custom attributes, methods or managers. Make sure your ``allow_migrate``
		methods are only referring to fields or other items in ``model._meta``.

		Passing ``None`` to ``Manager.db_manager()``
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

		In previous versions of Django, it was possible to use
		``db_manager(using=None)`` on a model manager instance to obtain a manager
		instance using default routing behavior, overriding any manually specified
		database routing. In Django 1.7, a value of ``None`` passed to db_manager will
		produce a router that retains any manually assigned database routing -- the
		manager will not be reset. This was necessary to resolve an inconsistency in
		the way routing information cascaded over joins. See `Ticket #13724`_ for more
		details.

		.. _Ticket #13724: https://code.djangoproject.com/ticket/13724

		pytz may be required
		~~~~~~~~~~~~~~~~~~~~

tests/multiple_database/tests.py

+241 −0

Original line number	Diff line number	Diff line
		@@ -1902,3 +1902,244 @@ class MigrateTestCase(TestCase):
		router.routers = old_routers

		self.assertEqual(cts.count(), 0)




		class RouterUsed(Exception):
		WRITE = 'write'

		def __init__(self, mode, model, hints):
		self.mode = mode
		self.model = model
		self.hints = hints


		class RouteForWriteTestCase(TestCase):
		multi_db = True
		RAISE_MSG = 'Db for write called'

		class WriteCheckRouter(object):
		def db_for_write(self, model, **hints):
		raise RouterUsed(mode=RouterUsed.WRITE, model=model, hints=hints)

		def setUp(self):
		self._old_rtrs = router.routers

		def tearDown(self):
		router.routers = self._old_rtrs

		def enable_router(self):
		router.routers = [RouteForWriteTestCase.WriteCheckRouter()]

		def test_fk_delete(self):
		owner = Person.objects.create(name='Someone')
		pet = Pet.objects.create(name='fido', owner=owner)
		self.enable_router()
		try:
		pet.owner.delete()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Person)
		self.assertEqual(e.hints, {'instance': owner})

		def test_reverse_fk_delete(self):
		owner = Person.objects.create(name='Someone')
		to_del_qs = owner.pet_set.all()
		self.enable_router()
		try:
		to_del_qs.delete()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Pet)
		self.assertEqual(e.hints, {'instance': owner})

		def test_reverse_fk_get_or_create(self):
		owner = Person.objects.create(name='Someone')
		self.enable_router()
		try:
		owner.pet_set.get_or_create(name='fido')
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Pet)
		self.assertEqual(e.hints, {'instance': owner})

		def test_reverse_fk_update(self):
		owner = Person.objects.create(name='Someone')
		pet = Pet.objects.create(name='fido', owner=owner)
		self.enable_router()
		try:
		owner.pet_set.update(name='max')
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Pet)
		self.assertEqual(e.hints, {'instance': owner})

		def test_m2m_add(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		self.enable_router()
		try:
		book.authors.add(auth)
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': book})

		def test_m2m_clear(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		book.authors.clear()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': book})

		def test_m2m_delete(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		book.authors.all().delete()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Person)
		self.assertEqual(e.hints, {'instance': book})

		def test_m2m_get_or_create(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		self.enable_router()
		try:
		book.authors.get_or_create(name='Someone else')
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book)
		self.assertEqual(e.hints, {'instance': book})

		def test_m2m_remove(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		self.assertRaisesMessage(AttributeError, self.RAISE_MSG, )
		try:
		book.authors.remove(auth)
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': book})

		def test_m2m_update(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		book.authors.all().update(name='Different')
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Person)
		self.assertEqual(e.hints, {'instance': book})

		def test_reverse_m2m_add(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		self.enable_router()
		try:
		auth.book_set.add(book)
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': auth})

		def test_reverse_m2m_clear(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		auth.book_set.clear()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': auth})

		def test_reverse_m2m_delete(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		auth.book_set.all().delete()
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book)
		self.assertEqual(e.hints, {'instance': auth})

		def test_reverse_m2m_get_or_create(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		self.enable_router()
		try:
		auth.book_set.get_or_create(title="New Book", published=datetime.datetime.now())
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Person)
		self.assertEqual(e.hints, {'instance': auth})

		def test_reverse_m2m_remove(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		auth.book_set.remove(book)
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book.authors.through)
		self.assertEqual(e.hints, {'instance': auth})

		def test_reverse_m2m_update(self):
		auth = Person.objects.create(name='Someone')
		book = Book.objects.create(title="Pro Django",
		published=datetime.date(2008, 12, 16))
		book.authors.add(auth)
		self.enable_router()
		try:
		auth.book_set.all().update(title='Different')
		self.fail('db_for_write() not invoked on router')
		except RouterUsed, e:
		self.assertEqual(e.mode, RouterUsed.WRITE)
		self.assertEqual(e.model, Book)
		self.assertEqual(e.hints, {'instance': auth})