Opened 4 years ago

Last modified 13 months ago

#12685 new New feature

Serialized objects' save function does not respect force_insert

Reported by: dcotruta Owned by: nobody
Component: Core (Serialization) Version: 1.1
Severity: Normal Keywords: save, force_insert, force_update
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by russellm)

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"),
    xyz_is_open = models.BooleanField(default=True)
    xyz_last_updated = models.DateTimeField(verbose_name=_("Last Report"),
    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?

Attachments (0)

Change History (15)

comment:1 Changed 4 years ago by russellm

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

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

comment:2 Changed 4 years ago by anonymous

  • Resolution invalid deleted
  • Status changed from closed to reopened

Erm, I'm not asking a question, I'm stating a fact. states very clearly the function signature as:[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 follow-up: Changed 4 years ago by dcotruta

  • Has patch set
  • Needs tests set

The problem is that the call to save_base in core/serializers/ 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 4 years ago by dcotruta

  • Has patch unset

comment:5 Changed 4 years ago by dcotruta

  • Component changed from Database layer (models, ORM) to Serialization
  • milestone set to 1.2

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

comment:6 Changed 4 years ago by dcotruta

  • Keywords save, force_insert, force_update added

comment:7 Changed 4 years ago by dcotruta

  • Triage Stage changed from Unreviewed to Accepted

comment:8 in reply to: ↑ 3 ; follow-up: Changed 4 years ago by ramiro

  • Triage Stage changed from Accepted to Unreviewed

Replying to dcotruta:

The problem is that the call to save_base in core/serializers/ 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:

comment:9 in reply to: ↑ 8 Changed 4 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 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:11 Changed 4 years ago by ubernostrum

  • milestone 1.2 deleted

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

comment:12 Changed 3 years ago by mattmcc

  • Severity set to Normal
  • Type set to New feature

comment:13 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:14 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:15 Changed 13 months ago by aaugustin

  • Status changed from reopened to new

Add Comment

Modify Ticket

Change Properties
<Author field>
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'

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

Note: See TracTickets for help on using tickets.