-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lib/runtime): Implement ext_offchain_http_request_add_header_version_1
host function
#1994
Changes from 45 commits
2899a29
8f7a581
4d6167f
faa5752
e08300e
70e6351
ad8c0bb
5606599
f2ca718
81c0fb9
6da5ad8
fb21f11
881fc51
7444590
8896ae5
ee372c4
78bfd0b
42ff50e
13d540a
60a1f11
4cd379f
f1130ad
5f980ab
c8913d2
fcb5da9
9d2b7c5
dd89472
8624e09
427c183
b791f35
750caa9
6cdef2b
c856a87
85c4703
d203860
c8463f8
5353927
281ed22
a7e74d3
18a2cc8
2e57583
d02a434
eee9136
9410059
a4441a2
92b9f49
75af512
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,9 @@ package offchain | |
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"net/http" | ||
"strings" | ||
"sync" | ||
) | ||
|
||
|
@@ -15,6 +17,9 @@ var ( | |
errIntBufferEmpty = errors.New("int buffer exhausted") | ||
errIntBufferFull = errors.New("int buffer is full") | ||
errRequestIDNotAvailable = errors.New("request id not available") | ||
errRequestInvalid = errors.New("request is invalid") | ||
errRequestAlreadyStarted = errors.New("request has already started") | ||
errInvalidHeaderKey = errors.New("invalid header key") | ||
) | ||
|
||
// requestIDBuffer created to control the amount of available non-duplicated ids | ||
|
@@ -48,10 +53,36 @@ func (b requestIDBuffer) put(i int16) error { | |
} | ||
} | ||
|
||
// Request holds the request object and update the invalid and waiting status whenever | ||
// the request starts or is waiting to be read | ||
type Request struct { | ||
Request *http.Request | ||
invalid, waiting bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this concurrent safe? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, when are these variables set and what are they used for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for now, there is no concurrency (will be implemented here #2020). The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see where in the code they're set (unless it wasn't in this PR?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, right now these values aren't being set because the function that does this is not implemented yet! |
||
} | ||
|
||
// AddHeader adds a new HTTP header into request property, only if request is valid and has not started yet | ||
func (r *Request) AddHeader(name, value string) error { | ||
if r.invalid { | ||
return errRequestInvalid | ||
} | ||
|
||
if r.waiting { | ||
return errRequestAlreadyStarted | ||
} | ||
|
||
name = strings.TrimSpace(name) | ||
if len(name) == 0 { | ||
return fmt.Errorf("%w: empty header key", errInvalidHeaderKey) | ||
} | ||
|
||
r.Request.Header.Add(name, value) | ||
return nil | ||
} | ||
|
||
// HTTPSet holds a pool of concurrent http request calls | ||
type HTTPSet struct { | ||
*sync.Mutex | ||
reqs map[int16]*http.Request | ||
reqs map[int16]*Request | ||
idBuff requestIDBuffer | ||
} | ||
|
||
|
@@ -60,7 +91,7 @@ type HTTPSet struct { | |
func NewHTTPSet() *HTTPSet { | ||
return &HTTPSet{ | ||
new(sync.Mutex), | ||
make(map[int16]*http.Request), | ||
make(map[int16]*Request), | ||
newIntBuffer(maxConcurrentRequests), | ||
} | ||
} | ||
|
@@ -85,7 +116,10 @@ func (p *HTTPSet) StartRequest(method, uri string) (int16, error) { | |
return 0, err | ||
} | ||
|
||
p.reqs[id] = req | ||
p.reqs[id] = &Request{ | ||
Request: req, | ||
} | ||
|
||
return id, nil | ||
} | ||
|
||
|
@@ -100,7 +134,7 @@ func (p *HTTPSet) Remove(id int16) error { | |
} | ||
|
||
// Get returns a request or nil if request not found | ||
func (p *HTTPSet) Get(id int16) *http.Request { | ||
func (p *HTTPSet) Get(id int16) *Request { | ||
p.Lock() | ||
defer p.Unlock() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use http.Request's context to store those two booleans? Just to reduce the Go API. Although it might be not worth it depending how many times we check these values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we can get improvements storing these values on
http.Request's context
the reason we had these values is to avoid operations over already ongoing or finalized HTTP callsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not for improvements except to use
*http.Request
directly. Most http routers store stuff in the http.Request's context, I think it would fit nicely, so we would have one less type to manage ourselves.Example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qdm12, done!