Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding utility methods for parsing webhooks #208

Merged
merged 2 commits into from
Jul 17, 2020
Merged

Conversation

slorello89
Copy link
Contributor

ASP.NET / ASP.NET Core doesn't do the best job of handling the webhooks inbound from Vonage. This is mostly because we use - and _ characters in our webhook structs. This update is adding a utility library to allow users to parse webhooks easily from the common endpoints they would be using.

For reference the two main entry points I see are:

  1. ASP.NET Core MVC/web API Controllers
  2. ASP.NET Classic Web API Controllers

We support 3 content types for our webhooks as best I can gather

  1. Queries - this is actually not technically a content type - but it's also the default state your account is put in when you create it. I've added the methods ParseQuery to handle the ASP.NET Core MVC Request.Query property inbound to the controller. I've also added ParseQueryNameValuePairs to handle the Legacy Request.GetQueryNameValuePairs() from ASP.NET Classic
  2. URL Form Content
  3. JSON Content

in the case of both 2 and 3 I've added ParseWebhook methods that access the HttpRequestMessage (from the legacy ASP.NET implementation) as well as a raw stream and content type for the ASP.NET Core implementation. As both methods access the underlying stream from the request I've added async implementations for them as well.

@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2020

Codecov Report

Merging #208 into master will increase coverage by 0.06%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
+ Coverage   88.02%   88.08%   +0.06%     
==========================================
  Files         211      213       +2     
  Lines        6486     6606     +120     
  Branches      596      612      +16     
==========================================
+ Hits         5709     5819     +110     
- Misses        690      692       +2     
- Partials       87       95       +8     
Impacted Files Coverage Δ
Nexmo.Api/Utility/WebhookParser.cs 88.88% <88.88%> (ø)
Nexmo.Api.Test.Unit/WebhookParserTests.cs 93.93% <93.93%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e506f2f...ce00e8e. Read the comment docs.

Copy link
Contributor

@dragonmantank dragonmantank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks great, it will go a long way toward making the incoming webhook data much more consumable.

Can we add documentation for how the user would implement this, especially since this would be beneficial in generic webapp controllers?

…for multi-word inbound messages, updating readme
Copy link
Contributor

@dragonmantank dragonmantank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@slorello89 slorello89 merged commit 5462d81 into master Jul 17, 2020
@slorello89 slorello89 deleted the webhook_utility branch March 12, 2021 21:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants