Fixed #7350, #7202 -- Fixed serialization for multi-model inheritance, which had multiple problems:

* Serializers were including all superclass fields in their output. Now only local fields are included.
 * Implicit OneToOne primary keys were not correctly added to the metamodel, so they were always marked to be serialized, even though they were primary
 * Model saving was too aggressive about creating new parent class instances during deserialization. Raw save on a model now skips saving of the parent class.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@7600 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/core/serializers/base.py

@@ -38,7 +38,7 @@ class Serializer(object):
         self.start_serialization()
         for obj in queryset:
             self.start_object(obj)
-            for field in obj._meta.fields:
+            for field in obj._meta.local_fields:
                 if field.serialize:
                     if field.rel is None:
                         if self.selected_fields is None or field.attname in self.selected_fields:

django/db/models/base.py

@@ -290,12 +290,17 @@ class Model(object):
             meta = cls._meta
             signal = False
 
-        for parent, field in meta.parents.items():
+        # If we are in a raw save, save the object exactly as presented.
-            self.save_base(raw, parent)
+        # That means that we don't try to be smart about saving attributes
-            setattr(self, field.attname, self._get_pk_val(parent._meta))
+        # that might have come from the parent class - we just save the 
+        # attributes we have been given to the class we have been given.
+        if not raw:
+            for parent, field in meta.parents.items():
+                self.save_base(raw, parent)
+                setattr(self, field.attname, self._get_pk_val(parent._meta))
 
         non_pks = [f for f in meta.local_fields if not f.primary_key]
-
+            
         # First, try an UPDATE. If that doesn't update anything, do an INSERT.
         pk_val = self._get_pk_val(meta)
         # Note: the comparison with '' is required for compatibility with

django/db/models/options.py

@@ -102,7 +102,7 @@ class Options(object):
                 # field.
                 field = self.parents.value_for_index(0)
                 field.primary_key = True
-                self.pk = field
+                self.setup_pk(field)
             else:
                 auto = AutoField(verbose_name='ID', primary_key=True,
                         auto_created=True)

docs/serialization.txt

@@ -63,6 +63,41 @@ be serialized.
     doesn't specify all the fields that are required by a model, the deserializer
     will not be able to save deserialized instances.
 
+Inherited Models
+~~~~~~~~~~~~~~~~
+
+If you have a model that is defined using an `abstract base class`_, you don't
+have to do anything special to serialize that model. Just call the serializer
+on the object (or objects) that you want to serialize, and the output will be
+a complete representation of the serialized object.
+
+However, if you have a model that uses `multi-table inheritance`_, you also
+need to serialize all of the base classes for the model. This is because only
+the fields that are locally defined on the model will be serialized. For
+example, consider the following models::
+	
+	class Place(models.Model):
+		name = models.CharField(max_length=50)
+		
+	class Restaurant(Place):
+		serves_hot_dogs = models.BooleanField()
+		
+If you only serialize the Restaurant model::
+
+	data = serializers.serialize('xml', Restaurant.objects.all())
+
+the fields on the serialized output will only contain the `serves_hot_dogs`
+attribute. The `name` attribute of the base class will be ignored.
+
+In order to fully serialize your Restaurant instances, you will need to
+serialize the Place models as well::
+
+	all_objects = list(Restaurant.objects.all()) + list(Place.objects.all())
+	data = serializers.serialize('xml', all_objects)
+
+.. _abstract base class: http://www.djangoproject.com/documentation/model-api/#abstract-base-classes
+.. _multi-table inheritance: http://www.djangoproject.com/documentation/model-api/#multi-table-inheritance
+
 Deserializing data
 ------------------
 

tests/modeltests/model_inheritance/models.py

@@ -147,8 +147,13 @@ Test constructor for Restaurant.
 >>> c.save()
 >>> ir = ItalianRestaurant(name='Ristorante Miron', address='1234 W. Ash', serves_hot_dogs=False, serves_pizza=False, serves_gnocchi=True, rating=4, chef=c)
 >>> ir.save()
+>>> ItalianRestaurant.objects.filter(address='1234 W. Ash')
+[<ItalianRestaurant: Ristorante Miron the italian restaurant>]
+
 >>> ir.address = '1234 W. Elm'
 >>> ir.save()
+>>> ItalianRestaurant.objects.filter(address='1234 W. Elm')
+[<ItalianRestaurant: Ristorante Miron the italian restaurant>]
 
 # Make sure Restaurant and ItalianRestaurant have the right fields in the right
 # order.

tests/regressiontests/model_inheritance_regress/models.py

@@ -0,0 +1,120 @@
+"""
+Regression tests for Model inheritance behaviour.
+"""
+
+from django.db import models
+
+class Place(models.Model):
+    name = models.CharField(max_length=50)
+    address = models.CharField(max_length=80)
+
+    class Meta:
+        ordering = ('name',)
+        
+    def __unicode__(self):
+        return u"%s the place" % self.name
+
+class Restaurant(Place):
+    serves_hot_dogs = models.BooleanField()
+    serves_pizza = models.BooleanField()
+
+    def __unicode__(self):
+        return u"%s the restaurant" % self.name
+
+class ItalianRestaurant(Restaurant):
+    serves_gnocchi = models.BooleanField()
+
+    def __unicode__(self):
+        return u"%s the italian restaurant" % self.name
+
+class ParkingLot(Place):
+    # An explicit link to the parent (we can control the attribute name).
+    parent = models.OneToOneField(Place, primary_key=True, parent_link=True)
+    capacity = models.IntegerField()
+
+    def __unicode__(self):
+        return u"%s the parking lot" % self.name
+
+__test__ = {'API_TESTS':"""
+# Regression for #7350, #7202
+# Check that when you create a Parent object with a specific reference to an existent
+# child instance, saving the Parent doesn't duplicate the child. 
+# This behaviour is only activated during a raw save - it is mostly relevant to 
+# deserialization, but any sort of CORBA style 'narrow()' API would require a
+# similar approach.
+
+# Create a child-parent-grandparent chain
+>>> place1 = Place(name="Guido's House of Pasta", address='944 W. Fullerton')
+>>> place1.save_base(raw=True)
+>>> restaurant = Restaurant(place_ptr=place1, serves_hot_dogs=True, serves_pizza=False)
+>>> restaurant.save_base(raw=True)
+>>> italian_restaurant = ItalianRestaurant(restaurant_ptr=restaurant, serves_gnocchi=True)
+>>> italian_restaurant.save_base(raw=True)
+
+# Create a child-parent chain with an explicit parent link
+>>> place2 = Place(name='Main St', address='111 Main St')
+>>> place2.save_base(raw=True)
+>>> park = ParkingLot(parent=place2, capacity=100)
+>>> park.save_base(raw=True)
+
+# Check that no extra parent objects have been created.
+>>> Place.objects.all()
+[<Place: Guido's House of Pasta the place>, <Place: Main St the place>]
+
+>>> dicts = Restaurant.objects.values('name','serves_hot_dogs')
+>>> [sorted(d.items()) for d in dicts]
+[[('name', u"Guido's House of Pasta"), ('serves_hot_dogs', True)]]
+
+>>> dicts = ItalianRestaurant.objects.values('name','serves_hot_dogs','serves_gnocchi')
+>>> [sorted(d.items()) for d in dicts]
+[[('name', u"Guido's House of Pasta"), ('serves_gnocchi', True), ('serves_hot_dogs', True)]]
+
+>>> dicts = ParkingLot.objects.values('name','capacity')
+>>> [sorted(d.items()) for d in dicts]
+[[('capacity', 100), ('name', u'Main St')]]
+
+# You can also update objects when using a raw save.
+>>> place1.name = "Guido's All New House of Pasta"
+>>> place1.save_base(raw=True)
+
+>>> restaurant.serves_hot_dogs = False
+>>> restaurant.save_base(raw=True)
+
+>>> italian_restaurant.serves_gnocchi = False
+>>> italian_restaurant.save_base(raw=True)
+
+>>> place2.name='Derelict lot'
+>>> place2.save_base(raw=True)
+
+>>> park.capacity = 50
+>>> park.save_base(raw=True)
+
+# No extra parent objects after an update, either.
+>>> Place.objects.all()
+[<Place: Derelict lot the place>, <Place: Guido's All New House of Pasta the place>]
+
+>>> dicts = Restaurant.objects.values('name','serves_hot_dogs')
+>>> [sorted(d.items()) for d in dicts]
+[[('name', u"Guido's All New House of Pasta"), ('serves_hot_dogs', False)]]
+
+>>> dicts = ItalianRestaurant.objects.values('name','serves_hot_dogs','serves_gnocchi')
+>>> [sorted(d.items()) for d in dicts]
+[[('name', u"Guido's All New House of Pasta"), ('serves_gnocchi', False), ('serves_hot_dogs', False)]]
+
+>>> dicts = ParkingLot.objects.values('name','capacity')
+>>> [sorted(d.items()) for d in dicts]
+[[('capacity', 50), ('name', u'Derelict lot')]]
+
+# If you try to raw_save a parent attribute onto a child object,
+# the attribute will be ignored.
+
+>>> italian_restaurant.name = "Lorenzo's Pasta Hut"
+>>> italian_restaurant.save_base(raw=True)
+
+# Note that the name has not changed
+# - name is an attribute of Place, not ItalianRestaurant
+>>> dicts = ItalianRestaurant.objects.values('name','serves_hot_dogs','serves_gnocchi')
+>>> [sorted(d.items()) for d in dicts]
+[[('name', u"Guido's All New House of Pasta"), ('serves_gnocchi', False), ('serves_hot_dogs', False)]]
+
+"""}

tests/regressiontests/serializers_regress/models.py

@@ -223,3 +223,23 @@ class ModifyingSaveData(models.Model):
         "A save method that modifies the data in the object"
         self.data = 666
         super(ModifyingSaveData, self).save(raw)
+
+# Tests for serialization of models using inheritance.
+# Regression for #7202, #7350
+class AbstractBaseModel(models.Model):
+    parent_data = models.IntegerField()
+    class Meta:
+        abstract = True
+
+class InheritAbstractModel(AbstractBaseModel):
+    child_data = models.IntegerField()
+    
+class BaseModel(models.Model):
+    parent_data = models.IntegerField()
+
+class InheritBaseModel(BaseModel):
+    child_data = models.IntegerField()
+
+class ExplicitInheritBaseModel(BaseModel):
+    parent = models.OneToOneField(BaseModel)
+    child_data = models.IntegerField()

tests/regressiontests/serializers_regress/tests.py

@@ -32,7 +32,7 @@ def data_create(pk, klass, data):
     instance = klass(id=pk)
     instance.data = data
     models.Model.save_base(instance, raw=True)
-    return instance
+    return [instance]
 
 def generic_create(pk, klass, data):
     instance = klass(id=pk)
@@ -40,32 +40,45 @@ def generic_create(pk, klass, data):
     models.Model.save_base(instance, raw=True)
     for tag in data[1:]:
         instance.tags.create(data=tag)
-    return instance
+    return [instance]
 
 def fk_create(pk, klass, data):
     instance = klass(id=pk)
     setattr(instance, 'data_id', data)
     models.Model.save_base(instance, raw=True)
-    return instance
+    return [instance]
 
 def m2m_create(pk, klass, data):
     instance = klass(id=pk)
     models.Model.save_base(instance, raw=True)
     instance.data = data
-    return instance
+    return [instance]
 
 def o2o_create(pk, klass, data):
     instance = klass()
     instance.data_id = data
     models.Model.save_base(instance, raw=True)
-    return instance
+    return [instance]
 
 def pk_create(pk, klass, data):
     instance = klass()
     instance.data = data
     models.Model.save_base(instance, raw=True)
-    return instance
+    return [instance]
 
+def inherited_create(pk, klass, data):
+    instance = klass(id=pk,**data)
+    # This isn't a raw save because:
+    #  1) we're testing inheritance, not field behaviour, so none
+    #     of the field values need to be protected.
+    #  2) saving the child class and having the parent created
+    #     automatically is easier than manually creating both. 
+    models.Model.save(instance)
+    created = [instance]
+    for klass,field in instance._meta.parents.items():
+        created.append(klass.objects.get(id=pk))
+    return created
+    
 # A set of functions that can be used to compare
 # test data objects of various kinds
 def data_compare(testcase, pk, klass, data):
@@ -94,6 +107,11 @@ def pk_compare(testcase, pk, klass, data):
     instance = klass.objects.get(data=data)
     testcase.assertEqual(data, instance.data)
 
+def inherited_compare(testcase, pk, klass, data):
+    instance = klass.objects.get(id=pk)
+    for key,value in data.items():
+        testcase.assertEqual(value, getattr(instance,key))
+    
 # Define some data types. Each data type is
 # actually a pair of functions; one to create
 # and one to compare objects of that type
@@ -103,6 +121,7 @@ fk_obj = (fk_create, fk_compare)
 m2m_obj = (m2m_create, m2m_compare)
 o2o_obj = (o2o_create, o2o_compare)
 pk_obj = (pk_create, pk_compare)
+inherited_obj = (inherited_create, inherited_compare)
 
 test_data = [
     # Format: (data type, PK value, Model Class, data)
@@ -255,6 +274,10 @@ The end."""),
 
     (data_obj, 800, AutoNowDateTimeData, datetime.datetime(2006,6,16,10,42,37)),
     (data_obj, 810, ModifyingSaveData, 42),
+    
+    (inherited_obj, 900, InheritAbstractModel, {'child_data':37,'parent_data':42}),
+    (inherited_obj, 910, ExplicitInheritBaseModel, {'child_data':37,'parent_data':42}),
+    (inherited_obj, 920, InheritBaseModel, {'child_data':37,'parent_data':42}),
 ]
 
 # Because Oracle treats the empty string as NULL, Oracle is expected to fail
@@ -277,13 +300,19 @@ def serializerTest(format, self):
 
     # Create all the objects defined in the test data
     objects = []
+    instance_count = {}
     transaction.enter_transaction_management()
     transaction.managed(True)
     for (func, pk, klass, datum) in test_data:
-        objects.append(func[0](pk, klass, datum))
+        objects.extend(func[0](pk, klass, datum))
+        instance_count[klass] = 0
     transaction.commit()
     transaction.leave_transaction_management()
 
+    # Get a count of the number of objects created for each class
+    for klass in instance_count:
+        instance_count[klass] = klass.objects.count()
+        
     # Add the generic tagged objects to the object list
     objects.extend(Tag.objects.all())
 
@@ -304,6 +333,11 @@ def serializerTest(format, self):
     for (func, pk, klass, datum) in test_data:
         func[1](self, pk, klass, datum)
 
+    # Assert that the number of objects deserialized is the
+    # same as the number that was serialized.
+    for klass, count in instance_count.items():
+        self.assertEquals(count, klass.objects.count())
+
 def fieldsTest(format, self):
     # Clear the database first
     management.call_command('flush', verbosity=0, interactive=False)