Code

Opened 7 years ago

Closed 7 years ago

#3564 closed (wontfix)

flexible JSON serializing mechanism

Reported by: boxed@… Owned by: jacob
Component: Core (Serialization) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

JSON serialization is rather awkward in django, since you either have to extract all the data in the view code och make your own JSON encoder. I suggest adding a new class DjangoJSONEncoder (which extends DateTimeAwareJSONEncoder) that will do essentially this:

def default(self, o):

try:

return o.json_serialize()

except AttributeError:

return super(self, DateTimeAwareJSONEncoder).default(o)

This way I can put the JSON serialization where it belongs: with my models.

Attachments (0)

Change History (10)

comment:1 Changed 7 years ago by boxed@…

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

After some more research (and now some working code for this) I came to the conclusion that what is needed is something like this:

class DjangoJSONEncoder(DateTimeAwareJSONEncoder):
    """
    DateTimeAwareJSONEncoder subclass that knows how to forward encoding to the object if it has the method json_serialize()
    """
    
    def encode(self, o):
        from django.db.models.query import QuerySet
        if isinstance(o, QuerySet):
            o = list(o)
        return super(DjangoJSONEncoder, self).encode(o)

    def default(self, o):
        try:
            return o.json_serialize()
        except AttributeError:
            return super(DjangoJSONEncoder, self).default(o)

This still leads to some rather messy code in the model, but it is at least an improvement. A better system would be for a marker class like Admin, so you can do "class JSON:pass" and then you get some reasonable default for these things.

comment:2 Changed 7 years ago by mtredinnick

I think there's a conceptual problem here: json serialization isn't something that should be part of a model. It's a presentation data-munging feature, so it's very much view-layer, rather than intrinsic to the model. So something like a JSON inner-class in models isn't going to fly.

I'm not completely clear on what problem you are trying to address with this ticket. Given that you are always going to have to specify which data to serialise and that will be done most naturally via a queryset, we currently have it down to a call to something like serializers.serialize("json", my_queryset). What situation do you have where that isn't doing the right thing? A (short) example of where that gets into trouble and is made easier by your suggestion would probably help here.

comment:3 Changed 7 years ago by boxed@…

Hmm, I see your point, but there is still some fuglyness in the serializing. What I want to do in this specific case is pass a year, month and a queryset. It would be really nice if I could just do: serializers.serialize("json", {'year':year, 'month':month, 'events':events}). This will raise a AttributeError: 'str' object has no attribute '_meta'. It seems to me that objects that are supported by the serializer should just be passed through and not blindly attempted to be read as django models and there should be some recursion or something to handle when the input parameter has a QuerySet inside it. Serializing two QuerySets is also a case that the current system doesn't seem to handle in a straight forward manner.

When reading the code for serializers.serialize() it becomes clear that it really is only intended to serialize single QuerySets. The name is slightly misleading for this in my opinion. I don't really know how to solve this problem in a clean way with the current code.

comment:4 Changed 7 years ago by mtredinnick

The reason core.serialization only worries about QuerySets is because you can already serialize other stuff directly using simplejson. Just import utils.simplejson and go to work there. Normal Python structures are already supported; the only hard bit was the results of queries.

As far as serializing multiple queries goes, since the result of a serialize() is just a list of dictionaries, you can serialise things one at a time and concatenate the lists without any real effort. It's not really worth adding extra syntax for that (and one day, unions of QuerySet instances will work properly, so it will be possible to do that as well).

I think the only thing we can do here is add a bit more to the documentation. We don't want to duplicate the stuff that simplejson already does so well -- only add the necessary bits for Django-specifics. I agree, it's not immediately obvious how to do some of the above, but you haven't described anything that isn't already possible in only a line or two of code, which is about as minimal as it gets.

comment:5 Changed 7 years ago by boxed@…

You are mistaken about the return type of serialize, it is a string, which makes building up a bigger structure and using dumps() not as nice as you suggest. If it returned dictionaries I agree with you that it would have been simple. As it stands now I'm contemplating a really ugly hack with string replacing :/

I don't really think we're talking about adding syntax here, just changing the existing to follow the principle of least surprise.

comment:6 Changed 7 years ago by boxed@…

The problem is "solved" by the following code: return HttpResponse(dumps({'year':year, 'month':month, 'events':'<replace>'}, ensure_ascii=False).replace('"<replace>"', json), mimetype="text/json; charset=UTF-8") Note the extra quotation marks in the replace call.. this is not on par with the rest of Django in maintainability :P

comment:7 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

OK, I'd not noticed that the return type was a string. That does make it necessary to add in something extra to support concatenation of results, at least.

comment:8 follow-up: Changed 7 years ago by SmileyChris

Just to correct one of boxed's comments, serializing two QuerySets is easy. Just convert both to lists, concatenate, then pass that concatenated list to serialize.

comment:9 in reply to: ↑ 8 Changed 7 years ago by Michael Radziej <mir@…>

Replying to SmileyChris:

Just to correct one of boxed's comments, serializing two QuerySets is easy. Just convert both to lists, concatenate, then pass that concatenated list to serialize.

Or use itertools.chain(qset1, qset2)

comment:10 Changed 7 years ago by russellm

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

The end point of this discussion seems to be that the ticket is requesting a way to easily serialize multiple query sets in a single output. As noted by Michael and SmileyChris, this doesn't require any changes to the serializer - just pass in a concatenated/chained lists in your call to serialize.

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.