Skip to content

PUT does not completely replace object #3648

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

Closed
tysonzero opened this issue Nov 17, 2015 · 8 comments
Closed

PUT does not completely replace object #3648

tysonzero opened this issue Nov 17, 2015 · 8 comments

Comments

@tysonzero
Copy link

Currently PUT does not completely replace the object. Optional fields will be left alone, and IMO that is a little strange.

This means that PUT only overwrites data that has been provided, and successfully leaves everything else alone (like a PATCH request), but then it gets mad at you if you don't supply required data. I personally think that not supplying an argument for a field in a PUT request should result in the same thing that you would get by not supplying an argument for a field in a POST request (when creating the object), that is: the default value.

This way we it would be clearly defined that PATCH updates all fields supplied and leaves all other fields alone, and PUT updates ALL fields and leaves none alone.

@xordoquy
Copy link
Collaborator

While this sort of change will highly affect the current behavior, here's an extract from the RFC 5789 defining the PATCH verb:

The difference between the PUT and PATCH requests is reflected in the
way the server processes the enclosed entity to modify the resource
identified by the Request-URI. In a PUT request, the enclosed entity
is considered to be a modified version of the resource stored on the
origin server, and the client is requesting that the stored version
be replaced. With PATCH, however, the enclosed entity contains a set
of instructions describing how a resource currently residing on the
origin server should be modified to produce a new version. The PATCH
method affects the resource identified by the Request-URI, and it
also MAY have side effects on other resources; i.e., new resources
may be created, or existing ones modified, by the application of a
PATCH.

@fikisipi
Copy link

The behaviour you described can be only achieved by using UpdateAPIView (or it's corresponding mixin UpdateModelMixin together with ModelSerializer.

DRF does not in any way limit you to that specific behaviour. Here is how it works:

  1. UpdateAPIView receives a PUT request and calls put(self, request, *args, **kwargs)
  2. The put function calls self.update which is implemented since the view has UpdateModelMixin
  3. The mixin creates a serializer instance like YourSerializer(instance, data=your_data, partial=False)
  4. The mixin saves it

As you can see, the PUT request and the views have nothing to do with the optional fields. Overwriting the optional fields with default values should be possible if you modify your ModelSerializer appropriately. (by overriding some method, preferably save)

@smcoll
Copy link

smcoll commented Apr 28, 2016

So is the only current implementation of partial in a ModelSerializer that it will not raise a validation error for what would otherwise be a required attribute? Unless PUT is being used to create new objects (which is not the default behavior since 3.0), i don't see how that's very useful. If the instance already exists, we can presume that a value exists for every required field, therefore update would behave exactly as partial_update would. Am i missing something? It seems to me that a PUT ought to reset undeclared attributes to their default or blank/empty value in order to have differing behavior in a non-create scenario.

@tysonzero
Copy link
Author

@smcoll Yes, partial_update would behave exactly like update, which may seem like a strange request, but IMO the differing behavior of partial_update is not useful (some may argue it is useful in odd circumstances, but that hardly deserves an entire HTTP verb dedicated to it), but in general I don't see point in REQUIRING needless repetition of existing values. So I basically think current PATCH behavior should be slapped onto PUT, and for now PATCH/PUT will do the same thing. But in future it could open up the possibility of doing some cool stuff with PATCH like specifying instruction (increase by 1, append "foo") instead of field values, which could be useful for avoiding race conditions / simultaneous editing issues.

@tomchristie
Copy link
Member

We're not in a position to modify the behavior of existing APIs in a way like this.
Alternative takes on this that wouldn't require modification of core:

  • Third party package that has an alternative to ModelSerializer that always makes all fields required on the resulting serializer. I believe that'd behave in the way you've described.
  • Specify your serializers explicitly, and ensure that there are no default/optional fields.

@tysonzero
Copy link
Author

@tomchristie Yeah in hindsight this does seem like something I can solve with a third party or custom library. However I will say that I would probably go the opposite route and just get rid of the half-done PUT behavior and make everything optional (on update: PUT / PATCH). Perhaps disabling PATCH in case I later want to handle them in a special way like I was talking about in my last comment.

@cancan101
Copy link
Contributor

cancan101 commented Jan 6, 2017

Just my two cents, and I got bit by this as well but it might be nice to have some third party library that requires a default value be specified on all required=False fields. Otherwise at create time the default comes from the model but at PUT time, since there is no default on the serializer, the PUT just keeps whatever value the model has if the field is not in the data payload (see also: #4703).

@tomchristie
Copy link
Member

tomchristie commented Sep 29, 2017

some third party library that requires a default value be specified on all required=False fields.

That would be quite a graceful approach, yup. Agreed.

I think there's potential for a nicely scoped third party package here, at least as a starting point.

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

No branches or pull requests

6 participants