Opened 7 years ago

Closed 18 months ago

#12685 closed New feature (fixed)

Serialized objects' save function does not respect force_insert

Reported by: dcotruta Owned by: Russell Keith-Magee <russell@…>
Component: Core (Serialization) Version: 1.1
Severity: Normal Keywords: save, force_insert, force_update
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Russell Keith-Magee)

Given a serialized trivial object, of the type models.Model, which does not override any functions, a deserialized object does not accept the force_insert keyword argument for its save function.

class XYZ(models.Model):
    xyz_creation_date = models.DateTimeField(verbose_name=_("Date Opened"),
                                         blank=False,
                                         auto_now_add=True,
                                         editable=False
                                         )
    
    xyz_is_open = models.BooleanField(default=True)
    
    xyz_last_updated = models.DateTimeField(verbose_name=_("Last Report"),
                                        blank=False,
                                        auto_now=True
                                        )
    
    xyz_synced = models.BooleanField(default=False)

If this model exists on two separate servers, and instances of this object are serialized, passed over the wire, and then deserialized, the save function on the receiving server cannot pass force_insert=True - doing so yields "save() got an unexpected keyword argument 'force_insert'".

Any ideas?

Change History (20)

comment:1 Changed 7 years ago by Russell Keith-Magee

Description: modified (diff)
Resolution: invalid
Status: newclosed

Trac is not for asking how-to questions. If you want to know how to do something, ask on django-users.

comment:2 Changed 7 years ago by anonymous

Resolution: invalid
Status: closedreopened

Erm, I'm not asking a question, I'm stating a fact.
http://docs.djangoproject.com/en/dev/ref/models/instances/#saving-objects states very clearly the function signature as:
Model.save([force_insert=False, force_update=False, using=DEFAULT_DB_ALIAS])

Deserialized objects of the type I've described above, which DO NOT override the save behavior, don't accept the force_insert keyword argument. This is not a question - it's a bug.

Unless I'm missing something, there should be no difference in the behavior of save for deserialized objects.

comment:3 Changed 7 years ago by dcotruta

Has patch: set
Needs tests: set

The problem is that the call to save_base in core/serializers/base.py within the class DeserializedObject does not have the capability to carry through the correct signature.

It's simple to fix:

class DeserializedObject(object):[[BR]]
...

    def save(self, save_m2m=True, force_insert=False, force_update=False):
        # Call save on the Model baseclass directly. This bypasses any
        # model-defined save. The save is also forced to be raw.
        # This ensures that the data that is deserialized is literally
        # what came from the file, not post-processed by pre_save/save
        # methods.
       
        # dcotruta - changed signature to handle force_insert and force_update

        models.Model.save_base(self.object, raw=True, force_insert=force_insert, force_update=force_update)
...

I'll provide a patch tonight, when I get round to it.

russellm - next time actually read the problem - I'm not trying to be rude, but this kind of attitude is not helpful in open source!

comment:4 Changed 7 years ago by dcotruta

Has patch: unset

comment:5 Changed 7 years ago by dcotruta

Component: Database layer (models, ORM)Serialization
milestone: 1.2

Fixed component to show this is a serialization, not models, bug.

comment:6 Changed 7 years ago by dcotruta

Keywords: save force_insert force_update added

comment:7 Changed 7 years ago by dcotruta

Triage Stage: UnreviewedAccepted

comment:8 in reply to:  3 ; Changed 7 years ago by Ramiro Morales

Triage Stage: AcceptedUnreviewed

Replying to dcotruta:

The problem is that the call to save_base in core/serializers/base.py within the class DeserializedObject does not have the capability to carry through the correct signature.

Exactly, and this (the existence of DeserializedObject, the fact that its save method isn't the same as the Model save method, and that you need to provide your own logic to decide if a DeserializedObject instance should be saved or not) is intended and documented (in the documentation that should be read in first place: The one about (de)serializing) behavior:

http://docs.djangoproject.com/en/1.1/topics/serialization/#deserializing-data

comment:9 in reply to:  8 Changed 7 years ago by dcotruta

I do see what you're trying to say, but you must admit that in many cases serialization is used to carry out trivial synchronization. In those cases, it makes sense to be able to implement the normal save behavior - indeed it would be doubly useful to be able to use new attributes like force_insert since they would throw pk errors "for free" - a very easy way of preventing data overwrites.

Just because you can create your own save function doesn't mean the built in one should fail to support standard attributes available everywhere else.
I'm not submitting this bug just for the fun of it, but because force_insert is a rather unusual call which happens to be very useful when handling over-the-wire object movement.

comment:10 Changed 7 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:11 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:12 Changed 6 years ago by Matt McClanahan

Severity: Normal
Type: New feature

comment:13 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:14 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:15 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:16 Changed 18 months ago by Michael Thornhill

Owner: changed from nobody to Michael Thornhill
Status: newassigned

comment:17 Changed 18 months ago by Michael Thornhill

Added test case and fix here: https://github.com/django/django/pull/4811

comment:18 Changed 18 months ago by Michael Thornhill

Has patch: set
Needs tests: unset

comment:19 Changed 18 months ago by Michael Thornhill

Owner: Michael Thornhill deleted
Status: assignednew

comment:20 Changed 18 months ago by Russell Keith-Magee <russell@…>

Owner: set to Russell Keith-Magee <russell@…>
Resolution: fixed
Status: newclosed

In 9851e541:

Merge pull request #4811 from mthornhill/12685

Fixed #12685 - Ensure that deserialised model instances honor the same arguments as normal models.

Note: See TracTickets for help on using tickets.
Back to Top