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

Does DRF support nested writable serializer with multipart/form-data? #7262

Closed
4 of 5 tasks
mohmyo opened this issue Apr 8, 2020 · 16 comments
Closed
4 of 5 tasks

Does DRF support nested writable serializer with multipart/form-data? #7262

mohmyo opened this issue Apr 8, 2020 · 16 comments

Comments

@mohmyo
Copy link
Contributor

mohmyo commented Apr 8, 2020

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.

Hello,

I'm trying to upload a file with some nested data via POST and content-type:multipart/form-data, no matter how I'm sending the value for the nested field as a text or JSON my serializer always rasing the exception for the nested field as This field is required despite I'm sure it is being correctly mapped to request.data - fields and values are as expected - and passed to the parent serializer, I have even tried to write a parser or to use a package but nothing changes, for some reason the nested field is always dropped at validation.

@xordoquy
Copy link
Collaborator

xordoquy commented Apr 8, 2020

TL;DR: No, DRF doesn't support nested writable serializers with multipart/form-data

Nested serializers that are flagged many=True are not working with multipart/form-data mostly because there's no standard notation about how a field name should looks like.
I, once, worked on it but got burnt out and lost that work.
Most of the work to do should be within https://github.com/encode/django-rest-framework/blob/master/rest_framework/utils/html.py

@xordoquy xordoquy closed this as completed Apr 8, 2020
@tim-mccurrach
Copy link
Contributor

@xordoquy I'd be very interested to hear the non-TLDR version of your comment above.

Serializers get_value method seems to be over-written with exactly this purpose in mind:

    def get_value(self, dictionary):
        # We override the default field access in order to support
        # nested HTML forms.
        if html.is_html_input(dictionary):
            return html.parse_html_dict(dictionary, prefix=self.field_name) or empty
        return dictionary.get(self.field_name, empty)

Is this just a left-over relic from the past? Looking at parse_html_list I can see what you mean, that particular naming conventions are required to make it work. From a not-very-thorough-look, it seems like over-writing get_value in your nested serializer is all that is required though. Is there anything more I'm not thinking about?

If all that is needed is to over-write the get_value method, would it be worth adding a note about this in the documentation. If so, I'll happily submit a PR.

Thanks

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 2, 2020

@tim-mccurrach the code should work with a nested "single" serializer - ie without many=True. That said, I'll assume by nested serializers it'll mean those with many=True hereafter.
Note that to make nested serialisers work, it requires a lot of work on the BrowsableAPI.
The HTMLRenderer is a step forward and you'd still have some JS to write to delete or add new nested.
Also, you should not forget that nested serializers can go beyond a single level.

class A(Serializer):
    pass

class B(Serializer):
    nested_a = A(many=True)

class C(Serializer):
    nested_b = B(many=True)

This would give something like c.b[0].a[0].value = XXX which parse_html_dict and parse_html_list are -currently- unable to parse, not to mention the headache if c.b[0].a[0].value is a multi value field.

It's a really tough topic to tackle and quite very little traction to get it working.

@tim-mccurrach
Copy link
Contributor

@xordoquy Thanks for the detailed (and speedy) reply :)

I wasn't really thinking about the BrowsableAPI, but what you've made written makes a lot of sense.

@zfedoran
Copy link

@tim-mccurrach and anyone else stuck on this... you can technically get around the issue by sending JSON.stringify({ ... }) version of the nested object list in the FormData. Then, you'll need to do is add a write_only=true field to the parent serializer. Optionally, add a validator to check the nested values.

For example:

class A(Serializer):
    pass

class B(Serializer):
    nested_a = A(many=True, read_only=True)
    nested_a_json = JsonField(write_only=True)

    def validate_nested_a_json(self, value):
        if not isinstance(value, list):
            ValidationError("nested_a_json expects a list")

        for item in value:
            serializer = A(data=item)
            serializer.is_valid(raise_exception=True)
        return value

@xordoquy I'd love for the framework to detect that the nested_a property has been provided as a stringified version instead of needing to split things out into read_only and write_only then having to handle the nesting manually...

However, for the time being, do you see any issues with the approach above?

@s4ke
Copy link

s4ke commented Jan 29, 2021

We achieved this with a custom Parser that uses https://pypi.org/project/FormEncode/ . The browsable API does not work however as we only need this in a bulk endpoint.

Below is our code, with a dirty hack to detect if files were uploaded:

class MultipartFormencodeParser(parsers.MultiPartParser):

    def parse(self, stream: Any, media_type: Any = None, parser_context: Any = None) -> Dict[str, Any]:
        result = cast(parsers.DataAndFiles, super().parse(
            stream,
            media_type=media_type,
            parser_context=parser_context
        ))

        _data_keys: Set[str] = set(result.data.keys())
        _file_keys: Set[str] = set(result.files.keys())

        _intersect = _file_keys.intersection(_data_keys)
        if len(_intersect) > 0:
            raise ValidationError('files and data had intersection on keys: ' + str(_intersect))

        # merge everything together
        merged = QueryDict(mutable=True)

        merged.update(result.data)
        merged.update(result.files)  # type: ignore

        # decode it together
        decoded_merged = variable_decode(merged)

        parser_context['__JSON_AS_STRING__'] = True

        if len(result.files) > 0:
            # if we had at least one file put everything into files so we
            # later know we had at least one file by running len(request.FILES)
            parser_context['request'].META['REQUEST_HAD_FILES'] = True
            return parsers.DataAndFiles(decoded_merged, {})  # type: ignore
        else:
            # just put it into data, doesnt matter really otherwise
            return parsers.DataAndFiles(decoded_merged, {})  # type: ignore

@maingoh
Copy link

maingoh commented Oct 26, 2021

The following example seems to work for me via API (not html renderer). However it should be sent without . after brackets:

{
    'c[0]b[0]a.text': 'my-text',
    'c[0]b[1]a.text': 'my-text2',
}

Serializers:

class A(serializers.Serializer):
    text = serializers.CharField()

class B(serializers.Serializer):
    a = A()

class C(serializers.Serializer):
    b = B(many=True)

class D(serializers.Serializer):
    c = C(many=True)

    def validate(self, data):
        print(data)
        return data

@auvipy
Copy link
Member

auvipy commented Oct 27, 2021

did you check drf-writable-nested?

@maingoh
Copy link

maingoh commented Oct 27, 2021

No i didn't

@rick2ricks
Copy link

The following example seems to work for me via API (not html renderer). However it should be sent without . after brackets:

{
    'c[0]b[0]a.text': 'my-text',
    'c[0]b[1]a.text': 'my-text2',
}

Serializers:

class A(serializers.Serializer):
    text = serializers.CharField()

class B(serializers.Serializer):
    a = A()

class C(serializers.Serializer):
    b = B(many=True)

class D(serializers.Serializer):
    c = C(many=True)

    def validate(self, data):
        print(data)
        return data

Hi,

Do you know why this works?
When sending an array of strings it only worked for me by sending the same attribute name without brackets and the numeric key.


attr: 'one',
attr: 'two'

@maingoh
Copy link

maingoh commented Jan 5, 2022

I am not sure, it might depend on your serializer ? In your case it might not be a nested serializer but just a ListField ?

class A(serializers.Serializer):
    attr = ListField(CharField())

@rick2ricks
Copy link

Yes, you are right, I was just pointing out this other case when it is sent a list of strings.

@merabtenei
Copy link

merabtenei commented Apr 26, 2022

Adding my contribution into this in case anyone face this issue in the future.
I had the same issue as @mohmyo. I needed to send multipart/form-data with one or many fields that are List of nested objects (Nested serializers that are flagged many=True). As @xordoquy stated this indeed doesn't work and it was quite difficult to get it to work.
The solution that worked for me was to intercept request.data in the Viewset and transform it in a way to make the structure the same as if the data was sent as application/json.

Here is a simple example using Two models with a ManyToMany relation using a through Model.

Models:

class Invoice(models.Model):
    date = models.DateField(default=date.today)
    amount = models.FloatField(default=0)
    image = models.ImageField(upload_to='images/', blank=True, null=True)
    products = models.ManyToManyField('Product', through='InvoiceProduct')

class Product(models.Model):
    name = models.CharField(max_length=200)
    price = models.FloatField(default=0)

class InvoiceProduct(models.Model):
    invoice = models.ForeignKey(Invoice, on_delete=models.CASCADE, related_name='invoiceproduct_set')
    product = models.ForeignKey(Product, on_delete=models.CASCADE)
    quantity = models.IntegerField(default=0)

Serializers:

class ProductSerializer(serializers.ModelSerializer):
    class Meta:
        model = Product
        fields = '__all__'

class InvoiceProductSerializer(serializers.ModelSerializer):
    invoice = serializers.PrimaryKeyRelatedField(read_only=True)
    class Meta:
        model = InvoiceProduct
        fields = '__all__'

class InvoiceSerializer(serializers.ModelSerializer):
    products = InvoiceProductSerializer(source='invoiceproduct_set', many=True)

    @transaction.atomic
    def create(self, validated_data):
         items = validated_data.pop('invoiceproduct_set')
         invoice = self.Meta.model.objects.create(validated_data)
         for item in items:
             item['invoice'] = invoice
             InvoiceProduct.objects.create(**item)

Viewset:

class MultipartNestedSupportMixin:

    def transform_request_data(self, data):
        # transform data sctructure to dictionnary
        force_dict_data = data
        if type(force_dict_data) == QueryDict:
            force_dict_data = force_dict_data.dict()
        

        # transform JSON string to dictionnary for each many field
        serializer = self.get_serializer()
        #print(force_dict_data)

        for key, value in serializer.get_fields().items():
            if isinstance(value, serializers.ListSerializer) or isinstance(value, serializers.ModelSerializer):
                if key in force_dict_data and type(force_dict_data[key]) == str:
                    if force_dict_data[key] == '':
                        force_dict_data[key] = None
                    else:
                        try:
                            force_dict_data[key] = json.loads(force_dict_data[key])
                        except:
                            pass

        return force_dict_data

    def create(self, request, *args, **kwargs):
        force_dict_data = self.transform_request_data(request.data)
        serializer = self.get_serializer(data=force_dict_data)
        serializer.is_valid(raise_exception=True)
        self.perform_create(serializer)
        headers = self.get_success_headers(serializer.data)
        return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

    def update(self, request, *args, **kwargs):
        force_dict_data = self.transform_request_data(request.data)
        partial = kwargs.pop('partial', False)
        instance = self.get_object()
        serializer = self.get_serializer(instance, data=force_dict_data, partial=partial)
        serializer.is_valid(raise_exception=True)
        self.perform_update(serializer)

        if getattr(instance, '_prefetched_objects_cache', None):
            # If 'prefetch_related' has been applied to a queryset, we need to
            # forcibly invalidate the prefetch cache on the instance.
            instance._prefetched_objects_cache = {}

        return Response(serializer.data)

class InvoiceViewSet(MultipartNestedSupportMixin, viewsets.ModelViewSet):
    queryset = Invoice.objects.all()
    serializer_class = InvoiceSerializer

With this you shloud be able to send multipart formdata with files and nested objects as JSON text :
Example:
image

I managed to create a Mixin that can handle the creation of the Nested Objects for any similar use case as above, you can use this :

class WritableNestedThroughMixin():
    @transaction.atomic
    def create(self, validated_data):
        validated_data_no_nested = validated_data.copy()
        for key, value in self.get_fields().items():
            if isinstance(value, serializers.ListSerializer):
                validated_data_no_nested.pop(value.source if value.source is not None else key)

        main_obj = self.Meta.model.objects.create(**validated_data_no_nested)

        for key, value in self.get_fields().items():
            if isinstance(value, serializers.ListSerializer):
                items = validated_data.pop(value.source if value.source is not None else key, None)
                if items is not None:
                    ItemModel = value.child.Meta.model
                    link_field = None
                    for field in ItemModel._meta.fields:
                        if field.related_model == self.Meta.model:
                            link_field = field.name
                    #print('### PARENT FIELD : ', parent_field)
                    if link_field is not None:
                        for item in items:
                            item[link_field] = main_obj
                            ItemModel.objects.create(**item)
        return main_obj

#You can then use :
class InvoiceSerializer(WritableNestedThroughMixin, serializers.ModelSerializer):
    products = InvoiceProductSerializer(source='invoiceproduct_set', many=True)

Hope this helps someone in the future. feel free to make any remarks or suggestions about the code or how to improve things in it.

@faisalakhlaq
Copy link

faisalakhlaq commented Oct 31, 2022

@transaction.atomic

@merabtenei why is this @transaction.atomic? I will try to use the solution and I have similar questions on the stackoverflow question1 and question2 but haven't got any answers to the questions.

My other question is this issue seems to be closed. Is it solved in any other form? Or there is no plan to solve this issue?

Thank you.

@merabtenei
Copy link

@transaction.atomic

This will revert all changes if any exception is raised. Nested objects require many instances to be created, we need to make sure that either all instances are created or none of them.

@ifigueroap
Copy link

I already customized the update method in my viewset, hence I only needed to add transform_request_data as a step previous to instantiating the serializer. Thank you!!!

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

No branches or pull requests