From ed37f7e979186c99a6f351c289eb486461601d80 Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Wed, 8 Oct 2014 03:27:31 +0700 Subject: [PATCH] Fixed #23611 -- update_or_create failing from a related manager Added update_or_create to RelatedManager, ManyRelatedManager and GenericRelatedObjectManager. Added missing get_or_create to GenericRelatedObjectManager. --- django/contrib/contenttypes/fields.py | 14 +++++++ django/db/models/fields/related.py | 21 +++++++++-- docs/releases/1.7.1.txt | 3 ++ tests/generic_relations/tests.py | 48 ++++++++++++++++++++++++ tests/get_or_create/tests.py | 53 ++++++++++++++++++++++++++- 5 files changed, 134 insertions(+), 5 deletions(-) diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index e0f615030af..201016ae6e0 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -537,6 +537,20 @@ def create_generic_related_manager(superclass): return super(GenericRelatedObjectManager, self).using(db).create(**kwargs) create.alters_data = True + def get_or_create(self, **kwargs): + kwargs[self.content_type_field_name] = self.content_type + kwargs[self.object_id_field_name] = self.pk_val + db = router.db_for_write(self.model, instance=self.instance) + return super(GenericRelatedObjectManager, self).using(db).get_or_create(**kwargs) + get_or_create.alters_data = True + + def update_or_create(self, **kwargs): + kwargs[self.content_type_field_name] = self.content_type + kwargs[self.object_id_field_name] = self.pk_val + db = router.db_for_write(self.model, instance=self.instance) + return super(GenericRelatedObjectManager, self).using(db).update_or_create(**kwargs) + update_or_create.alters_data = True + return GenericRelatedObjectManager diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index dce02aaa45a..69934c037ac 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -733,13 +733,17 @@ def create_foreign_related_manager(superclass, rel_field, rel_model): create.alters_data = True def get_or_create(self, **kwargs): - # Update kwargs with the related object that this - # ForeignRelatedObjectsDescriptor knows about. kwargs[rel_field.name] = self.instance db = router.db_for_write(self.model, instance=self.instance) return super(RelatedManager, self.db_manager(db)).get_or_create(**kwargs) get_or_create.alters_data = True + def update_or_create(self, **kwargs): + kwargs[rel_field.name] = self.instance + db = router.db_for_write(self.model, instance=self.instance) + return super(RelatedManager, self.db_manager(db)).update_or_create(**kwargs) + update_or_create.alters_data = True + # remove() and clear() are only provided if the ForeignKey can have a value of null. if rel_field.null: def remove(self, *objs, **kwargs): @@ -999,8 +1003,7 @@ def create_many_related_manager(superclass, rel): def get_or_create(self, **kwargs): db = router.db_for_write(self.instance.__class__, instance=self.instance) - obj, created = \ - super(ManyRelatedManager, self.db_manager(db)).get_or_create(**kwargs) + obj, created = super(ManyRelatedManager, self.db_manager(db)).get_or_create(**kwargs) # We only need to add() if created because if we got an object back # from get() then the relationship already exists. if created: @@ -1008,6 +1011,16 @@ def create_many_related_manager(superclass, rel): return obj, created get_or_create.alters_data = True + def update_or_create(self, **kwargs): + db = router.db_for_write(self.instance.__class__, instance=self.instance) + obj, created = super(ManyRelatedManager, self.db_manager(db)).update_or_create(**kwargs) + # We only need to add() if created because if we got an object back + # from get() then the relationship already exists. + if created: + self.add(obj) + return obj, created + update_or_create.alters_data = True + def _add_items(self, source_field_name, target_field_name, *objs): # source_field_name: the PK fieldname in join table for the source object # target_field_name: the PK fieldname in join table for the target object diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt index 1604e75da86..65110745423 100644 --- a/docs/releases/1.7.1.txt +++ b/docs/releases/1.7.1.txt @@ -100,3 +100,6 @@ Bugfixes * Fixed ``UnicodeDecodeError`` crash in ``AdminEmailHandler`` with non-ASCII characters in the request (:ticket:`23593`). + +* Fixed missing ``get_or_create`` and ``update_or_create`` on related managers + causing ``IntegrityError`` (:ticket:`23611`). diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index 38077df7c32..626adbbc8f1 100644 --- a/tests/generic_relations/tests.py +++ b/tests/generic_relations/tests.py @@ -35,6 +35,54 @@ class GenericRelationsTests(TestCase): obj.tag, obj.content_type.model_class(), obj.object_id ) + def test_generic_update_or_create_when_created(self): + """ + Should be able to use update_or_create from the generic related manager + to create a tag. Refs #23611. + """ + count = self.bacon.tags.count() + tag, created = self.bacon.tags.update_or_create(tag='stinky') + self.assertTrue(created) + self.assertEqual(count + 1, self.bacon.tags.count()) + + def test_generic_update_or_create_when_updated(self): + """ + Should be able to use update_or_create from the generic related manager + to update a tag. Refs #23611. + """ + count = self.bacon.tags.count() + tag = self.bacon.tags.create(tag='stinky') + self.assertEqual(count + 1, self.bacon.tags.count()) + tag, created = self.bacon.tags.update_or_create(defaults={'tag': 'juicy'}, id=tag.id) + self.assertFalse(created) + self.assertEqual(count + 1, self.bacon.tags.count()) + self.assertEqual(tag.tag, 'juicy') + + def test_generic_get_or_create_when_created(self): + """ + Should be able to use get_or_create from the generic related manager + to create a tag. Refs #23611. + """ + count = self.bacon.tags.count() + tag, created = self.bacon.tags.get_or_create(tag='stinky') + self.assertTrue(created) + self.assertEqual(count + 1, self.bacon.tags.count()) + + def test_generic_get_or_create_when_exists(self): + """ + Should be able to use get_or_create from the generic related manager + to get a tag. Refs #23611. + """ + count = self.bacon.tags.count() + tag = self.bacon.tags.create(tag="stinky") + self.assertEqual(count + 1, self.bacon.tags.count()) + tag, created = self.bacon.tags.get_or_create(id=tag.id, defaults={'tag': 'juicy'}) + self.assertFalse(created) + self.assertEqual(count + 1, self.bacon.tags.count()) + # shouldn't had changed the tag + self.assertEqual(tag.tag, 'stinky') + + def test_generic_relations_m2m_mimic(self): """ Objects with declared GenericRelations can be tagged directly -- the diff --git a/tests/get_or_create/tests.py b/tests/get_or_create/tests.py index d4dc961a180..56339a25566 100644 --- a/tests/get_or_create/tests.py +++ b/tests/get_or_create/tests.py @@ -9,7 +9,7 @@ from django.utils.encoding import DjangoUnicodeDecodeError from django.test import TestCase, TransactionTestCase from .models import (DefaultPerson, Person, ManualPrimaryKeyTest, Profile, - Tag, Thing, Publisher, Author) + Tag, Thing, Publisher, Author, Book) class GetOrCreateTests(TestCase): @@ -239,6 +239,57 @@ class UpdateOrCreateTests(TestCase): formatted_traceback = traceback.format_exc() self.assertIn('obj.save', formatted_traceback) + def test_create_with_related_manager(self): + """ + Should be able to use update_or_create from the related manager to + create a book. Refs #23611. + """ + p = Publisher.objects.create(name="Acme Publishing") + book, created = p.books.update_or_create(name="The Book of Ed & Fred") + self.assertTrue(created) + self.assertEqual(p.books.count(), 1) + + def test_update_with_related_manager(self): + """ + Should be able to use update_or_create from the related manager to + update a book. Refs #23611. + """ + p = Publisher.objects.create(name="Acme Publishing") + book = Book.objects.create(name="The Book of Ed & Fred", publisher=p) + self.assertEqual(p.books.count(), 1) + name = "The Book of Django" + book, created = p.books.update_or_create(defaults={'name': name}, id=book.id) + self.assertFalse(created) + self.assertEqual(book.name, name) + self.assertEqual(p.books.count(), 1) + + def test_create_with_many(self): + """ + Should be able to use update_or_create from the m2m related manager to + create a book. Refs #23611. + """ + p = Publisher.objects.create(name="Acme Publishing") + author = Author.objects.create(name="Ted") + book, created = author.books.update_or_create(name="The Book of Ed & Fred", publisher=p) + self.assertTrue(created) + self.assertEqual(author.books.count(), 1) + + def test_update_with_many(self): + """ + Should be able to use update_or_create from the m2m related manager to + update a book. Refs #23611. + """ + p = Publisher.objects.create(name="Acme Publishing") + author = Author.objects.create(name="Ted") + book = Book.objects.create(name="The Book of Ed & Fred", publisher=p) + book.authors.add(author) + self.assertEqual(author.books.count(), 1) + name = "The Book of Django" + book, created = author.books.update_or_create(defaults={'name': name}, id=book.id) + self.assertFalse(created) + self.assertEqual(book.name, name) + self.assertEqual(author.books.count(), 1) + def test_related(self): p = Publisher.objects.create(name="Acme Publishing") # Create a book through the publisher.