Possible bug in UpdateCustomer about northwindtraders HOT 5 CLOSED

afaik, the SuppressModelStateInvalidFilter setting doesn't prevent the validation from running, it only stops the MVC pipeline from returning a BadRequest response when the Model is invalid.

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.apibehavioroptions.suppressmodelstateinvalidfilter?view=aspnetcore-2.2

from northwindtraders.

You have one more option:

   command.id = id; // enforced consistency
   await Mediator.Send(command);
   return NoContent();

from northwindtraders.

This is also not that perfect -- in swagger (and more importantly, in auto-generated clients) id will appear both in route and body.

And, what's even worse, fluentvalidation will get broken because it runs before controller is actually called.

from northwindtraders.

A good question. We need the Id in the command, since the command needs to contain all information nececessary to update a customer. We also need the Id in the route, since we are building (at least trying to build) restful APIs.

I think the best approach is to handle this at the Web API level, since it is an API concern. That being the case, my recommendation is as follows:

            if (id != command.Id)
            {
                return BadRequest();
            }

At this stage, I don't think more effort is required. If this approach ends up being too painful, I'll change it later.

And, what's even worse, fluentvalidation will get broken because it runs before controller is actually called.

Just on the above point. In this case, fluent validation only runs on the commands and queries, not the controller actions. This is a result of the following within Startup.cs:

            services.Configure<ApiBehaviorOptions>(options =>
            {
                options.SuppressModelStateInvalidFilter = true;
            });

And the following validation behaviour; https://github.com/JasonGT/NorthwindTraders/blob/master/Northwind.Application/Infrastructure/RequestValidationBehavior.cs.

Hope this helps.

from northwindtraders.

A good question. We need the Id in the command, since the command needs to contain all information nececessary to update a customer. We also need the Id in the route, since we are building (at least trying to build) restful APIs.

I think it is not mandatory to include server generated (Id, TimeStapmps (creation date), Unique Sequential numbers) info in a PUT resource representation, if like you say at least trying to build restful APIs.

The reason is that resource reprepresntation from the GET request may be different from the resource representation you use in PUT request, but it does not meant that the representation is not complete/full, but the resource is still same. e.g. up to you to decide that concrete representation is complete/describes the resource completely. Its obvious and mandatory to include in the put representation all fields that user may/can change but not for the server side generated stuff.

From the RFC 7231 4.3.4 PUT:

If the target resource does not have a current representation and the
PUT successfully creates one, then the origin server MUST inform the
user agent by sending a 201 (Created) response.

It (HTTP) does not define how
resource state is "stored", nor how such storage might change as a
result of a change in resource state, nor how the origin server
translates resource state into representations. Generally speaking,
all implementation details behind the resource interface are
intentionally hidden by the server.

The RFC 7231 itself does not says that it is requred to create a new resource on a put, only to respond correctly if server actually creates a new one. Also the Id field is not the resource identifier in a REST. Identificator for the client is a resource url itself, location - the whole url. The Id field in this case is more like implementation details that your backend API is using internally to track entites and recognize them. But if your client using ID field for url construction (hacking urls) thats not good and should not affect your API design. This leads to a hypermedia question, but that not the case of this topic.

An origin server SHOULD verify that the PUT representation is
consistent with any constraints the server has for the target
resource that cannot or will not be changed by the PUT. This is
particularly important when the origin server uses internal
configuration information related to the URI in order to set the
values for representation metadata on GET responses.

The Rest CookBook says:

Use PUT to create new resources only when the client can control part of the URI. For
instance, a storage server may allocate a root URI for each client and let clients create
new resources using that root URI as a root directory on a filesystem. Otherwise,
use POST.

When you support PUT to create new resources, clients must be able to assign URIs for
resources.

So i assume if API is not supporting creation of resources via PUT(only replacement the contents of an existing resource, not location change) because Id is generated on server side by ORM (EF) and client cant decide it by himself, removing the Id field from the PUT representation is ok and it will be still complete (representation is still complete).

Another good example of different representations provided in Rest in Practice:
In example there used a PUT reqeust to to change resource state with one representation while having a
respone 200 OK with other reprsentation that contains a status field, that cannot be desided by client.

There’s no order ID embedded
in the payload, since it would be superfluous. Following this DRY (Don’t Repeat
Yourself) pattern, we avoid potential inconsistencies between the domain model
and the resources the service exposes, and keep the URI as the authoritative
identifier, as it should be.

**PUT /order/1234 HTTP/1.1**
Host: restbucks.com
Content-Type: application/xml
Content-Length: 246
<order xmlns=″http://schemas.restbucks.com/order″>
<location>takeAway</location>
<items>
<item>
<milk>skim</milk>
<name>cappuccino</name>
<quantity>1</quantity>
<size>large</size>
</item>
</items>
</order>

HTTP/1.1 200 OK
Content-Length: 275
Content-Type: application/xml
Date: Sun, 30 Nov 2008 21:47:34 GMT
<order xmlns=″http://schemas.restbucks.com/order″>
<location>takeAway</location>
<items>
<item>
<milk>skim</milk>
<name>cappuccino</name>
<quantity>1</quantity>
<size>large</size>
</item>
</items>
**<status>preparing</status>**
</order>

There are a couple of question on a stack about it:
https://stackoverflow.com/questions/19821221/rest-api-design-of-a-resource-whose-properties-are-not-editable-by-the-client
https://stackoverflow.com/questions/36498982/same-representation-for-get-and-put

from northwindtraders.