Skip to content
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

Http Methods #53

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

Http Methods #53

wants to merge 34 commits into from

Conversation

jlacivita
Copy link
Contributor

@jlacivita jlacivita commented Aug 15, 2022

Creates the ability to add HTTP/REST methods to an SDK. Would be used by Extension SDKs

Creates a new module called Http that looks like a Transport Layer.

Methods that have the "http" tag will:

import Transport from '../Http'

So that existing method templates will just work over Http.

The "http" tag supports several extensions:

  • x-http-endpoint - The baseUrl for the REST API
  • x-http-path - the path to append to the baseUrl
  • x-http-headers - any headers to include
  • x-http-parameters - any query parameters to include

Additionally, all of these extensions will support the following macros:

  • ${param.} - insert the value of param
  • ${token} - insert the value of the Extension SDK's token

Creates an Extensions Module that allows Apps to initialize any imported Extension SDKs. This module will be exported from Core, and potentially any other SDKs that talk directly to the FEE over RPC.

Extensions.initialize(id, config)

Allows apps to pass in a configuration to each Extension SDK. Additionally, this method will pass callbacks for:

  • token() - allows Extension SDKs to get a token, will be mapped to Authentication.token('platform') by default
  • authorize(permissions) - allows Extension SDKs to acquire user grants before grabbing the token, mapped to Capabilities.request() by default.

This PR also adds file-level tree-shaking to the SDK generation: unimported files are scrubbed from the build, to avoid bulking up SDKs with code they don't need. Tree-shaking starts at the main file in package.json and crawls through any import foo from bar or export foo from bar statements.

- Add Http module: a transport layer, but for REST-based Extension SDKs
- Remove global window hack for testing from production code
- Added a Browser.js module that set's up a global window object for Jest
- Added a Server.js module that creates a REST server using OpenRPC examples
- Core SDK / Extension SDK handhsake
- Config
- Query params
- POST body
@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: e784330

Report detail: gist

@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: 7f55ab5

Report detail: gist

test/Server.js Outdated
else if (request.url.endsWith('/account/authenticate')) {
response.write(JSON.stringify({
oat: "OAT",
bearerToken: "BEARER",

Choose a reason for hiding this comment

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

Hi Jeremy: Please confirm that these two lines are not sensitive. (Unlikely but I have to ask.)
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, they are not. The code deals with tokens, but there are no secrets or example values present.

})
})

return new Promise((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

might be better to rewrite this as an async function, a lot of nested .thens going on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stylistically, I agree.

However, I've been resisting the urge to use async/await because we haven't used them yet in the code. I just did a quick search and saw it's used in the new version of Lifecycle.ready() in the Core SDK. Have we tested that code on all of the browsers we care about?

Http.onAuthorize(apis.authorize)
}

export const initialize = window.__firebolt ? () => { throw new Error('Use Extensions.initialize() from \\'@firebolt-js/sdk\\' to initialize ${pkg.name}.') } : _initialize
Copy link
Contributor

Choose a reason for hiding this comment

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

think I might need this walked through. Why would the initialize method throw an error if __firebolt is not undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This export const initialize is for non-Firebolt apps that need to use the SDK. They can import the initialize method and use the SDK w/out Firebolt. Not a high priority use case, but seemed like a good bonus to get teams to leverage our framework.

Firebolt get's access to the real initialize method (not the exported one) via window.__firebolt.registerExtensionSDK.

@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: 69dedd6

Report detail: gist

@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: f29fa0c

Report detail: gist

@rdkcmf-jenkins
Copy link

WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

  • Upvote reason: Thanks Jeremy

  • Commit: f29fa0c

@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: caad894

Report detail: gist

@rdkcmf-jenkins
Copy link

Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/firebolt-openrpc/53/rdkcentral/firebolt-openrpc

  • Commit: c4128ec

Report detail: gist

@rdkcmf-jenkins
Copy link

WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

  • Upvote reason: Token detected but not significant

  • Commit: c4128ec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants