hagsteel / swampdragon

swampdragon
Other
557 stars 73 forks source link

On update action related models are not serialized. #118

Open BuckinghamIO opened 9 years ago

BuckinghamIO commented 9 years ago

I was having a werid issue last night which only happened when updating and I am not sure what is causing it as I swear it worked before.

Basically on create I both my user ForeignKey and my message CharField are serialized as they are both in the updated fields when the serializer is called.

However when updating a object the serializer only recieves the message CharField as the User has not changed but the message has.

The serializer does find the realated User object ID and adds it to the data however only the fields are serialized when added to the data dict meaning that the User object even though is a related model is left which just a ID and none of its publish fields set in the serializer for the User related object.

 class Messages(SelfPublishModel, models.Model):
    serializer_class = MessageSerializer

    user = models.ForeignKey(User)
    message = models.CharField(max_length=100)
    sent = models.DateTimeField(blank=True, null=True)

    def __unicode__(self):
        return '%s' % self.message

from swampdragon.serializers.model_serializer import ModelSerializer

class UserSerializer(ModelSerializer):
    class Meta:
       model = 'accounts.User'
       publish_fields = ('nickname', 'avatar_small')

class MessageSerializer(ModelSerializer):
    user = UserSerializer

    class Meta:
       model = 'chat.Messages'
       publish_fields = ('user', 'message')
       update_fields = ('message',)

And here is the Serializer code which is causing the problem:

def serialize(self, fields=None, ignore_serializers=None):
       if not fields:
            fields = self.opts.publish_fields
       if not self.instance:
            return None

        data = self.get_object_map_data()

        # Set all the ids for related models
        # so the datamapper can find the connection
        data.update(get_id_mappings(self))

        # Serialize the fields
        for field in fields:
            data[field] = self._serialize_value(field, ignore_serializers)

        custom_serializer_functions = self._get_custom_field_serializers()
        for custom_function, name in custom_serializer_functions:
            serializer = getattr(self, name, None)
            if serializer:
                serializer = get_serializer(serializer, self)
                data[name] = custom_function(self.instance, serializer)
            else:
                data[name] = custom_function(self.instance)

        return data

I am not sure if this is a bug or a issue cause by my settings however I couldn't see how I could have caused this so I made a temp patch by adding this:

    ##Get related model objects even if not in changed fields but in publisher_fields
    for related in data:
        if related not in fields:
            data[related] = self._serialize_value(related, ignore_serializers)

It's not perfect and can certaintly be improved however It does work for now until you can confirm its a bug and not just my fault.

hagsteel commented 9 years ago

The issue here is that if you get all the related objects all the time you can end up with lots of big queries. Instead it will publish the fields that have changed on the model, but rather than pulling out the user from the db and serialising that user it will simply include the ID property that is already available on the model.

This is to prevent having lots of trips to the database.

BuckinghamIO commented 9 years ago

This strategy while I see why, forces the client to think that any atrribute inside the user foreign key has been removed.

hagsteel commented 9 years ago

I am unsure if the self publishing model is something that should even exist. There seem to be too many edge cases and it's too easy to end up with code that is not optimised.

Write your own publisher is the best choice.

The drawback: Not as easy as hitting save on the model The upside: you decide when and what to publish

This way if you need to serialise related data you can, and if you don't you don't have to. If you do most of your work via the routers there is the ModelPubRouter which will publish for you: http://swampdragon.net/documentation/routers-base-model-publisher-router/

BuckinghamIO commented 9 years ago

I understand that completly and if I ever see my self in a situation where I need to I know I can because you developed the framework in such a way its possible which is great and the percise control we have is good incase we dont agree with your methods like right now.

However what I am saying is the self publishing model is a self publishing model and therefor can compromise a little especially if it means hitting the database a few more times to ensure that the out of the box method that you provide works properly in all situations and if people want to optimize they can still but thats their choice.

oleksii-sl commented 9 years ago

Hello, I also met this issue, the idea to send only what was changed is ok, but it'll be cool to know on client side what was changed (in order to merge only updated data on client side). Is it possible to implement such feature?