Skip to content

Helper function to create event from HTTP Request or Response#799

Merged
n3wscott merged 7 commits intocloudevents:mainfrom
grayside:from-http-request
Sep 21, 2022
Merged

Helper function to create event from HTTP Request or Response#799
n3wscott merged 7 commits intocloudevents:mainfrom
grayside:from-http-request

Conversation

@grayside
Copy link
Copy Markdown
Contributor

@grayside grayside commented Sep 6, 2022

Fixes #766

This PR provides two new functions: NewEventFromHttpRequest and NewEventFromHttpResponse. Both are added to v2/alias.go, and NewEventFromHttpRequest is also demonstrated in the README and docs.

I included HttpResponse because I saw the Message methods covered it. I included docs updates because I've visited the README and docs multiple times looking for this function. Happy to remove either if they are considered unnecessary or out of scope.

Creating this PR as a draft because the test coverage I've created routinely segfaults when it begins executing the transformation logic at:

sdk-go/v2/binding/to_event.go

Line 61 in c623f8b

return &e, Transformers(transformers).Transform((*EventMessage)(&e), encoder)

Here is an example of how this looks on my machine:

go test -v -run TestNewEventFromHttpRequest
=== RUN   TestNewEventFromHttpRequest
=== RUN   TestNewEventFromHttpRequest/Event{"specversion":"1.0","id":"full-event-0","source":"http://example.com/source","type":"com.example.FullEvent","subject":"topic","datacontenttype":"text/json","dataschema":"http://example.com/schema","time":"2020-03-21T12:34:56.78Z","data":"hello","exurl":"http://example.com/source","extime":"2020-03-21T12:34:56.78Z","exbool":true,"exint":42,"exstring":"exstring","exbinary":"AAECAw=="}
=== RUN   TestNewEventFromHttpRequest/Event{"specversion":"1.0","id":"full-event-0","source":"http://example.com/source","type":"com.example.FullEvent","subject":"topic","datacontenttype":"text/json","dataschema":"http://example.com/schema","time":"2020-03-21T12:34:56.78Z","data":"hello","exurl":"http://example.com/source","extime":"2020-03-21T12:34:56.78Z","exbool":true,"exint":42,"exstring":"exstring","exbinary":"AAECAw=="}/Structured_encoding
--- FAIL: TestNewEventFromHttpRequest (0.00s)
    --- FAIL: TestNewEventFromHttpRequest/Event{"specversion":"1.0","id":"full-event-0","source":"http://example.com/source","type":"com.example.FullEvent","subject":"topic","datacontenttype":"text/json","dataschema":"http://example.com/schema","time":"2020-03-21T12:34:56.78Z","data":"hello","exurl":"http://example.com/source","extime":"2020-03-21T12:34:56.78Z","exbool":true,"exint":42,"exstring":"exstring","exbinary":"AAECAw=="} (0.00s)
        --- FAIL: TestNewEventFromHttpRequest/Event{"specversion":"1.0","id":"full-event-0","source":"http://example.com/source","type":"com.example.FullEvent","subject":"topic","datacontenttype":"text/json","dataschema":"http://example.com/schema","time":"2020-03-21T12:34:56.78Z","data":"hello","exurl":"http://example.com/source","extime":"2020-03-21T12:34:56.78Z","exbool":true,"exint":42,"exstring":"exstring","exbinary":"AAECAw=="}/Structured_encoding (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x141d695]

goroutine 6 [running]:
testing.tRunner.func1.2({0x151cfe0, 0x1982990})
	/Users/clipped/opt/go/libexec/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/Users/clipped/opt/go/libexec/src/testing/testing.go:1392 +0x39f
panic({0x151cfe0, 0x1982990})
	/Users/clipped/opt/go/libexec/src/runtime/panic.go:838 +0x207
github.com/cloudevents/sdk-go/v2/binding.Transformers.Transform(...)
	/Users/clipped/prototypes/snowpea-golang/sdk-go/v2/binding/transformer.go:34
github.com/cloudevents/sdk-go/v2/binding.ToEvent({0xc0000ec100?, 0x1466716?}, {0x1671030, 0xc0000207d0?}, {0xc00006ae78, 0x1, 0x0?})
	/Users/clipped/prototypes/snowpea-golang/sdk-go/v2/binding/to_event.go:65 +0x215
github.com/cloudevents/sdk-go/v2/protocol/http.NewEventFromHttpRequest(0x16707a0?)
	/Users/clipped/prototypes/snowpea-golang/sdk-go/v2/protocol/http/utility.go:19 +0x50
github.com/cloudevents/sdk-go/v2/protocol/http.TestNewEventFromHttpRequest.func1.1(0x0?)
	/Users/clipped/prototypes/snowpea-golang/sdk-go/v2/protocol/http/utility_test.go:42 +0xc6
testing.tRunner(0xc0002b81a0, 0xc0002ae1d0)
	/Users/clipped/opt/go/libexec/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/Users/clipped/opt/go/libexec/src/testing/testing.go:1486 +0x35f
exit status 2

@grayside grayside mentioned this pull request Sep 6, 2022
Closed
@grayside
Copy link
Copy Markdown
Contributor Author

grayside commented Sep 8, 2022

A bit more detail: The line linked above is reached for events in binary encoding. When it happens, the transform statement is creating a transformer. Non-binary events are successfully created and returned earlier, and the Transformers slice is empty.

@grayside
Add missing use of encoding options and remove redundant function call
b916b0b 
Signed-off-by: Adam Ross <adamross@google.com>
n3wscott
n3wscott reviewed Sep 9, 2022
View reviewed changes
grayside and others added 4 commits September 15, 2022 10:10
@grayside @n3wscott
Update v2/protocol/http/utility.go
14d8640 
Co-authored-by: Scott Nichols <n3wscott@tableflip.dev>
@grayside @n3wscott
Update v2/protocol/http/utility.go
f729381 
Co-authored-by: Scott Nichols <n3wscott@tableflip.dev>
@grayside @n3wscott
Update v2/protocol/http/utility.go
6ec3a27 
Co-authored-by: Scott Nichols <n3wscott@tableflip.dev>
@grayside @n3wscott
Update v2/protocol/http/utility.go
3de37fb 
Co-authored-by: Scott Nichols <n3wscott@tableflip.dev>
@grayside
Copy link
Copy Markdown
Contributor Author

grayside commented Sep 15, 2022

Thanks for the help troubleshooting. I'm going to clean this up to make it a non-draft.

Do you think there should be a passthru for transformers?

@grayside
capitalize HTTP and WIP clean-up
6098325 
Signed-off-by: Adam Ross <adamross@google.com>
@grayside grayside marked this pull request as ready for review September 15, 2022 17:30
n3wscott
n3wscott approved these changes Sep 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

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

Looks like you just need to fix the signing and you will be good to go

@grayside
Copy link
Copy Markdown
Contributor Author

grayside commented Sep 20, 2022

I've signed off many of the commits, not sure how to fix it for all of them. Looking at the DCO doc, it looks like someone with write access can override. Can you point me to what my next step should be?

@n3wscott n3wscott merged commit a718752 into cloudevents:main Sep 21, 2022
@n3wscott
Copy link
Copy Markdown
Member

n3wscott commented Sep 21, 2022

I've signed off many of the commits, not sure how to fix it for all of them. Looking at the DCO doc, it looks like someone with write access can override. Can you point me to what my next step should be?

Thanks! I got it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@n3wscott n3wscott n3wscott approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Question, maybe add to documentation request: HTTP request to CloudEvent

2 participants

@grayside @n3wscott