Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError thrown when serializing UUIDField #9

Open
shydefoo opened this issue Mar 14, 2020 · 12 comments
Open

TypeError thrown when serializing UUIDField #9

shydefoo opened this issue Mar 14, 2020 · 12 comments

Comments

@shydefoo
Copy link

Issue rendering UUID field as foreign key

  • TypeError encountered when serializing certain models
  • Category objects could be rendered successfully
  • Photo objects throw TypeError when using UJSONRenderer
  • JSONRenderer works fine

Output from shell

>>> p = Photo.objects.all()[0]
>>> p
<Photo: LE123: Photo_L*>
>>> s = PhotoSerializer(p)
>>> s.data
{'id': 4875, 'name': 'LE123 Photo_L.jpg', 'image': '/images/LE123_Photo_L.jpg', 'product': UUID('ffdce82f-f094-499a-b221-2b49ec9c36ff')}
>>> UJSONRenderer().render(s.data)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python3.6/site-packages/drf_ujson/renderers.py", line 42, in render
    indent=indent or 0,
TypeError: �1a-�� is not JSON serializable

>>> from rest_framework.renderers import JSONRenderer
>>> JSONRenderer().render(s.data)
b'{"id":4875,"name":"LE123 Photo_L.jpg","image":"/images/LE123_Photo_L.jpg","product":"ffdce82f-f094-499a-b221-2b49ec9c36ff"}'

Models & Serializers

class Category(OrderedModel):
    category = models.CharField(max_length=50)
    order = models.PositiveIntegerField(_("order"), editable=False, db_index=True)

class Product(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    ...
    category = models.ManyToManyField(Category)

class Photo(models.Model):
    name = models.CharField(max_length=500, null=True)
    image = models.ImageField(null=True)
    product = models.ForeignKey(Product, on_delete=models.SET_NULL, null=True)

class PhotoSerializer(serializers.ModelSerializer):
    class Meta:
        model = Photo
        fields = "__all__"


class ProdSerializer(serializers.ModelSerializer):
    category = serializers.StringRelatedField(many=True)
    photo_set = PhotoSerializer(many=True)

    class Meta:
        model = Product
        fields = "__all__"


class CategorySerializer(serializers.ModelSerializer):
    class Meta:
        model = Category
        fields = "__all__"
  • Suspect it might be due to the UUID reference in the Photo object?
  • Any suggestions on how to resolve this would be great
@Amertz08
Copy link
Owner

I've ran into this issue myself. I'd consider explicitly declaring a serialize on ProdSerializer to handle the uuid.

uuid = serializers.CharField()

@shydefoo
Copy link
Author

Hmm, this seems to be a temp fix. It would be great if the UUID Fields could be rendered just like how JSONRenderer handles them.

@Amertz08
Copy link
Owner

Yeah so it looks like the built in json module supports passing an encoder class which is what the base JSONRenderer class takes advantage of in order to handle UUIDs. It looks like ujson is adding support for it [1]. So I don't think I can do anything particularly clean until then. I did post a ticket to the repo to suggest supporting UUIDs directly but it doesn't look like that will be implemented [2].

[1] ultrajson/ultrajson#124
[2] ultrajson/ultrajson#385

@Amertz08
Copy link
Owner

Amertz08 commented Mar 15, 2020

I think the closest thing to a fix at the renderer level would be something like follows.

class UJSONRenderer:

    translate_func = lambda v: v

    def render(self, ...):
        # ...
        ret = ujson.dumps(self.translate_func(data), ...)

Where you can write a function that translates your response into a ujson serializable object. There would be performance implications obviously that should be explored.

@Amertz08
Copy link
Owner

I might implement this in an alpha release and see how it goes.

@aaronn
Copy link

aaronn commented Aug 17, 2020

Would love to see this– I'm using UUIDs for all of my PKs. Doesn't sound like ujson is ever going to prioritize either custom encoders or support uuid serialization directly.

@Amertz08
Copy link
Owner

@aaronn The last time I looked (haven't in a while) at ujson they were going to add support for custom encoders. So my thought was wait to see what that interface looks like. I'll have to look into what their timeline is now. Last I checked they basically said if json out of the box doesn't support it then they won't either so encoding UUID objects directly is a no go. The custom encoders were thus their path forward (to my understanding).

@aaronn
Copy link

aaronn commented Aug 17, 2020

Just checked the issue and they've removed it from the milestone. It doesn't look like something they have any plans to prioritize.

@Amertz08
Copy link
Owner

Alright I'll put something out on this. I have had time for this I just haven't made time.

@Amertz08
Copy link
Owner

For this to work you'll have to implement a recursive function that will traverse the various structures and types. I think the recursion will really matter around dict/list. This was the reason I was so apprehensive about implementing it. That being said should be available via pip install drf_ujson2==1.7.0a1

@Amertz08
Copy link
Owner

I actually think I have the typing wrong on data: Union[dict, None], in the def render call. That might need to be Any or Union[Dict, List, None] not sure what makes sense.

@Amertz08
Copy link
Owner

Amertz08 commented Aug 17, 2020

Something like this.

def translate(data: Any):
    if isinstance(data, list):
        return [translate(d) for d in data]
    if isinstance(data, dict):
        return {key: translate(value) for key, value in data.items()}
    if isinstance(data, uuid.UUID):
        return str(data)
    return data

could be wrong though 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants