Looking for feedback

[jsdevel]

This issue is to request feedback from the community so express-openapi may be improved. For any who wish not to be apart of this, please comment herein with please remove me and I'll delete any comment herein containing your @ mention.

[jsdevel]

@hilkeheremans Just saw the issue you were having in swaggerize-routes regarding recursive schemas. I believe express-openapi doesn't have that issue. Would you be willing to try express-openapi to see if it is? If not no biggie. Thank you!

[hilkeheremans]

@jsdevel Too bad I didn't see this earlier; I had just forked swaggerize-express and stripped off the validation to circumvent the issue. Still, express-openapi looks interesting as well so I'll give it a go.

Just as an FYI, I will be combining this with swagger-express-middleware.

Expect some feedback tomorrow!

[jsdevel]

Sounds good! express-openapi leverages jsonschema to the max, so you should find it to be pretty stable. Looking forward to your feedback!

[hilkeheremans]

Seems to work perfectly for a very simple use case. FYI, we are using this in what most devs would probably call an enterprise-grade application, with a public API described by OpenAPI. We're still working on the plumbing and robustness. Over the next few weeks we'll be hooking up all our business logic etc, so feedback will get more interesting at that point.

Some quick observations. I'm putting them here as they are not feature requests/bug reports, just discussion points and a FYI for users who might want to move from swaggerize to express-openapi.

• Path resolution for the handlers seems to be relative to the working directory (standard behavior for '.'). This is fine for me but a minor caveat.
• The example in README.md uses a require to a .js file, which depends on a module.exports. At that point you can't directly use output from any swagger editor - you need to convert the .json to .js. Fortunately, you can just apiDoc: require('./api-doc.json')which works just as well. Might want to make this clear in the docs?
• If you insist on using .js just for the ability to comment, perhaps json5 (https://github.com/aseemk/json5) is an option instead? This is backwards compatible with json without need for any conversion. It's not a full community standard yet but is quite popular and even supported in node-config

[jsdevel]

Over the next few weeks we'll be hooking up all our business logic etc, so feedback will get more interesting at that point.

Looking forward to it!

Path resolution for the handlers seems to be relative to the working directory (standard behavior for '.'). This is fine for me but a minor caveat.

It can either be relative or absolute. Relative is based on the working directory of the running process. Should this be called out in the docs somehow?

The example in README.md uses a require to a .js file, which depends on a module.exports. At that point you can't directly use output from any swagger editor - you need to convert the .json to .js. Fortunately, you can just apiDoc: require('./api-doc.json')which works just as well. Might want to make this clear in the docs?

You can actually use json or js. .initialize() just needs a parsed javascript object that is your api definition. But yea, calling this out in the docs would be better.

If you insist on using .js just for the ability to comment, perhaps json5...

Haven't heard of json5! Looks pretty cool. Again, updating the docs here would likely be good.

Thanks for the feedback! I'll spend this week adding more clarification to the docs around these points.

[jsdevel]

@skindc should you have any issues with swaggerize-express, I'd be really interested to get your feedback on express-openapi. The benefits of this framework are that it is entirely built on top of jsonschema which is more standard in a swagger (openapi) sense.

[hilkeheremans]

@jsdevel It seems I was a little too enthusiastic; express-openapi currently also does NOT handle recursive definitions. See kogosoftwarellc/express-openapi-validation#1 for more details.

[jsdevel]

@hilkeheremans see my response kogosoftwarellc/express-openapi-validation#1 (comment).

I also updated the tests in this project to use recursion and it works. See test/sample-projects/basic-usage/api-doc.js.

[jsdevel]

@rcherny, @rochal Just an FYI, express-openapi returns batched validation errors. I'd love to see you try it and leave any feedback you may have.

[rcherny]

Hey @jsdevel I've tried it but had some issues with what I was precisely looking to do. Overall it looked like a nice set up. I'm trying to basically mock a swagger-based API with node.js and have validation that resembles what a .NET WebAPI back-end would be returning. I've got endpoints and all set up with Swaggerize-Express but the validation bypasses the handlers and I'm a relative ExpressJS noob so I'm trying to tweak the final output back to the client. We expect a 200 and a collection of validation errors on the front-end.

I'm certain there's a way to do this, but I'm just getting started... but when I tried express-openapi I got odd behavior since my swagger spec is already defined, paths and all — I wasn't looking to define the paths in code, just augment them. The moment I put a code-level API handler in the endpoint disappeared from the Swagger UI for some reason.

[rcherny]

@jsdevel sorry I should also have said, I'd be totally game for dropping express-openapi-validation into an existing ExpressJS set up but I'd need to:

1. drop it in as middleware
2. have it augment pre-existing paths
3. have it do it's "magic validation" and then augment it with express-openapi-response-validation

But I haven't had enough time to get that working so I had to move on for the moment.

Are there any examples in the Tests that would show effectively what I need?

I'm 100% certain there's a way to do this, but I just haven't had time to figure it out.

[jsdevel]

@rcherny I think what you may be looking to do is intercept the validation errors, and change the status code. If so, put some error catching middleware (defined with 4 parameters) as the last middleware in your chain like so:

app.use(function(err, req, res, next) {
  console.log(err);
  err.status = 200;
  res.status(200).json(err);
});

[rcherny]

Hey @jsdevel that's exactly what I ended up doing, more or less. The issue though is really that the err that comes back should have multiple errors listed, and other frameworks thus far are only giving me one. That said, I haven't tried the express-openapi modules listed above yet.

[jsdevel]

I think you'll be pleasantly surprised 😄

[jsdevel]

@rcherny checkout the multiple errors returned in this data driven test: https://github.com/kogosoftwarellc/express-openapi-validation/blob/master/test/data-driven/fail-multiple-invalid-path-params.js

[jsdevel]

@asaf not sure if you're using swaggerize-express, but express-openapi should handle empty strings just fine.

[rcherny]

Hey @jsdevel I am using swaggerize-express — main thing is I'm unsure where to tap into it to use express-openapi-validation instead — as I have an api.json file with the rules in it, and the examples for openapi seem to include validation inside the code. Given more time I might just move the whole thing over, but for this current sprint I'm on, this may have to suffice. Otherwise, this does sound superior :)

[jsdevel]

main thing is I'm unsure where to tap into it to use express-openapi-validation instead

You'll find with swaggerize-express that it isn't possible currently to disable input validation. If you add express-openapi-validation to your project as is, you'll incur the cost of double validation which isn't good.

Given more time I might just move the whole thing over, but for this current sprint I'm on, this may have to suffice. Otherwise, this does sound superior :)

I understand deadlines completely. express-openapi was borne because the maintainers of swaggerize just weren't very responsive to issues. You'll find the migration path to be very painless. Let me know if I can help.

[rcherny]

@jsdevel re: the validation issue with Swaggerize ... yeah it's buried pretty deep in the code, I was afraid of that. No I certainly don't want double validation.

If you could describe / document how to use ecpress-openapi with the definitions, paths, and rules in an external file, I'd be grateful. What I recall is the paths that I had file handlers for defined in the file system vanished from Swagger-UI.

But I could be mistaken. I may have time later this week to revisit.

[rcherny]

Sigh. Fat thumbed that on my phone. Sorry. express **

[jsdevel]

@rcherny I have a sample project that's really straight forward. Check it out https://github.com/kogosoftwarellc/express-openapi/tree/master/test/sample-projects/basic-usage.

That project is used in the travis builds as well.

[MugeSo]

Hi @jsdevel!
This project has more simple, readable, beautiful code than swaggerize- 😄
I have a high expectation for this 😆

BTW, it seems parameters property of path item object is not supported.
It makes api definition simpler! Especially, defining parameter in path.

[jsdevel]

@MugeSo parameters should be supported, unless I'm not understanding what you mean.

[MugeSo]

@jsdevel

unless I'm not understanding what you mean.

sorry, I'm not so good at English 😔
I just intended to say how important parameters property is.

[jsdevel]

@MugeSo no worries. So parameters is supported. See this example: https://github.com/kogosoftwarellc/express-openapi/blob/master/test/sample-projects/basic-usage/api-routes/users/%7Bid%7D.js#L45

What's probably throwing you off is that you define it in your handler files.

This project has more simple, readable, beautiful code than swaggerize- 😄

I'm glad you think so!

[MugeSo]

@jsdevel It's parameters of operation object, I said parameters of path item object.

[jsdevel]

Ah ha! I understand now! Cool! I didn't know swagger supported that. I should be able to add that support easily. Basically just concat the 2 arrays (path parameters and operation parameters). I'll add this support tonight most likely.

[MugeSo]

great!

[jsdevel]

@MugeSo added path parameters with e6e1835. Thanks again for the feedback!

[jsdevel]

Added support for custom json schema formats with b01fc0e.

Here's an example:

{
  "type": "string",
  "format": "myCustomFormat"
}

openapi.initialize({
  apiDoc: require('./api-doc'),
  app: app,
  customFormats: {
    myCustomFormat: function(input) {
      return input === 'my special format';// return a boolean for valid
    }
  }
});

[MugeSo]

@jsdevel I wrote type definition of this package for typescript.
https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/express-openapi

If you want, you can port it ;)
And add typings into package.json.
https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages

P.S.
... I know my type definition does not support customFormats yet.

[jsdevel]

Super awesome @MugeSo! I'd love to include it. Can you submit a PR?

[MugeSo]

@jsdevel OK, I'll do it.

[jsdevel]

@dcolens I'd love to get your feedback on express-openapi. It was inspired by swaggerize-express, but has fewer pain points. Your migration path should be trivial.

[jsdevel]

express-openapi now adds missing tags that are found in operation docs to the apiDoc tags array. See b94068f

[jsdevel]

Added support for error middleware that is scoped to the API's basePath property with 9f4b733.

[jsdevel]

@johnleetechnology I noticed that you were having issues using $ref in swaggerize-routes. I'd be interested in having you try express-openapi to see if it handles your use case.

[jsdevel]

Added del alias for delete and recognizing api docs on method handlers in array middleware with v0.14.0.

[jsdevel]

@rkarodia if you're looking for a swagger module that allows you to transform validation errors, checkout express-openapi. See args.errorTransformer for more details.

[MugeSo]

I wish something like:

// in "routes/my/handler.js"
var openapi = require('express-openapi');
module.exports.post = function(req, res, next) {
   req.status(204)
       .header('Content-Location',
           openapi.resolveRoute('someOperationId', {paramName: 'foo'})
       .json({progress: 0});
}

[MugeSo]

How about to add app.mountpath to apiDoc.baseUrl.

[jsdevel]

@MugeSo adding app.mountpath would be good. Can you submit a PR for that?

[jsdevel]

@MugeSo what would the use case of openapi.resolveRoute be?

[MugeSo]

@jsdevel OK, I'll send PR for app.mountpath

resolveRoute is to generate URL by operationId for Location and Content-Location header or other links.
It keeps programmer away from path except when touch directory or file name.

Now I think res.resolveRoute would be bettter.

[jsdevel]

Oh I see what you mean. Cool!

Can you submit a PR for that as well?

[MugeSo]

@jsdevel No, I can't because resolveRoute is just idea yet.
We need more considering and discussion.

[jsdevel]

@MugeSo makes sense. I feel getOperationURL makes more sense then as your intent is to get a URL by an operation name.

[jsdevel]

Thanks to @MugeSo, args.externalSchemas is now supported in v0.19.0.

[jsdevel]

@jacob-zneider just saw the issue you submitted on swaggerize-express. The main issue with that library is that it doesn't rely on jsonschema. Swagger/openapi are built on jsonschema, so any library not build on top of that is almost doomed to fail. I'd love to know what you think about express-openapi after giving it a shot.

[jsdevel]

@marcofranssen express-openapi used express-openapi-response-validation middleware. I'd love to see what you think of it. (saw that you posted a related issue in swaggerize-routes).