krakenjs / generator-swaggerize

Yeoman generator for design-driven apis with swagger 2.0 and krakenjs/swaggerize tools.
Other
70 stars 34 forks source link

Issue with allOf in swagger definitions #8

Closed MrWako closed 8 years ago

MrWako commented 9 years ago

Hi Folks,

I've been looking at the V2 JSON examples in https://github.com/swagger-api/swagger-spec/tree/master/examples/v2.0/json and using them with the generator has created a few issues that I've tried to breakdown a little. The first is from https://github.com/swagger-api/swagger-spec/blob/master/examples/v2.0/json/petstore-simple.json

Take 2 copies of this file and delete the "/pets/{id}": { ... } sections. In one use the definition

"petInput": {
      "allOf": [
        {
          "$ref": "pet"
        }
      ]
    }

in the other use the definition

"petInput": {
      "required": [
        "id",
        "name"
      ],
      "properties": {
        "id": {
          "type": "integer",
          "format": "int64"
        },
        "name": {
          "type": "string"
        },
        "tag": {
          "type": "string"
        }
      }
    }

I find the one with the $ref generates:

Swaggerize Generator
? What would you like to call this project: ref
? Your name: 
? Your github user name: 
? Your email: 
? Path (or URL) to swagger document: ref.json
? Express or Hapi: express
   create .jshintrc
   create .gitignore
   create .npmignore
   create index.js
   create package.json
   create README.md
   create config/ref.json
   create handlers/pets.js

undefined:10
_.forEach(Object.keys(properties), function (prop) {;
                      ^
ReferenceError: properties is not defined
    at eval (eval at template (/node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/node_modules/lodash/dist/lodash.js:6305:22), <anonymous>:10:23)
    at underscore [as _engine] (/node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/lib/util/engines.js:32:30)
    at engine (/node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/lib/actions/actions.js:303:10)
    at template (/node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/lib/actions/actions.js:281:15)
    at /node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/app/index.js:260:18
    at Array.forEach (native)
    at yeoman.generators.Base.extend.models (/node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/app/index.js:249:49)
    at /node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/lib/base.js:341:43
    at /node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/node_modules/async/lib/async.js:551:21
    at /node-v0.10.33-linux-x64/lib/node_modules/generator-swaggerize/node_modules/yeoman-generator/node_modules/async/lib/async.js:227:13

node version v0.10.33, yeoman version 1.3.3. Latest version of generator-swaggerize.

tlivings commented 9 years ago

allOf is not supported for validation at the moment due to some continued work on the enjoi module.

You can perform the same definition (from your example) with:

"petInput": {
   "$ref": "pet"
}

Also, in your other referenced schema, you are missing the type attribute (which should be object). The swagger examples are wrong as well I might add.

Thanks for your patience.

MrWako commented 9 years ago

great, thanks for the update - I also submitted an issue regarding these examples to the swagger-spec project as I would be good if the examples were accurate.

grawk commented 8 years ago

Is this an issue with the enjoi module then? Or in swaggerize-* somewhere? Running into this now as well, and with allOf being a good way to DRY, it's a shame to not be able to use it...

subeeshcbabu-zz commented 8 years ago

Following the enjoi docs - https://github.com/tlivings/enjoi#schema-support, allOf support got released as part of one of the recent versions.

I am waiting for a new version publish of the generator-swaggerize, that includes my change of enjoi version upgrade. Will test this out as soon as the new generator gets published.

subeeshcbabu-zz commented 8 years ago

https://github.com/krakenjs/generator-swaggerize/blob/master/app/templates/_model.js#L7 models template generator is expecting properties as opposed to the possibilities of allOf etc. I will work on a PR for addressing this.

mirchow commented 8 years ago

This feature would be much appreciated. Consider skipping generating of allOf models for now with a console warning. It would allow the rest of models to be generated. This section can be added to your pets.json tests

       "Pets": {
            "type": "array",
            "items": {
                "$ref": "#/definitions/Pet"
            }
        },
        "Paging": {
            "properties": {
                "totalItems": {
                    "type": "integer"
                },
                "totalPages": {
                    "type": "integer"
                },
                "pageSize": {
                    "type": "integer"
                },
                "currentPage": {
                    "type": "integer"
                }
            }
        },
        "PagedPets": {
            "allOf": [
                {
                    "$ref": "#/definitions/Pets"
                },
                {
                    "$ref": "#/definitions/Paging"
                }
            ]
        },
subeeshcbabu-zz commented 8 years ago

Please checkout the latest version generator-swaggerize@3.0.0. This should be resolved.