Add some WebAPI-like action results/helpers to Controller

[rynowak]

We could do with a few enhancements for API scenarios using Controller. Some ideas shamelessly stolen from WebAPI.

• BadRequest() - 400
• BadRequest(string) - 400 with a conneg error object (see Add HttpError or similar type to Core #1377)
• BadRequest(ModelState) - 400 with a conneg error object with model errors (see Add HttpError or similar type to Core #1377)
• Created(string location, object content) - 201 with conneg content
• CreatedAtAction(...) - 201 with conneg content
• CreatedAtRoute(...) - 201 with conneg content

There are more of these action result methods in ApiController but these seem like the useful ones that are gaps in the core framework.

[danroth27]

Let's discuss BadRequest(ModelState) as part of #1377

[danroth27]

Depends on outcome of discussion from #657.

[yishaigalatzer]

See http://bitoftech.net/2014/11/18/getting-started-asp-net-5-mvc-6-web-api-entity-framework-7/?utm_content=buffer2ebb1&utm_medium=social&utm_source=twitter.com&utm_campaign=buffer

[rynowak]

Recommendation for the scope of this item:

Enhancements to ObjectResult

• Add a status code property to ObjectResult
• Add a protected virtual OnFormatting(...) method to ObjectResult that's called before writing to the body. This will be overridden by various result types.

BadRequest()

• Move the BadRequestResult from WebAPI compat shim to Mvc.Core
• Add an HttpBadRequest() method to Controller
• The design/code is very consistent with HttpNotFoundResult

Created(string location, object content)

• Move CreatedNegotiatedContentResult<T> from the WebAPI compat shim to Mvc.Core
• It should be called CreatedResult and no longer be generic
• It should inherit from ObjectResult directly
• Set a status code and location header
• Add a Created(string uri, object value) and Created(Uri uri, object value) method to Controller

CreatedAtAction(...)

• Create a result called CreatedAtActionResult
• It should inherit from ObjectResult directly
• Set a status code and location header
• Use RedirectToActionResult as inspiration
• Add the same methods to controller as RedirectToAction(...)

CreatedAtRoute(...)

• Move CreatedAtRouteNegotiatedContentResult<T> from the WebAPI compat shim to Mvc.Core
• It should be called CreatedAtResult and no longer be generic
• It should inherit from ObjectResult directly
• Set a status code and location header
• Use RedirectToRouteResult as inspiration
• Add the same methods to controller as RedirectToRoute(...)

[ashmind]

Not sure if this should be separate, but please also add a helper for HttpUnprocessableEntity (422).
It is annoying to create manually as it is not in HttpStatusCode enum.

See

• http://stackoverflow.com/a/20215807/39068
• http://tools.ietf.org/html/rfc5789#section-2.2 -- non-WebDav use of 422

[yishaigalatzer]

@DamianEdwards there are the helpers we plan to add to the controller for API scenarios (other than improving the pattern), if there is anything missing please add here.

@ashmind we are not planning to add support for (422) in a helper, and we do not own the HttpStatusCode enum. Since we are only picking the most common responses. Of course you are free to add your own controller base class with the set of helpers relevant to your app. In fact in MVC 5, you can just start from a POCO class and just include what you need.

[ashmind]

@yishaigalatzer

we do not own the HttpStatusCode enum

I realize this, that's why I ask for a helper and not for an enum entry.

we are only picking the most common responses

By this logic 422 is a more useful helper than 400. 400 is a malformed request, which should not really happen after you got inside the controller action (handled by ModelBinders/MediaFormatters etc).

422 is most of semantic validation, which is pretty common inside controller actions.

[yishaigalatzer]

400 is what you return when you get model errors, so you are already in the controller.

Like I mentioned above the helper method is trivial to write, is purely additive and we would like to keep the controller api surface minimal, as its already pretty large.

We can take a second look, and if we change our mind we will post here.

[ashmind]

If you take a look at http://tools.ietf.org/html/rfc5789#section-2.2 it is pretty clear 422 is recommended for validation errors (it is talking about PATCH, but that's only because patch spec is newer than all other verbs -- older specs existed before 422).

See also

• http://stackoverflow.com/questions/16133923/400-vs-422-response-to-post-of-data/20215807#20215807
• http://stackoverflow.com/questions/15225325/is-http-status-422-appropriate-for-records-that-fail-uniqueness-validation
• http://www.bennadel.com/blog/2434-http-status-codes-for-invalid-data-400-vs-422.htm

Other users:

• Rails returns 422 for validation errors (not sure if all)
• GitHub returns 422 for validation errors (https://developer.github.com/v3/)
• Shopify (https://ecommerce.shopify.com/c/shopify-apis-and-technology/t/422-product-can-t-be-blank-error-when-trying-to-add-a-product-134876)
• Recurly (https://docs.recurly.com/api/basics/validation-errors)
• Redmine (http://www.redmine.org/projects/redmine/wiki/Rest_api#Validation-errors)
• etc

[yishaigalatzer]

@ashmind Thanks for the samples. Reading the RFC, it is still a big ambiguous when it comes to invalid data in the query (or the url for that matter), but I do see your point.

I suggest you open a separate issue, and we can follow up on this specifically.

[ajaybhargavb]

eb7283f