-
Notifications
You must be signed in to change notification settings - Fork 14
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
Introduce NetworkConfig
API
#449
Introduce NetworkConfig
API
#449
Conversation
6258f52
to
a3932c0
Compare
a3932c0
to
69d0930
Compare
return nil | ||
} | ||
return NetworkConfig() | ||
} |
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.
This func has no logic with the json because, as of now, the only things to serialise inside NetworkConfig
are callbacks, which have to be set up in a different way, and in their corresponding module.
|
||
if let bodyBase64EncodedString = json["body"] as? String { | ||
request.body = Data(base64Encoded: bodyBase64EncodedString) | ||
} |
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.
We are serialising a Data / NSData
object using base64-encoded strings
.then((resultResponse) => { | ||
NetworkModule.setPreprocessedHttpResponse(responseId, resultResponse); | ||
}); | ||
}; |
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.
The other API will be part of an additional PR, but it will most likely follow the same pattern
As this is basically the body of a closure, exported into a function for clarity.
And remove drm headers from code samples as it may be confusing
I am adding an Android reviewer to look at the public API changes in typescript files (and the changelog) |
Co-authored-by: Roland Kákonyi <roland.kakonyi@bitmovin.com>
Co-authored-by: Roland Kákonyi <roland.kakonyi@bitmovin.com>
As this will be aligned with the upcoming iOS extension PR
…orkConfig # Conflicts: # CHANGELOG.md
Description
Introduce NetworkConfig with two APIs for pre-processing HTTP requests and responses.
PRs planning 📝
This PR contains the public API in typescript, and the implementation of one of the two APIs (
preprocessHttpResponse
).At least three more PRs will follow. Specifically: #452 to implement the APIs in Android, #450 to implement the second API in iOS, and one to cover the API with system tests.
However, all PRs will not be merged into development until they're all ready. In fact we will use a base branch for this feature.
Changes ✍️
Typescript
Introduce the
NetworkConfig
API, accessible throughplayerConfig.networkConfig
Config definition in
network/networkConfig.ts
This file contains the config type definition together with its other related types.
NetworkConfig:
preprocessHttpRequest(type, request) => Promise<HttpRequest>
to pre-process an HTTP request before the request is madepreprocessHttpResponse(type, response) => Promise<HttpResponse>
to pre-process an HTTP response before it gets into the playerOther relevant types
HttpRequestType
has been moved here fromsrc/events.ts
Introduce Network module in
network/index.ts
Handles the Network module and binds the actual native calls to JS through custom listeners.
Why a module? We need a module here because to serialise callbacks we have to call functions on a platform from the other.
Note: this is very similar to what we did for
drm/index.ts
, although there is no asynchronousity in those callbacks.iOS
Implementation details 🔬 (*)
The main struggle here was serialising a config object containing two APIs which are both async callbacks. We went for the following approach:
(1) In this case the API has a one of its parameter being a completion handler, which is required to finalise the pre-processing. So we also assign an identifier to the response to keep track of it. We store the completion handlers on the native side
(2) In this case, given that the callback is async, this method also handles the resolving and rejection of the promise
(3) In this case it calls the completion handler assigned to the response
Reading this additional section may be helpful for serialising callbacks in future PRs
Testing
preprocessHttpResponse
(iOS) 🧪Use httpCallbacks.patch which modifies both BasicPlayback and DrmPlayback.
To test for a non-empty
body
field, use DRM stream samples.However note that, in this PR, the request callback is not yet implemented, so it will not log or do anything!
And finally: don't forget to run
yarn bootstrap
!Things to look for:
null
Checklist
CHANGELOG
entry