Add middleware for unwrapping cloud events #93
Conversation
Fixes: #74 note: This relies on the fix for dapr/dapr#574 which has been merged. This change introduces a middleware that can upwrap a *structured* cloud event. This is the format used by dapr by default now for pub/sub messaging. Adding the middleware makes it transparent to the developer whether the data can from a cloud event or was a basic RPC call. We're adding the middleware for this first since it's the most general approach. It has a drawback compared with other approaches, performance. Users could alternatively use the SDK from CloudEvents to read their data without the middleware. We might also want to add an MVC formatter in the future, which could do the unwrapping and deserialization to a user-type in a single operation.
| return this.ProcessBodyAsync(httpContext, charSet); | ||
| } | ||
|
|
||
| private async Task ProcessBodyAsync(HttpContext httpContext, string charSet) |
There was a problem hiding this comment.
This may be slightly confusing to follow but there are 3-4 special cases that have to be handled here:
- request body is not-UTF8 (S.T.J doesn't handle that natively)
- event specifies
datacontenttypethat carries a charset (will be wiped out) datais empty (super cool, super legal)datacontenttypeis empty (assumeapplication/json)
LMK if you think anything here needs more documentation.
There was a problem hiding this comment.
Gah this is inefficient... I’ll think about it though. Also usually there’s a way to get at the original content type
@davidfowl - let's argue 😀 Do we you know of anywhere else we've done this sort of thing in the stack? We had design approaches like this in the webhooks spike from a few years ago. The most performant way I can think of to do this would be to subclass to:
no extra allocations needed. There's a little bit of a layering problem here with using a formatter, because you can't handle different content types differently (based on However this only works for MVC. I think we want the middleware to exist regardless hence why I chose that. |
|
This would have been better as a formatter since dapr integration is pretty much MVC specific, though I don't have any real emotions about this doing crazy unwrapping besides how inefficient it is. |
| app.UseCloudEvents(); | ||
| ``` | ||
|
|
||
| `UseCloudEvents()` registers the Cloud Events middleware in the request processing pipeline. This middleware will unwrap requests with Content-Type `application/cloudevents+json` so that application code can access the event payload in the request body directly. This is recommended when using pub/sub unless you have a need to process the event metadata yourself. |
There was a problem hiding this comment.
The metadata should be available via a feature (as well as the original content type).
| } | ||
| else | ||
| { | ||
| using (var reader = new HttpRequestStreamReader(httpContext.Request.Body, Encoding.GetEncoding(charSet))) |
There was a problem hiding this comment.
You should have just borrowed this logic from MVC. https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.Core/src/Formatters/TranscodingReadStream.cs
There was a problem hiding this comment.
nah, I don't want to put that complicated stuff in their code. The sender of event is the dapr runtime so this code is never going to execute. It's here for completeness, and will go away when we add .ReadAsJson to the framework.
Description
Fixes: #74
note: This relies on the fix for dapr/dapr#574 which has been merged.
This change introduces a middleware that can upwrap a structured cloud
event. This is the format used by dapr by default now for pub/sub
messaging. Adding the middleware makes it transparent to the developer
whether the data can from a cloud event or was a basic RPC call.
We're adding the middleware for this first since it's the most general
approach. It has a drawback compared with other approaches, performance.
Users could alternatively use the SDK from CloudEvents to read their
data without the middleware.
We might also want to add an MVC formatter in the future, which could do
the unwrapping and deserialization to a user-type in a single operation.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #74
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: