aspnet / WebHooks

[Archived] Libraries to create and consume web hooks on ASP.NET Core. Project moved to https://github.com/aspnet/AspLabs
Apache License 2.0
627 stars 439 forks source link

GitHubWebHookAttribute should implement `IConsumesActionConstraint` #246

Closed pranavkm closed 6 years ago

pranavkm commented 6 years ago

Consider the following:

[GitHubWebHook(EventName = "issues")]
public IActionResult Post2([FromBody] JObject body) => Content("Post2");

[GitHubWebHook(EventName = "issues", AcceptFormData = true)]
public IActionResult Post3() => Content("Post3");

The two actions can be distinguished by the content type they accept. However, using this as-is results in action ambiguity. Adding a [Consumes("application/x-www-form-urlencoded")] on the latter fixes this, but I shouldn't have to since that requirement has already been indicated via AcceptFormData = true

dougbu commented 6 years ago

I suggest we close this as a duplicate of #194.


In #194, we decided to not do anything similar to this suggestion. [Consumes] is not flexible enough for use in a WebHooks scenario:

In addition, users do not need action selection in the scenario described above. The GitHub sender can be configured only one way per WebHook. So, the recommended way to receive multiple content types is to separate the configuration using id route values and corresponding Id settings in the [GitHubWebHook] attributes.

That said, we will be making changes as described in #194. Those changes will mostly make it easier to create actions that have no data parameter and simply don't care about the request body up front. An example would be an action with an associated [GeneralWebHook], no data parameter, and a method body using the IWebHookRequestReader if and when it needs to read the body.