z0mt3c / hapi-swaggered

Yet another hapi plugin providing swagger compliant API specifications based on routes and joi schemas to be used with swagger-ui.
MIT License
82 stars 38 forks source link

swagger 2.0 #3

Closed z0mt3c closed 9 years ago

grabbou commented 9 years ago

Need any help on that? I've choosen your library after playing around with plenty of similar available and by far this is the only one that does not have a problem with nested schema arrays!

z0mt3c commented 9 years ago

Thank you for noticing ;-) An alpha version is already published to npm (alpha: '2.0.0-alpha.2'). Basically it's just the tests (including some top-level integration tests), their coverage and some beta-testing which is left until a stable release. Feel free to support!

https://travis-ci.org/z0mt3c/hapi-swaggered/builds

63 tests complete
Test duration: 297 ms
No global variable leaks detected
Coverage: 98.65% (11/813)
lib/generator.js missing coverage on line(s): 18, 19, 21, 23
lib/index.js missing coverage on line(s): 72, 73
lib/resources.js missing coverage on line(s): 24, 29, 38
lib/utils.js missing coverage on line(s): 232, 246
Code coverage below threshold: 98.65 < 99
grabbou commented 9 years ago

Great, will check it out! I'm still impressed by the fact it handles all my nested objects with custom classNames without event a single collision!

z0mt3c commented 9 years ago

Switched branches... work is now happening in master.

grabbou commented 9 years ago

Do you need any help with that? As I said, lovely plugin, unfortunately not that popular as other swagger library which used to be full of bugs (at least when I used it for the last time)

z0mt3c commented 9 years ago

I think it's close to stable - got the coverage up, feels solid, may just #15 left (feel free to discuss) :-) Are you on the latest alpha?

grabbou commented 9 years ago

Just went back to lower version as mentioned in a different issue due to the grouping problems ;)

z0mt3c commented 9 years ago

Done

grabbou commented 9 years ago

Ok. I am updating to the lastest version to see what had changed.

grabbou commented 9 years ago

Unfortunately still throwin an error. Latest version + 1.3.0 UI.

model.js:223 Uncaught TypeError: Cannot read property '$ref' of undefined

No call stack available, but I've managed to find the issue in swagger-ui (which suggests we have an error in swagger spec)...

Resolver.prototype.resolveTo = function (property, objs) {
  var ref = property.$ref; //property undefined here
  ...
}
grabbou commented 9 years ago

Further observations...

The error actually occurs here:

Resolver.prototype.resolveTo = function (property, objs) {
  var ref = property.$ref;

  if (ref) {
    if (ref.indexOf('http') === 0) {
      if (Array.isArray(objs[ref])) {
        objs[ref].push({obj: property, resolveAs: '$ref'});
      } else {
        objs[ref] = [{obj: property, resolveAs: '$ref'}];
      }
    }
  } else if (property.type === 'array') {
    var items = property.items;
    this.resolveTo(items, objs);    // items is undefined here!
  }
};

which comes probably from the fact that swagger json looks like below:

"FirstNameLastNameEmailTokenScopesHostnameModel": {
      "required":  ["firstName","lastName","email","token","scopes","hostname"],
      "properties": {
              "firstName":{"type":"string"},
              "lastName":{"type":"string"},
              "email":{"type":"string"},
              "token":{"type":"string"},
              "scopes":{"type":"array"},  /// lack of type!
              "hostname":{"type":"string"}
        }
}

which I guess it's because of this line:

 // TODO: array without type?
// arrayModel.items = { type: '' }

Again - that's my issue that I forgot to specify string for that array, but if it's not implemented and will cause errors like this, let's let user know in some way that this did happen :)

z0mt3c commented 9 years ago

Yeah true under this condition it creates an invalid schema. I hit this issue once as well - but for some reason i decided not to fix this.... may i didnt know which type to choose as fallback and decided to let swagger-ui handle it :-) But an uncaught typeerror is pretty unsatisfying.... should we go for string? what do you think?

z0mt3c commented 9 years ago

Follow up @ #22

grabbou commented 9 years ago

Cool, thanks. I've managed to fix the issue on my side, all good. I think I am gonna provide extended schema validation in the small framework I am currently working on anyway as any value is not a good rest approach.