search

aarondcoleman / Fitbit.NET

Fitbit .NET API Client Library
MIT License
197 stars 140 forks source link

Subscription handling with aspnet webhooks #240

Closed WestDiscGolf closed 3 weeks ago

WestDiscGolf commented 6 years ago

I was reading the aspnet core 2.1 roadmap blog posts and after reading the details of the "WebHooks" - https://blogs.msdn.microsoft.com/webdev/2018/02/02/asp-net-core-2-1-roadmap/#webhooks - functionality I kept thinking this would be ideal to consume/handle a subscription call back end point.

Starter for 10 idea:

Thoughts?

aarondcoleman commented 6 years ago

Hey Adam! I see the Webhooks example in the blog, but that's essentially how our (Fitabase at least) consuming code looks like with Fitbit's PubSub. The only difference I'd guess is just we parse it using the Subscription Manager class. I guess, besides academic pursuit, the question is what value add does this really solve? Fitbit's PubSub isn't a true webhook, so we're already kind of fudging things I think trying to shoehorn this.

Thanks, --Aaron

Founder & CEO Fitabase by Small Steps Labs LLC @aaronc

On Tue, Feb 6, 2018 at 8:43 AM, Adam Storr notifications@github.com wrote:

I was reading the aspnet core 2.1 roadmap blog posts and after reading the details of the "WebHooks" - https://blogs.msdn.microsoft. com/webdev/2018/02/02/asp-net-core-2-1-roadmap/#webhooks - functionality I kept thinking this would be ideal to consume/handle a subscription call back end point.

Starter for 10 idea:

-

new project - Fitbit.AspNetCore.Webhooks

Model - FitbitSubscriptionNotification to deserialize (as defined - https://dev.fitbit.com/build/reference/web-api/subscriptions/ https://dev.fitbit.com/build/reference/web-api/subscriptions/)

[ { "collectionType": "activities", "date": "2010-03-01", "ownerId": "184X36", "ownerType": "user", "subscriptionId": "2345" } ]

-

Follow the project pattern - https://github.com/aspnet/ WebHooks/tree/dev/src/Microsoft.AspNetCore.WebHooks. Receivers.AzureAlert https://github.com/aspnet/WebHooks/tree/dev/src/Microsoft.AspNetCore.WebHooks.Receivers.AzureAlert

Use the subscription manager to handle the reverse lookup etc as part of the process (needs #81 https://github.com/aarondcoleman/Fitbit.NET/issues/81 resolving).

Thoughts?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aarondcoleman/Fitbit.NET/issues/240, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ1CXa-6-zZvSBcQ2qPMQqI2VJi2fpqks5tSIEfgaJpZM4R7XBC .

WestDiscGolf commented 6 years ago

Is it not a true pub/sub? Didn't realise that.

Yes we'd use the parsing of the subscription manager, although it's not been developed much so we can add to that.

As for why, saw something cool and thought "would this fit with fitbit.net?", would it aid ease of use for new people parsing and listening for subscription responses. Yes it can already be done but surely less steps to getting it working for consumers the better. If people don't have to worry about the inner workings then they can add value to their projects, and clients, quicker :-)

aarondcoleman commented 6 years ago

Hey Adam, I'm still not sure the why. You won't be able to, like, config-only using the out-of-box webhooks. The way we and others I know who have used Fitbit.NET do it is simply to make a controller, go to Fitbit's dev panel, point it to that controller, and then parse the message using the SubscriptionManager class and do something with that (we put that in an Azure Queue). There's no "hook" per say, no retry, no expectations on Fitbit's end for any closing the loop, so in order of complexity to set this up now. So I'm not sure we're solving a problem by doing the webhook framework mapping is my fear. It's pretty simple as-is -- maybe I'm missing something else we gain?

Cheers, --Aaron

Founder & CEO Fitabase by Small Steps Labs LLC @aaronc

On Tue, Feb 6, 2018 at 11:03 PM, Adam Storr notifications@github.com wrote:

Is it not a true pub/sub? Didn't realise that.

Yes we'd use the parsing of the subscription manager, although it's not been developed much so we can add to that.

As for why, saw something cool and thought would this fit with fitbit.net, would it aid ease of us for new people parsing and listening for subscription responses. Yes it can already be done but surely less steps to getting it working for consumers the better. If people don't have to worry about the inner workings then they can add value to their projects, and clients, quicker :-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aarondcoleman/Fitbit.NET/issues/240#issuecomment-363676535, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ1CSSWHOgrGOQ8yQ2J2NrN4NMObsO0ks5tSUrSgaJpZM4R7XBC .

WestDiscGolf commented 6 years ago

Hey, From my limited understanding it would make it so the controller could accept the updated response instantiated and populated object and then carry on. You wouldn't have to process the raw response or use the Subscription Manager manually yourself, it would be used under the hood. I think the fact that its called a "WebHook" might be where the confusion is.

You register the "hook" at startup and apply it as an attribute on the controller action eg:

[GitHubWebHook]
public IActionResult GitHubHandler(string id, string @event, JObject data)

Then you'd implement a WebHookVerifySignatureFilter eg:

public class GitHubVerifySignatureFilter : WebHookVerifySignatureFilter, IAsyncResourceFilter

Which would be registered in the startup. This class would implement OnResourceExecutionAsync from IAsyncResourceFilter at which point the details of the body post etc. can be verified and checked. This is where I would expect the reverse DNS look up to be done and the Signature checking before the instantiated action method parameter gets passed through.

The JObject above in the Github example can be swapped out into a specific model class we're expecting. Looking at the samples the Pusher controller has a specific parameter eg:

[PusherWebHook(Id = "It")]
public IActionResult PusherForIt(string[] eventNames, PusherNotifications data)

So I would expect the fitbit.net one, to look something like ...

[FitbitWebHook]
public IActionResult Fitbit(UpdatedResources data)

but by then it has been verified, checked and model bound (UpdatesResources would essentially be a wrapper for List<UpdatedResource>).

I believe then we potentially can implement OnResourceExecuted(ResourceExecutedContext context) from the IResourceFilter interface to make sure the response is a 204 as per the fitbit docs.

Note: Your server must return a 204 No Content response code.

ResourceExecutedContext has an IActionResult Result property on which makes me think it is possible.

        /// <summary>
        /// Gets or sets the result.
        /// </summary>
        /// <remarks>
        /// <para>
        /// The <see cref="Result"/> may be provided by execution of the action itself or by another
        /// filter.
        /// </para>
        /// <para>
        /// The <see cref="Result"/> has already been written to the response before being made available
        /// to resource filters.
        /// </para>
        /// </remarks>
        public virtual IActionResult Result { get; set; }

All of the above is just an idea and wanted to kick it around for a bit. All the webhooks code is aspnetcore 2.1 so it's not coming for a little while :-)

WestDiscGolf commented 6 years ago

Unfortunately the webhooks library has been cut from the aspnet core 2.1 release :-(

However, I have been playing with it over the past couple of months and have come up with this so would be interested in hearing peoples thoughts.