Start of an Asp.Net Core port

rposener commented 8 years ago

Haven't had a lot of time to do much else, yet. Coming weeks should provide more options. Any chance you guys will be at Build event? Would like to see a V1 of this published as nupkg's by then. If you guys can provide some guidance / how IServiceCollection should/should not be used, etc. that would be helpful. Also, should it leverage the HttpRequest ApplicationServices property to do all resolution? Do we need the Default Service functionality, or should that be handled as a default during registration using an option lambda on the extension method for Configuring WebHooks?

rposener commented 8 years ago

Does this look more in line with the approach you would expect?

rposener commented 8 years ago

Just a note about couple items that need input / review:

IOptions/IConfiguration setup for secrets / settings
CustomWebHookReceiver can't seem to read request body (it's all default initialized bytes of 0) not sure what I've missed on that?
Configuration Extensions of Services / Resolution of Services may need some revision - was not sure how to setup configuration extensions with options Action to pass in configuration
Error Handling for various accept types in the MiddleWare (I changed all exceptions to use a custom Exception type to specify status code + message) since HttpResponseException is not part of ASP.Net Core.

rposener commented 8 years ago

Any further information / details regarding this? Does it need to get updated to the current dev branch / RC2? Is it a good time to do that, or should we hold off until it releases?

HenrikFrystykNielsen commented 8 years ago

I am sorry it takes so long. I have poked the 'core' folks but I think they are busy. Will try again.

Eilon commented 8 years ago

{

This file shouldn't be needed; the root folder's global.json should be enough.

Refers to: src/Microsoft.AspNetCore.WebHooks.Common/global.json:1 in 1b1edb7. [](commit_id = 1b1edb7bda4975b9dc7941504d951dd91d5e5389, deletion_comment = False)

Eilon commented 8 years ago

Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Src.AspNetCore", "Src.AspNetCore", "{668B3959-3928-472F-9341-06BF8C9BADE9}"

I might just be misreading the SLN file, but are the names and paths of the projects here correct?

Refers to: WebHooks.sln:145 in 1b1edb7. [](commit_id = 1b1edb7bda4975b9dc7941504d951dd91d5e5389, deletion_comment = False)

Eilon commented 8 years ago

@rposener thank you for being so patient with us. As you may have seen earlier today, we just released ASP.NET Core 1.0 RC2, which was a long awaited released. Fortunately, that means I had a little bit of time to start reviewing this PR. I tried to focus most of my comments on the bigger questions and some other stuff that might help simplify the PR. Once we iterate on those I think things can speed up quite a bit.

Again, sorry for taking so, sooo very long to get back to you on this. I realize you've put a lot of effort into this and I want to make sure we can make WebHooks work super well with ASP.NET Core!

rposener commented 8 years ago

Excellent, thanks for the review. I will get some time in the next couple days to iterate from your comments and help things move along here. Excited to see RC2. Sounds like the cli will be simpler and a good move forward.

Eilon commented 8 years ago

Looking forward for the updates! 😄

rposener commented 8 years ago

I am going to create a new pull request - start from middle tier and work my way out. Will use a number of the same bits, but I have learned so much I really think this needs a full 1.0 revision like asp.net core instead of trying so hard to convert the prior. Will work to same interfaces for receivers and handlers as well as extensions for controllers to initiate sending.

HenrikFrystykNielsen commented 8 years ago

This sounds fantastic -- I very much appreciate all your work here!

davidfowl commented 8 years ago

👏

Eilon commented 8 years ago

Sweet 😄

rposener commented 8 years ago

OK I've updated my fork with a bunch of new projects for the core stuff. I have a few questions that I need some input on.

[x] For Receivers Middleware now calls Receivers, which produce a Context if the Handlers should be called. If no handler should be called (e.g. echo/Validation) then it just returns null. Otherwise, they set the context, or in error situation throw.
[x] All options are set via IOptions, and ServiceCollectionExtensions and MiddlewareExtensions have a setupAction to apply conifguration options as necessary.
[x] NotificationDictionary is now IWebHookNotification (I created 2 classes to support existing items, but should provide nice options for DTO's for WebHooks.
[x] Resources are not loading - trying to test sending of custom notification seems to die. Not sure what/how I've messed up resources. Any information anyone can provide would be helpful.
[x] Overall it converted much easier on RC2, some is just my experience with asp.net core growing, some is the tooling!
[x] Items not necessary for asp.net core have been omitted. Projects should be setup a bit cleaner now.

rposener commented 8 years ago

Figured out my Resource Loading issue (https://github.com/dotnet/corefx/issues/8754)

rposener commented 8 years ago

Ok - custom senders and receivers are working with all the same interfaces implemented. I have not made my way back to writing tests, but generally all the same tests as the .net project should be passing.

Question for Henrik - I had not paid attention to the Registrars before. It doesn't look like any sample or other project implements them. Is that there just in case someone needs to do some additional verification? I also did not really get the point of the IdValidator along the same lines. Could those not be just consolidated into a single validator, and leave it up to the implementation to decide what to validate?

I am heading out on Vacation till next Tuesday. If you guys can give this a bit of a review (Eilon) in the mean time, I will start on converting the tests, and look at updating the other receivers, which should not take too much effort to crank through.

MisterJames commented 8 years ago

@rposener @HenrikFrystykNielsen Hey folks, no milestone here, but it looks like it's getting close? Is there an ETA for this PR to get merged?

rposener commented 8 years ago

Well, I have had some time away where I was unable to work on it. Should be able to get more done soon. Kind of wish I could get some review from the asp.net core team to know if middleware is looking OK, and from Henrik based on questions above. Certainly is close - just want a little confidence I'm not severely overlooking something. If it's good, then we should be able to just start porting all the implementations and tests.

Or perhaps, the team wants it to be a single project that targets both frameworks. If that is the case, a lot of refactoring is probably needed... Open to input, and would love some feedback.

rposener commented 8 years ago

Should I just close this and let someone else start again?

HenrikFrystykNielsen commented 8 years ago

I really appreciate all the work you have put into it and I apologize for the lack of response. Part of it has been because core has been in flux but with core 1.0 out it should be possible to get a stable model together. I absolutely do want this to run on core but it has been difficult to get things to align. Let me chat with some of the core folks and see how we can bring your work forward. thanks!

tutukar commented 8 years ago

Any word on if and when this branch is going to be merged to master? We have a Webhook based running we are looking to Migrate to .net core. Thanks!

invisibleaxm commented 8 years ago

+1 it would be awesome if we can start developing webhooks in .net core

dalestone commented 8 years ago

Any further update on this? It would be great if we could use WebHooks in .NET core

RobertPaulson90 commented 8 years ago

I'm super excited for this! Any eta?

yilativ commented 8 years ago

Anyone try this yet?

dkent600 commented 7 years ago

It seems no one is paying attention

CharlBest commented 7 years ago

This would be very cool indeed

alhardy commented 7 years ago

We have a project were this would be very useful, hesitant to use it though because there doesn't appear to be much recent work done on the project, is work going to continue on the project?

HenrikFrystykNielsen commented 7 years ago

It is on the road map but the stars haven't aligned yet to make it come together. I will provide an update when that happens. Thanks! Henrik

tutukar commented 7 years ago

Anything on this yet?

henningst commented 7 years ago

When can we expect to get WebHooks support in ASP.NET Core? This blog post says:

A port to the ASP.NET Core is being planned so please stay tuned!

Could you share any more details on that?

@HenrikFrystykNielsen

Eilon commented 7 years ago

Closing this PR because we're going with https://github.com/aspnet/WebHooks/pull/153 instead. Thanks!

ghost commented 6 years ago

Webhook is not working for ASP.NET core 2.1 with GitHub. Does any one have any working samples.

Eilon commented 6 years ago

@ALMWEARE please log a new issue with details.

aspnet / WebHooks

Start of an Asp.Net Core port #47