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

idea: use swagger sec to abstract away more of the express/req details from handler implementation #96

Open
lucidNTR opened this issue Dec 18, 2017 · 1 comment

Comments

@lucidNTR
Copy link

lucidNTR commented Dec 18, 2017

currently swagger specs are used to parse and validate the incoming requests to a blue oak server but a handler has to get the data from the request like in old express times similar to this example facebook webhook definition:

"/facebook": {
      "x-bos-handler": "facebook",
      "get": {
        "operationId": "facebookVerify",
        "consumes": [
          "text/plain"
        ],
        "produces": [
          "application/json"
        ],
        "parameters": [
          {
            "name": "hub.mode",
            "in": "query",
            "type": "string",
            "required": true,
            "example": "subscribe"
          },
          {
            "name": "hub.challenge",
            "in": "query",
            "type": "number",
            "required": true,
            "example": "1231054077"
          },
          {
            "name": "hub.verify_token",
            "in": "query",
            "type": "string",
            "required": true,
            "example": "test_token"
          }
        ],
        "responses": {
          "200": {
            "schema": {
              "type": "number"
            }
          }, ... error spec...

The handler code would currently look similar to:

export const facebookVerify = (req, res) => {
  if (req.query['hub.mode'] === 'subscribe' && req.query['hub.verify_token'] === process.env.fbWebhookVerifyToken) {
    res.status(200).set('Content-Type', 'application/json').send(req.query['hub.challenge'])
  } else {
    res.sendStatus(403)
  }
}

As a first draft to make better use of the swagger spec:

"/facebook": {
      "x-bos-handler2": "handlers/facebook",

/*-> depending if backward compatibility to express handlers of current bos is needed, 
would be named differently. handler values would not be interpreted as js files 
inside a handler folder but simply as javascript import/require statements.
This way typescipt support would be trivial, as it is handled by the node runtime/ compiler,
also this would allow direct handover to external modules or other modules
without writing server handler glue functions.
Also this would make using plugins for bos handlers as trivial as eg. 
"yarn install bos-rabbitMQ-forwarder" and then using 
"x-bos-handler2": "bos-rabbitMQ-forwarder"*/

      "get": {
        "operationId": "facebookVerify", 
-> would not need to change as the call format of operation id is implementation specific anyways

        "consumes": [
          "text/plain"
        ],
        "produces": [
          "application/json"
        ],
        "parameters": [
          {
            "name": "hub.mode",
             "x-bos-rename": "mode", 
/* optional if the key name is awkward to work with as in this facebook example,
if not set the name would be used for key */

            "in": "query",
            "type": "string",
            "required": true,
            "example": "subscribe"
          }, ... etc.

then the x-bos-handler2 function signature would look like this:

export const facebookVerify = ({mode, verify_token, challenge, res}) => {
  if (mode === 'subscribe' && verifyToken === process.env.fbWebhookVerifyToken) {
    res.status(200).set('Content-Type', 'application/json').send(challenge)
  } else {
    res.sendStatus(403)
  }
}

notice:

  • this could also made to work for the body and all nested json schemas, hugely obsoleting handler glue code for most use cases,
  • the function signature with parameter destructuring would free us of sticking to a parameter order.
  • of course the req object and next function would still be there if needed, so all use cases of the old function signature would be possible ({mode, verifyToken, challenge, res, req, next}) => {...}
  • default values in swagger would be respected for everything
  • maybe its advantageous to use a data object for all parsed out data like this
    ({data, res, req, next}) => { data.mode .... data.verifyToken...}?
@seanpk
Copy link
Member

seanpk commented Jan 8, 2018

@lucidNTR - I can't believe it's taken me so long to review and reply! Sorry.

Further leveraging the Swagger/OpenAPI spec is something I'm very interested in exploring.
I've had the thought for a while that it should be possible to generate TypeScript types based on the models in the spec and then use those to help write the correct code.
I think this idea relates and can help.

For some of the specifics (number for reference only)

  1. I like the idea of bundling all spec-defined parameters into a single parameter
  2. Even with this change res and next are needed for responses and common error handling ... I wonder if there's a useful abstraction for that too ...?
  3. (If we have a little bit more of an abstraction from express, could it be possible to plugin other engines at the base? LoopBack might be a simple step, but may Koa or Hapi too ... I've also been wondering about whether we could make some of the benefits of BOS available to developers who already have a project underway or prefer another base ...)
  4. (Back from the clouds) For backward compatibility we'll have to provide a new "type" like you show
  5. I like the idea of using the require/import approach to get the file to run
  6. While operationId is built-in to the OpenAPI specification, I've found it annoying to use because the spec requires that it is globally unique so you end up with functions like getFooDetails and getBarDetails, even when those functions are already in handlers/foo.js and handlers/bar.js, respectively (i.e. you can't just call the functions getDetails)
  7. What if we abandon the user of operationId and instead mimic the import expression directly, e.g. (in yaml): x-bos-interface: "getDetails from 'handlers/foo'" (or x-bos-handler-func or x-bos-handler-fn or x-bos-hfn ... ?)
  8. Perhaps the function signature for these methods looks something like function (input, output); where all the parsed data was on the input parameter, as was the underlying req object from express, and the output parameter would include the underlying res and next objects

@dannylevenson and @jwsm - you may be interested in this too

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

No branches or pull requests

2 participants