Code

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#11486 closed (fixed)

There are no way to provide empty (or null/None) "pk" for XML serialized object to deserialized

Reported by: zdmytriv Owned by: niall
Component: Core (Serialization) Version: master
Severity: Keywords: xml, deserialization
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I've serialized my django model:

    serializers.serialize(MyModel.objects.filter(color="Red"))

and got this output:

    <object model="example.example" pk="133">
    	<field name="name" type="CharField">John Smith</field>
    	<field name="color" type="CharField">Red</field>
        ... #more fields
    </object>

So you can see I have pk="133":

And now I want to deserialize this into django model again and save() into database but with different pk so it should create new record with new id.

I'm trying to parse XML and change pk using:

  • pk="" - parser complains that pk should be integer
  • pk="-1" or "0" - actually creates record with id/pk = "1" or "0"
  • pk="None" or None or "null" - parser complains that pk should be integer
  • remove "pk" attribute - parser complains that attribute is mandatory

In Django http://www.djangoproject.com/documentation/0.96/models/serializers/ article there is an example how to deserialize from JSON with null "pk".

    # You can easily create new objects by deserializing data with an empty PK
    # (It's easier to demo this with JSON...)
    >>> new_author_json = '[{"pk": null, "model": "serializers.author", "fields": {"name": "Bill"}}]'
    >>> for obj in serializers.deserialize("json", new_author_json):
    ...     obj.save()

(It's actually for 0.96, but I'm assuming it should work for 1.* also)

So in JSON pk can be null but in XML it complains.

Looks like bug. There are no way to provide empty (or null/None) "pk" for XML serialized object.

From django/core/serializers/xml_serializer.py:

    class Deserializer(base.Deserializer):
        ...
        def _handle_object(self, node):
    	...
            pk = node.getAttribute("pk")
            if not pk:
                raise base.DeserializationError("<object> node is missing the 'pk' attribute")
    
            data = {Model._meta.pk.attname : Model._meta.pk.to_python(pk)}
    	...

If pk attribute is missing - exception is raised. So we have to provide some pk.

From django/models/fields/init.py

    class AutoField(Field):
        ...
        def to_python(self, value):
            if value is None:
                return value
            try:
                return int(value)
            except (TypeError, ValueError):
                raise exceptions.ValidationError(
                    _("This value must be an integer."))
        ...

If pk is not integer - also exception.

It looks like there are no way to provide empty(null/Node) pk.

Attachments (2)

remove_doctests.diff (24.2 KB) - added by niall 4 years ago.
Replaces the existing doctests with unittests.
xml_serializer_fix.diff (3.9 KB) - added by niall 4 years ago.
Fixes the ticket and adds an explicit test for serialized strings without PKs.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by zdmytriv

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Posible solution:

Don't complain if pk is missing or if pk="None"

In django/core/serializers/xml_serializer.py:

    class Deserializer(base.Deserializer):
        ...
        def _handle_object(self, node):
    	...
            pk = node.getAttribute("pk")
         ---if not pk:
         ---    raise base.DeserializationError("<object> node is missing the 'pk' attribute")
         +++if pk == "None" or not pk:
         +++    pk = None
            data = {Model._meta.pk.attname : Model._meta.pk.to_python(pk)}
    	...

comment:2 Changed 5 years ago by russellm

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

This is an interesting omission from the XML serializer. However, your proposed solution won't work - what happens when you have an object with a string primary key and you want to use a primary key value of "None"? Using this approach, there is no difference between pk=None and pk="None".

In this case, I suspect the right approach is to change the parser so that the pk attribute isn't mandatory, and then interpret the absence of the pk attribute as a null PK value.

From a procedural point of view, any patch would also require tests before it would be accepted for trunk.

comment:3 Changed 5 years ago by zdmytriv

I come up with workaround like this:

  1. Set pk="999999" (some temporary integer value). So my serialized model looks like:
    <object model="example.example" pk="999999">
    	<field name="name" type="CharField">John Smith</field>
    	<field name="color" type="CharField">Red</field>
        ... #more fields
    </object>
  1. During iteration set id and pk to None and later save()
      mymodels_iterator = serializers.deserialize("xml", fixed_pk_serialized_xml_model)

      for mymodel in mymodels_iterator:
          mymodel.object.id = None
          mymodel.object.pk = None
          mymodel.save()

@russellm you are right about string pk. pk="None" wouldn't work. pk should not be mandatory.

It's not a patch but suggestion:

    class Deserializer(base.Deserializer):
        ...
        def _handle_object(self, node):
    	...
            pk = node.getAttribute("pk")
            if not pk:
         ---    raise base.DeserializationError("<object> node is missing the 'pk' attribute")
         +++    pk = None
            data = {Model._meta.pk.attname : Model._meta.pk.to_python(pk)}
    	...

It doesn't look that this will brake something because methods _handle_object(self, node) in Deserializer in serializers/xml_serializer.py and method Deserializer in serializers/python.py looks very similar. And Python and JSON serializers accepts None as pk.
But it has to be tested anyway.

comment:4 Changed 5 years ago by Alex

  • milestone 1.0.3 deleted

Remove inappropriate milestone.

comment:5 Changed 4 years ago by anonymous

  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:6 Changed 4 years ago by niall

  • Owner changed from anonymous to niall
  • Status changed from assigned to new

Changed 4 years ago by niall

Replaces the existing doctests with unittests.

Changed 4 years ago by niall

Fixes the ticket and adds an explicit test for serialized strings without PKs.

comment:7 Changed 4 years ago by niall

  • Has patch set
  • Needs tests unset
  • Version changed from 1.0 to SVN

The patch that converts the doctests to unittests contains a single failing test (caused by the problem described in the ticket).
The second diff fixes this by patching xml_serializers.py to handle cases where the pk attribute is not defined on an object node. In those cases the model is created with pk=None. The second diff also contains tests to explicitly cover this case for xml, json and yaml.

comment:8 Changed 4 years ago by niall

  • Patch needs improvement unset

comment:9 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [13862]) Fixed #11486 -- Corrected the XML serializer to allow for the serialization of objects with a null PK value. Also includes migration of doctests to unittests (we have always been at war with doctests). Thanks to zdmytriv for the report, and Niall Kelly for the patch.

comment:10 Changed 4 years ago by russellm

(In [13863]) [1.2.X] Fixed #11486 -- Corrected the XML serializer to allow for the serialization of objects with a null PK value. Also includes migration of doctests to unittests (we have always been at war with doctests). Thanks to zdmytriv for the report, and Niall Kelly for the patch.

Backport of r13862 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.