HTTP CloudEvent coercion #421

RaeesBhatti · 2018-05-09T10:23:23Z

Add supports for proper coercion of HTTP and Invoke events to CloudEvent format.

This PR also introduces new CloudEvents processing flow:

Is the Content-Type "application/cloudevents+json"?

If yes, we're in Structured mode for CloudEvents HTTP Transport Binding. Check if the request body is a valid CloudEvent. Pass along the CloudEvent if yes, reject (400 Bad Request) if not.

Are the binary CloudEvent headers present? CE-EventType, CE-CloudEventsVersion, etc.

If yes, we're in Binary mode for CloudEvents HTTP Transport Binding. Check if the request is a valid CloudEvent. Pass along the CloudEvent if yes, reject if not.

Is there a header of "Event" that's not the value of "invoke"?

If yes, we're in legacy Custom Event mode. Use the following steps:
- If the Content-Type is "application/json" or "application/cloudevents+json", check the request body to see if it is a valid CloudEvent.
  - If yes, pass along the unaltered request body to any subscriptions.
  - If no, create a new CloudEvent with default envelope fields, and put the request body into the "data" property on the CloudEvent.
- If it's any other Content-Type, create a new CloudEvent with default envelope fields, and put the request body into the "data" property on the CloudEvent.

Default:

We have a "http.request" Event Type. Make a CloudEvent with default envelope fields, and include the HTTP request details in the "data" property of the CloudEvent.

Todos:

Write tests
Write documentation
Fix linting errors
Make sure code coverage hasn't dropped
Provide verification commands / resources
Enable "Allow edits from maintainers" for this PR
Update the messages below

Is this ready for review?: YES
Is it a breaking change?: YES

This reverts commit 45104ce.

mthenw · 2018-05-15T10:04:43Z

event/request.go

+	return eventType
+}
+
+func mapHeadersToEvent(event *Event, headers http.Header) *Event {


let's rename this function to parseAsCloudEventBinary

mthenw · 2018-05-15T10:08:14Z

event/request_test.go

+	expectedEvent *eventpkg.Event
+}{
+	{
+		url:         "https://something.eventgateway.com/myspace",


we need similar test case for application/json

mthenw · 2018-05-15T10:09:29Z

event/request_test.go

+	expectedEvent *eventpkg.Event
+}{
+	{
+		url:         "https://something.eventgateway.com/myspace",


can you change that to example.com. For some moment I was confused because I thought it's hosted EG url.

mthenw · 2018-05-15T10:11:14Z

event/request_test.go

+			CloudEventsVersion: eventpkg.TransformationVersion,
+			Source:             "/mysource",
+			ContentType:        "text/plain",
+			Data: &eventpkg.HTTPEvent{


Let's rename this stuct to HTTPRequestData

mthenw · 2018-05-15T10:16:36Z

event/request.go

+)
+
+const (
+	mimeCloudEventsJSON = "application/cloudevents+json"


it's not used

mthenw · 2018-05-15T10:18:19Z

event/event.go

 	return nil, errors.New("couldn't cast to []byte")
 }

 func isJSONContent(mime string) bool {


Can you rename this function to isJSONMimeType?

mthenw · 2018-05-15T10:20:23Z

event/request_test.go

+	}
+}
+
+var fromRequestTests = []struct {


we need more cases here. also with Event header set

mthenw · 2018-05-15T11:02:48Z

event/request.go

+	return eventType
+}
+
+func parseAsCloudEventBinary(event *Event, headers http.Header) *Event {


One more request here. It should not get Event as argument. It should be similar to parseAsCloudEvent. So the arguments should be only what's needed which is headers and body []byte. Event should be created inside.

mthenw · 2018-05-15T11:03:26Z

event/http.go


 // NewHTTPEvent returns a new instance of HTTPEvent
-func NewHTTPEvent(r *http.Request, eventData interface{}) *HTTPEvent {
+func NewHTTPEvent(r *http.Request, eventData interface{}) *HTTPRequestData {


This should also be renamed to NewHTTPRequestData

mthenw · 2018-05-15T11:04:35Z

event/request.go

@@ -0,0 +1,96 @@
+package event


can you move the content of this file to event.go? those file are pretty short and the are basically about the same thing.

codecov · 2018-05-24T11:54:43Z

Codecov Report

Merging #421 into master will increase coverage by 0.01%.
The diff coverage is 80.32%.

@@            Coverage Diff             @@
##           master     #421      +/-   ##
==========================================
+ Coverage   63.87%   63.89%   +0.01%     
==========================================
  Files          29       30       +1     
  Lines        1650     1681      +31     
==========================================
+ Hits         1054     1074      +20     
- Misses        553      563      +10     
- Partials       43       44       +1

Impacted Files	Coverage Δ
event/errors.go	`0% <0%> (ø)`
router/event.go	`100% <100%> (+19.56%)`	⬆️
event/http.go	`100% <100%> (+100%)`	⬆️
router/router.go	`52.14% <30%> (-5.81%)`	⬇️
event/event.go	`77.41% <91.48%> (+7.1%)`	⬆️

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 807d0e9...f0a134d. Read the comment docs.

RaeesBhatti

LGTM

RaeesBhatti · 2018-05-24T12:20:26Z

event/event.go

+			key = strings.TrimLeft(key, "Ce-X-")
+			// Make first character lowercase
+			runes := []rune(key)
+			runes[0] = unicode.ToLower(runes[0])


The HTTP spec says that the header keys are case-insensitive. So, I think it will be better to do a strings.ToLower here for consistency.

yeah, that's true, but in the same time, when you look at the example in HTTP binding spec you will see that desired behavior is to preserve case (testExtension).

RaeesBhatti added 5 commits May 9, 2018 14:21

HTTP CloudEvent coercion

762b8ff

Event FromRequest: Dont require path param

48155e1

Event: Remove reduant CloudEvent parsing logic

79f1cc3

Event mapHeadersToEvent: Add Extensions if transforming

ed07412

Router: emitSystemEventReceived on successful event parsing

69c8e84

mthenw changed the title ~~HTTP CloudEvent coercion~~ [WIP] HTTP CloudEvent coercion May 9, 2018

RaeesBhatti and others added 13 commits May 10, 2018 16:20

Event mapHeadersToEvent: Headers to Extensions

9d85ec1

Event: Rename a var

c4c594c

Event: New parsing method

2113cf2

event.FromRequest: Make data an HTTPEvent if an required

cac69d6

Fix some tests

fd1eced

Event: Move extensions attachment to New func

fb0057c

event.FromRequest: use isJSONContent instead of switch

e32db68

event: Simplify a condition

45104ce

event: Update mapHeadersToEvent func

6aba28e

Revert "event: Simplify a condition"

062cee0

This reverts commit 45104ce.

Formatting

f811b6a

event: Remove eventType param from parseAsCloudEvent

51fa22c

event: Add tests for event.FromRequest

f058d1c

RaeesBhatti force-pushed the httpCloudEvents branch from 187e644 to f058d1c Compare May 14, 2018 21:34

mthenw suggested changes May 15, 2018

View reviewed changes

RaeesBhatti added 4 commits May 15, 2018 14:32

event: Rename HTTPEvent struct to HTTPRequestData

b22d3c9

event: Rename func isJSONContent to isJSONMimeType

1c2d356

event: Rename func mapHeadersToEvent to parseAsCloudEventBinary

aa265cf

event: Remove redundant tests for event.go

5927f91

mthenw suggested changes May 15, 2018

View reviewed changes

RaeesBhatti added 4 commits May 15, 2018 15:15

event: Rename NewHTTPEvent to NewHTTPRequestData

d6163ab

router: Fix a struct type in test

c17fa39

Format

ceb7240

event: Merge request.go into event.go

e514de7

RaeesBhatti added 2 commits May 15, 2018 16:20

event.parseAsCloudEventBinary: Change params

1082d05

event TestFromRequest: Add loggin for error assertion

7bb9a74

mthenw approved these changes May 15, 2018

View reviewed changes

mthenw and others added 15 commits May 15, 2018 14:26

Merge branch 'master' into httpCloudEvents

d7829db

inernet/http.FlattenHeader: Make header keys lowercase

bf63c59

event.parseAsCloudEventBinary: Fix Event.Extensions

7762757

event TestFromRequest: Correct expected headers

fc3aa46

event TestFromRequest: Assert SchemaURL and Extensions

e22b1ff

internal/http: Fix tests for FlattenHeader

e1933a1

event.FromRequest: Mark http event with JSON content-type

4ffa7f0

event.FromRequest: Simplify a condition

69eb6e5

event.FromRequest: Move event.Data coercion in

8547ba0

Fix an encoding issue

428c0c5

Better content-type detection

e8cb681

Put the request in http event wrapper

6b08ab8

Update tests

5d1b5a3

Introduce new flow

8297f95

reduce complexity

929e690

mthenw changed the title ~~[WIP] HTTP CloudEvent coercion~~ HTTP CloudEvent coercion May 24, 2018

Maciej Winnicki added 3 commits May 24, 2018 13:46

Fix tests

f853c32

Add missing file

9c7b3db

fix remaining tests

7f3d0e3

fix tests

18437c3

RaeesBhatti commented May 24, 2018

View reviewed changes

Merge branch 'master' into httpCloudEvents

f0a134d

mthenw merged commit bb87dcb into master May 24, 2018

mthenw deleted the httpCloudEvents branch May 24, 2018 12:49

This was referenced Jun 1, 2018

Body is base64 encoded for http.request event with application/json Content-Type. #434

Closed

CORS config on a per-path basis within a space #449

Closed

HTTP CloudEvent coercion #421

HTTP CloudEvent coercion #421

Uh oh!

Conversation

RaeesBhatti commented May 9, 2018 • edited by mthenw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todos:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RaeesBhatti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RaeesBhatti commented May 9, 2018 •

edited by mthenw

Loading

codecov bot commented May 24, 2018 •

edited

Loading