api-guidelines icon indicating copy to clipboard operation
api-guidelines copied to clipboard

Q. must a PATCH upsert that creates a new resource return 201 Created and Location header?

Open TimLovellSmith opened this issue 3 years ago • 9 comments

It seems logical/implied, by the ideas that PATCH will have create semantics when it allows upsert semantics (same as PUT), and a similar clause that exists for POST - that if POST creates a resource it should return 201 created and a Location header.

TimLovellSmith avatar Sep 02 '21 05:09 TimLovellSmith

I think the Azure API Guidelines are reasonable clear that 201 should be returned whenever a resource is created:

DO return the state of the resource after a PUT, PATCH, POST, or GET operation with a 200-OK or 201-Created.

But there is no explicit guideline for returning Location on a create, and there probably should be.

mikekistler avatar Nov 16 '21 13:11 mikekistler

Is PATCH being treated as upsert instead of update existing discouraged in general?

TimLovellSmith avatar Oct 18 '22 19:10 TimLovellSmith

PATCH can be used for create as well as for update. If this is done, then PUT is not required at all. A location response header is not required for PUT or PATCH because the URL IS the location. Do I understand your question accurately?

JeffreyRichter avatar Oct 18 '22 19:10 JeffreyRichter

Is the question for LRO case or non-LRO case (latest data-plane guideline discourages LRO Patch, but LRO Patch is common in ARM)?

weidongxu-microsoft avatar Oct 19 '22 02:10 weidongxu-microsoft

@JeffreyRichter

PATCH can be used for create as well as for update.

To be honest as a consumer of the API / Swagger/OpenAPI Definitions to be honest that's concerning to hear.

Since we've not seen any instances of this across Resource Manager (aside from the Monitor API, which is frankly pretty broken/different to the rest of Azure) - we're using the following to determine the CRUD for each Resource in the API:

  • A POST/PUT against a given Resource to indicate how to Create [or CreateOrUpdate] a given Resource.
  • A GET against a given Resource to indicate how a given resource can be retrieved.
  • A GET returning an Array of an item (against a Resource Type, e.g. virtualMachines rather than virtualMachines/{virtualMachineName}) to indicate a List operation.
  • A DELETE against a given Resource indicating that this can be used to delete this resource.
  • A PATCH against a given Resource (if it exists) is used for the Update method.

Given some operations use PATCH for Update and others reuse the POST/PUT to make the Service Teams life easier (which it's worth noting, ultimately pushing the complexity onto clients to determine which fields are valid) - we have to determine which is appropriate.

Whilst this may seem like an odd tangent, I'm trying to call out that from a consumer perspective this would lead to a considerable UX degradation for users of the API. If a PATCH method can now (conditionally) create resources, are all fields going to be Optional in the Swagger/OpenAPI? How will consumers know which fields to use?

I'd argue that whilst creating a new resource from a PATCH method is technically allowed per the HTTP Specification the "may" indicates this is pretty niche:

2.  The PATCH Method

The PATCH method requests that a set of changes described in the request entity be applied to the resource identified by the Request-URI.  The set of changes is represented in a format called a "patch document" identified by a media type.  If the Request-URI does not point to an existing resource, the server MAY create a new resource, depending on the patch document type (whether it can logically modify a null resource) and permissions, etc.
   
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.

https://datatracker.ietf.org/doc/html/rfc5789

MDN explains it as such:

A PATCH request is considered a set of instructions on how to modify a resource. Contrast this with PUT; which is a complete representation of a resource.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PATCH

Which matches the explanations I've heard for PATCH that I've heard from other developers.. so I think most would find this quite surprising?

So I guess my question is, are there any existing PATCH operations within Resource Manager that are used for Create?

tombuildsstuff avatar Oct 19 '22 16:10 tombuildsstuff

@tombuildsstuff To be clear, I'm definitely not recommending people use PATCH as their primary way of creating resources. PUT is great for that.

I'm just looking to clarify what Azure considers OK behavior when someone does a PATCH on a non-existent resource.

A PATCH request on a non-existing resource with an 'insufficient' set of properties (missing required details), seems to pretty obviously be a bad request.

I know there are a bunch of services where PATCH with a 'sufficient' set of properties will have enough information that you could create the resource from scratch. So, I also think that the services MAY choose create a resource in response (given that we don't specifically say not to today!), and some services very probably already do.

Sounds like you would like to discourage services behaving such a way, but I'm not sure if I understood the objections yet.

TimLovellSmith avatar Oct 19 '22 16:10 TimLovellSmith

One of the main points I wanted to bring up in this issue was that service that DOES choose to create resources in response to such request, if its allowed, should be returning 201, to communicate that it had the creation side effect to the caller.

Since we don't normally require services to declare 201 in the list of possible responses for PATCH requests in swaggers etc, you can guess that if they contain 201 in the possible responses, they implement this behavior. But recently I think I saw some text somewhere saying 201 should not be returned from PATCH, which flummoxed me greatly.

TimLovellSmith avatar Oct 19 '22 16:10 TimLovellSmith

201 and 200 are effectively the same thing and our SDKs treat them identically. The reason is because if a PUT/PATCH creates a resource, the client may not get the 201 due to network failure/timeout. The client will retry the same operation again; but, this time, since the resource was previously created, the service returns a 200 (not 201). So, the client has to treat 200 & 201 as the same thing.

JeffreyRichter avatar Oct 19 '22 17:10 JeffreyRichter

I can't answer if ARM allows PATCH for create as I focus mostly on Azure data plane services. Someone else (like @TimLovellSmith) will have to respond.

JeffreyRichter avatar Oct 19 '22 17:10 JeffreyRichter