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

Last refactor broke the Github example due to GeneralWebHook #260

Closed WestDiscGolf closed 6 years ago

WestDiscGolf commented 6 years ago

After getting latest changes and re-running the Github sample, on start up an error occurs in WebHookModelBindingProvider in the OnProviderExecuting method.

It errors when registering the FallbackHandler from the github controller ...

[GeneralWebHook]
public IActionResult FallbackHandler(string receiverName, string id, string eventName)

The error occurs when it hits the following code ...

// Reachable only for [GeneralWebHook] cases. That attribute implements
// IWebHookBodyTypeMetadata and WebHookMetadataProvider passed it along.
bodyTypeMetadata = action.Properties[typeof(IWebHookBodyTypeMetadata)];
var actionBodyTypeMetadata = (IWebHookBodyTypeMetadata)bodyTypeMetadata;
bodyType = actionBodyTypeMetadata.BodyType;

as there is no property for the typeof(IWebHookBodyTypeMetadata) for the GeneralWebHook.

image

It feels like there needs to be a GeneralMetadata class, which derives from WebHookMetadata, is registered in WebHookServiceCollectionSetup.

Thoughts?

WestDiscGolf commented 6 years ago

Thinking maybe a general metadata class for the attribute to work from, or maybe a way to override the type it associated with.

Maybe an api like ...

[GeneralWebHook(Type = typeof(GitHubMetadata))]

... where you can override which type of meta data to use for the general web hook.

I've tried a hack of creating a GeneralMetadata class and registering it but didn't work. I'm obviously missing something :-(

dougbu commented 6 years ago

Thank you for your feedback. We're closing this issue as it is fixed in our working code base. More recent builds are available from https://dotnet.myget.org/gallery/aspnetcore-dev

This was a known temporary situation in 1e89a0fb21:

temporarily breaks GitHub scenarios; real IWebHookBodyTypeMetadataService requirement coming in #254

Fixed in 730adf3128 (merged two days later).

WestDiscGolf commented 6 years ago

I am experiencing the issue running the latest in the dev branch which has the referenced commit (https://github.com/aspnet/WebHooks/commit/730adf3128b60425157b53e248e1cea19dd74ad0) in it?

I've reset my local repository copy to make sure I have the latest ... image ... and I am still getting the same error for the GeneralWebHookAttribute on the GithubController?

I am running the main dev branch from this repository.

dougbu commented 6 years ago

Thanks for reporting this issue.

Details

Ah, I see. Simple bug in WebHookModelBindingProvider's check for the typeof(IWebHookBodyTypeMetadata) property. The property won't exist unless the user has chosen a BodyType when adding a [GeneralWebHook] attribute.

WestDiscGolf commented 6 years ago

I've confirmed this by updating the controller action to be ...

[GeneralWebHook(bodyType:WebHookBodyType.Json)]
public IActionResult FallbackHandler(string receiverName, string id, string eventName)
{
    if (!ModelState.IsValid)
    {
        return BadRequest(ModelState);
    }

    return Ok();
}

And it starts up ok. Not tested the controller action though.

dougbu commented 6 years ago

349a1ea864

dougbu commented 6 years ago

Merged to dev in 607d59b9e6