Convert ListField error indexes to str

[sigvef]

Hi, this pull request is split out from #6423 . I did some more digging -- initially I thought this was direct bug in DRF, but I was able to trace it back to an integration incompatibility with rapidjson. Either way I think it's worth considering, but I can see how there are both arguments for and against doing this :)

Convert ListField error indexes to str

Before this commit, child validation errors returned from ListField
would be held in a dictionary with the list index as an integer key.
This commit changes it so that the key is converted to str before being
used as a key.

The motivation for this is that rapidjson (an alternative python json
package) doesn't allow converting dicts with anything other than str
keys (i.e., it does not do any type conversion). Other json packages
(json, ujson, simplejson) tend to allow this and just do the type
conversion themselves. With this fix, json, ujson and simplejson will
continue to work as before, but rapidjson will become compatible
out-of-the-box without having to process the dictionary before rendering
to json.

One could perhaps argue that this conversion should rather be done in
rapidjson itself, or perhaps in glue/adapter code (e.g. in some
RapidJSONRenderer implementation); but then again, this seems like an
easy fix that hopefully doesn't break much, and saves a potentially
costly tree traversal in the renderer.

Existing tests have been updated to match this change in behavior. I
guess a new test using rapidjson could be useful, but I don't know what
DRF's policy is on integration tests that interface with third-party
libraries is.

[carltongibson]

Hi @sigvef. So how are you using rapidjson? Do you have a custom renderer? I'm inclined to say the code should go in there (or indeed be pushed back upstream...)

Happy to chat about it, but going to close this on that basis. Thanks.

[sigvef]

Happy to chat about it, but going to close this on that basis. Thanks.

Makes sense, no worries.

Hi @sigvef. So how are you using rapidjson? Do you have a custom renderer? I'm inclined to say the code should go in there (or indeed be pushed back upstream...)

Yep, pretty basic custom renderer. The entire implementation:

from rest_framework.renderers import BaseRenderer    
      
import rapidjson    
      
      
class RapidJSONRenderer(BaseRenderer):    
    media_type = 'application/json'    
    format = 'json'    
     
    def render(self, data, media_type=None, renderer_context=None):    
        return rapidjson.dumps(    
            data,    
            ensure_ascii=False,    
            datetime_mode=rapidjson.DM_ISO8601,    
            uuid_mode=rapidjson.UM_CANONICAL,    
            indent=renderer_context.get('indent', None)).encode()    
 

[carltongibson]

Right, so in there. Just a dictionary comprehension and you're done no? (I'd still see what the upstream folks have to say...)

[sigvef]

That would work (at a slight performance cost). We opted for a parallel reimplementation of ListField with this change that we've been running in production for a couple of weeks.

[rpkilby]

Just to add further context, the restriction of string-based object keys is a specification detail of JSON. Other encoding formats may support numeric object keys, so the existing validation behavior is correct. It's up to the renderer in this case to correctly convert the numeric keys to strings. Either, it's the responsibility of rapidjson, some json encoder, or the RapidJSONRenderer.

[rpkilby]

some json encoder

e.g., DRF provides its own encoder for handing non-native JSON types, such as datetimes.

https://github.com/encode/django-rest-framework/blob/master/rest_framework/utils/encoders.py