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

Can not add new items for nested object via PATCH request with "Content-Type" : "multipart/form-data" ! #106

Open
Shrek53 opened this issue Mar 18, 2020 · 12 comments

Comments

@Shrek53
Copy link

Shrek53 commented Mar 18, 2020

I can add new item via patch request with Content-Type application/json

{
    "invoice_services": [
        {
            "id": 17
            "description_of_service": "TV servicing",
            "quantity": "1.00",
            "unit": "h",
            "price": "1000.00",
            "discount_percent": "6.00",
            "vat_percent": "0.00",
            "invoice": 9
        },
        {
            "description_of_service": "AC servicing",
            "quantity": "1.00",
            "unit": "h",
            "price": "1000.00",
            "discount_percent": "6.00",
            "vat_percent": "0.00",
            "invoice": 9
        }
    ]
}

Now If I want to do the same thing with Content-Type multipart/form-data I can't do that

invoice_services[1]description_of_service:TV servicing
invoice_services[1]id:17
invoice_services[1]quantity:1.00
invoice_services[1]unit:h
invoice_services[1]price:1000.00
invoice_services[1]discount_percent:0.00
invoice_services[1]vat_percent:6.00
invoice_services[2]description_of_service:AC servicing
invoice_services[2]quantity:1.00
invoice_services[2]unit:h
invoice_services[2]price:1000.00
invoice_services[2]vat_percent:0.00

Any suggestions ?
@ruscoder

@Shrek53 Shrek53 changed the title How to add new items for nested object with Content-Type multipart/form-data in header ? Can not add new items for nested object via PATCH request with "Content-Type" : "multipart/form-data" ! Apr 5, 2020
@xalien10
Copy link

xalien10 commented May 29, 2020

Sending patch request with Content-Type multipart/form-data can be achieved like below

You can send the value for "invoice_services" field as [{"id": 17,"description_of_service": "TV servicing","quantity": "1.00","unit": "h","price": "1000.00","discount_percent": "6.00","vat_percent": "0.00","invoice": 9},{"description_of_service": "AC servicing","quantity": "1.00","unit": "h","price": "1000.00","discount_percent": "6.00","vat_percent": "0.00", "invoice": 9}]

Here, the whole nested json dict object will be passed as a string value which will contain exact same structure if you send it via Content-Type application/json. It'll be sent as a string value in the backend.

In backend, you need to parse this string value to a list of dict object then you can replace the fields value with the parsed data.

It can be parsed like this,

invoice_services = request.data['invoice_services']
parsed_data = eval(invoice_services)

Now, you need to convert the form-data request to a dict object

request_data = request.data.dict()

And finally, you need to update the invoice_services field's value with the above parsed data

request_data['invoice_services'] = parsed_data

Congratulations!!!

You've got your desired json request data using form-data with file/image which can be sent to serializers as a request data.

@uripeled2
Copy link

@xalien10 I don't understated your solution

@C4UT1ON
Copy link

C4UT1ON commented Aug 30, 2021

I've run into the same issue recently and its was pretty much a major roadblock for the application I'm developing. I hacked together a temporary solution that appears to get the results I want, albeit not in what I consider to be a good way. having said that, I figure that it might help somebody else or perhaps someone will be inspired and come up with an improvement based on what I have. Its not a general use solution as it assumes that the related models use an auto incremented id but again, might be useful to somebody until a better fix is added. I added the following code to my serializer. The differences between my code and the original writable nested serializer code are marked by ###########

`

def delete_reverse_relations_if_need(self, instance, reverse_relations):
    reverse_relations = OrderedDict(
        reversed(list(reverse_relations.items())))

    # Delete instances which is missed in data
    for field_name, (related_field, field, field_source) in \
            reverse_relations.items():
        model_class = field.Meta.model
        
        related_data = self.get_initial()[field_name]

        ####################
        initial_data_pk_list = [obj.get('id') for obj in related_data] #get id of relationships to be maintained or None if its an object to be created
        current_obj_modified_pk_list = list(set([str(obj.id) for obj in getattr(instance, field_source).all()]) - set(initial_data_pk_list)) #get all objects currently attached to the instance that were not in the initial_data_pk_list
        current_obj_modified_pk_list.sort(key=int) #sort the list according the pk
        created_obj_pk_list = []

        for pk in initial_data_pk_list:
            if pk is None: 
                created_obj_pk_list.append(int(current_obj_modified_pk_list.pop(-1))) #for each occurrence of None in the pk list, a related instance was created in this request
        ######################
        # Expand to array of one item for one-to-one for uniformity
        if related_field.one_to_one:
            related_data = [related_data]

        # M2M relation can be as direct or as reverse. For direct relation
        # we should use reverse relation name
        if related_field.many_to_many and \
                not isinstance(related_field, ForeignObjectRel):
            related_field_lookup = {
                related_field.remote_field.name: instance,
            }
        elif isinstance(related_field, GenericRelation):
            related_field_lookup = \
                self._get_generic_lookup(instance, related_field)
        else:
            related_field_lookup = {
                related_field.name: instance,
            }

        current_ids = self._extract_related_pks(field, related_data)
        #####################
        for pk in created_obj_pk_list:
            current_ids.append(pk) #add the keys that were created in this request to the list of pk to be excluded from the delete below
        #####################

        try:
            pks_to_delete = list(
                model_class.objects.filter(
                    **related_field_lookup
                ).exclude(
                    pk__in=current_ids
                ).values_list('pk', flat=True)
            )
            self.perform_nested_delete(pks_to_delete, model_class, instance, related_field, field_source)

        except ProtectedError as e:
            instances = e.args[1]
            self.fail('cannot_delete_protected', instances=", ".join([
                str(instance) for instance in instances]))

`

@xalien10
Copy link

@xalien10 I don't understated your solution

@uripeled2 I just used the string which actually contained string instead of json object in formdata in postman. After receiving the request I was using python's eval to convert the string to appropriate data structure in python.

@ir4y
Copy link
Member

ir4y commented Aug 31, 2021

Hi!

Thank you for raising the issue.
Personally, I don't think that using multipart/form-data with PATCH request is a good idea.
When you are using REST API you usually operate with some nested data structure like JSON rm XML.
multipart/form-data is a flat structure, so it by design doesn't support any nesting.
If there is a spec that defines multipart/form-data behavior for nested objects please add a link. We will appreciate it a lot and update the package to follow standards.
Otherwise, you have to implement your desired behavior in the application layer.

P.S.
@xalien10 @uripeled2

I was using python's eval to convert the string to appropriate data structure in python.

I would like to warn you that it never should be done this way. https://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
Using eval on any user input causes a remote code execution vulnerability. You should use json.loads for data conversion instead of eval.

@C4UT1ON
Copy link

C4UT1ON commented Sep 1, 2021

Personally, I don't think that using multipart/form-data with PATCH request is a good idea.
When you are using REST API you usually operate with some nested data structure like JSON rm XML.

The use-case of form-data type requests in drf is very often that you want to be able to handle sending a file in your request. In order to not have to handle images and data separately, which could dramatically increase the amount of requests your api has to handle, it becomes useful to handle data also in the same request. This isn't ordinarily a problem as Django model serializers support this use out of the box. drf-writable-nested is a package that builds on top of django's default model serializers and allows you to also create AND update related objects in one endpoint. The whole benefit of this is supposed to be to prevent having to make multiple api calls.

Having said that, it seems fairly straightforward that since writable nested fully supports nested created AND update operations for json requests and works perfectly fine for nested create operations for form-data requests, it should also support nested update operations for form-data requests because otherwise, we're back to the issue of having to handle such things using multiple requests to perform what would ideally be done in one. Its unclear what kind of documentation is needed to encourage a package to fully solve the problem it was literally designed to solve. Its not obvious that form-data is designed not to support nesting since all http request clients seem to support support sending form-data in the format list_field[index]nested_field_name:value or field_name.nested_field_name:value I cant think of a good reason why writable nested would intentionally decline to support that.

@xalien10
Copy link

xalien10 commented Sep 1, 2021

Hi!

Thank you for raising the issue.
Personally, I don't think that using multipart/form-data with PATCH request is a good idea.
When you are using REST API you usually operate with some nested data structure like JSON rm XML.
multipart/form-data is a flat structure, so it by design doesn't support any nesting.
If there is a spec that defines multipart/form-data behavior for nested objects please add a link. We will appreciate it a lot and update the package to follow standards.
Otherwise, you have to implement your desired behavior in the application layer.

P.S.
@xalien10 @uripeled2

I was using python's eval to convert the string to appropriate data structure in python.

I would like to warn you that it never should be done this way. https://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
Using eval on any user input causes a remote code execution vulnerability. You should use json.loads for data conversion instead of eval.

As dry-writable-nested doesn't support nested fields update with files from form-data not only in patch but also in put. So, we don't have any choice to do that at this moment. I know there might be problems when the we want to convert a json string using eval if that is not a valid json. Same thing will also happen for json parser also. The main fact is if nested field update was supported by drf-writable-nested via form-data that could solve a lot of headache. Conversion is not an issue in this regard

@ir4y
Copy link
Member

ir4y commented Sep 1, 2021

It is a bit tricky that it seems after a high-level overview.

  1. There's no standard notation about how a field name should look like. So, any user may decide to do resolve this task. There are multiple ways of representing nested data in flat formats like multipart-from data. Each of them has pros and cons. We can't just choose one of them.
  2. When you face nested-nested serializers things become more tricky. Please review this comment Does DRF support nested writable serializer with multipart/form-data? encode/django-rest-framework#7262 (comment)

I think that the best what we can do is to provide some samples of how it could be done on the application layer.

@C4UT1ON @xalien10
I would like to kindly ask you to contribute to the Known problems with solutions sections of the README file.
It would be great to add examples that will help the community to overcome such edge cases.

P.S.
From my experience, Django's FileField is not the best way to store media. Usually, systems use something like S3 to store media. If you are not using clouds, you always can deploy https://min.io/ it provides an S3 compatible API.
So, working with media files in the REST way is usually a two-step process.

  1. Upload a file to external file storage.
  2. Create/update the reference to file at database via REST API call.

@xalien10
Copy link

xalien10 commented Sep 1, 2021

@ir4y I've updated the README.md file and opened a PR.
#148
May be you can check and put some comments on that

@C4UT1ON
Copy link

C4UT1ON commented Sep 2, 2021

It is a bit tricky that it seems after a high-level overview.

  1. There's no standard notation about how a field name should look like. So, any user may decide to do resolve this task. There are multiple ways of representing nested data in flat formats like multipart-from data. Each of them has pros and cons. We can't just choose one of them.

list_field[index]nested_field_name:value and field_name.nested_field_name:value syntax works for representing nested data for django. if one notation had to be chosen, it would be the one that already works with django

  1. When you face nested-nested serializers things become more tricky. Please review this comment Does DRF support nested writable serializer with multipart/form-data? encode/django-rest-framework#7262 (comment)

that is indeed tricky...i'll have to do some more research

@nathanieldwiputra
Copy link

After I did some digging on version 0.7.0, WritableNestedModelSerializer can actually add new instance(s) on PATCH request with form-data Body format. This process is done by update_or_create_reverse_relations() method within NestedUpdateMixin class. But unlike the JSON format counterpart, they're deleted immediately by delete_reverse_relations_if_need() method within the same class. Therefore I inherit a new serializer class from WritableNestedModelSerializer and switch the order when update_or_create_reverse_relations() and delete_reverse_relations_if_need() are executed.

from drf_writable_nested import WritableNestedModelSerializer, NestedUpdateMixin


class NestedFormDataModelSerializer(WritableNestedModelSerializer):
    def update(self, instance, validated_data):
        relations, reverse_relations = self._extract_relations(validated_data)

        # Create or update direct relations (foreign key, one-to-one)
        self.update_or_create_direct_relations(
            validated_data,
            relations,
        )

        # Update instance
        instance = super(NestedUpdateMixin, self).update(
            instance,
            validated_data,
        )

        # The only difference is switch these 2 methods order. Delete existing instance(s) first then update & add later.
        self.delete_reverse_relations_if_need(instance, reverse_relations)
        self.update_or_create_reverse_relations(instance, reverse_relations)

        instance.refresh_from_db()
        return instance

@PriyanshNegi
Copy link

I've come across the same issue when attempting to add new items for a nested object using a PATCH request with "Content-Type" set to "multipart/form-data". After investigating and experimenting, I discovered that the multipart/form-data format doesn't provide a straightforward way to represent nested objects with new items.

One possible workaround for this issue is to modify the API design to accept the nested object's items as individual fields within the request. For example, instead of sending an array of nested object items, you can send the items as separate fields with distinct names, such as "nested_object_1", "nested_object_2", and so on.

On the server side, you can then handle these fields and construct the nested object with the new items programmatically. This approach allows you to work within the constraints of the multipart/form-data format.

However, it's worth noting that if you have control over the API design, it may be more straightforward to use JSON-based payloads instead of multipart/form-data for complex nested objects. JSON provides a more natural representation for nested structures, making it easier to handle updates and additions.

I hope this information helps others facing the same challenge. If anyone has found alternative solutions or workarounds, please feel free to share them as well.

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

7 participants