feathersjs-ecosystem / feathers-swagger

Add documentation to your FeatherJS services and feed them to Swagger UI.
MIT License
225 stars 63 forks source link

Attached model schema not available in app.js using crow generated service #168

Closed bravo-kernel closed 4 years ago

bravo-kernel commented 4 years ago

What did I do?

  1. running feathers v4
  2. app with multiple fully functioning swagger model-schemas (services generated using feathers v3)
  3. generated a new service using feather v4
  4. added a model schema to service.model for that new service
  5. confirmed the model schema is generated and attached to the service (in the service file)

What happens?

I checked the Swagger UI and not model schema for the newly generated service. Looking further it appears service.model is not available/undefined in app.js whereas it is for all other services that were generated using the feathers v3 cli.

as a workaround I keep using the v3 service style for now.

app.js

The console.log message below indicates the exact point where things go amiss, undefined for the v4 service. Interesting: the console.debug at the very bottom does show the model schema so I am sure it is added.

app.js

```js // Configure other middleware (see `middleware/index.js`) app.configure(middleware); app.configure(authentication); // Set up swagger (OpenAPI) app.configure( swagger({ openApiVersion: 3, uiIndex: true, docsPath: '/docs', docsJsonPath: '/docs/schema', specs: { info: { title: 'P2 API', description: 'A description', version: '0.0.1', }, }, defaults: { schemasGenerator(service, model, modelName) { if (model === 'projects') { console.log(service.model); // not set for the v4 service } return { [model]: service.model, [`${model}_list`]: { title: `${modelName} list`, type: 'array', items: { $ref: `#/components/schemas/${model}` }, }, }; }, }, }), ); // Set up our services (see `services/index.js`) app.configure(services); console.log('------------------------'); console.log(app.service('projects').model); ```

Services

v3 generated projects service (working)

`service.model` set here is available in `app.js` for use by the SchemaGenerator. ```js // Initializes the `projects` service on path `/projects` const createService = require('feathers-sequelize'); const createModel = require('../../models/projects.model'); const hooks = require('./projects.hooks'); module.exports = function init(app) { const Model = createModel(app); const paginate = app.get('paginate'); const options = { Model, paginate, }; // create service const service = createService(options); // generate OpenAPI 3.0 model schema as required by feathers-swagger service.model = app .get('jsonSchemaManager') .generate(Model, app.get('openApi3Strategy')); // get initialized service so we can register hooks app.use('/projects', service); // register hooks app.service('projects').hooks(hooks); }; ```

v4 generated projects service (not working)

`service.model` set here is NOT available in `app.js` for use by the SchemaGenerator. ```js // Initializes the `projects` service on path `/projects` const { Projects } = require('./projects.class'); const createModel = require('../../models/projects.model'); const hooks = require('./projects.hooks'); module.exports = function (app) { const Model = createModel(app); const paginate = app.get('paginate'); const options = { Model, paginate }; // Initialize our service with any options it requires app.use('/projects', new Projects(options, app)); // Get our initialized service so that we can register hooks const service = app.service('projects'); // generate OpenAPI 3.0 model schema as required by feathers-swagger service.model = app .get('jsonSchemaManager') .generate(Model, app.get('openApi3Strategy')); service.hooks(hooks); }; ```

Mairu commented 4 years ago

This has nothing to do with feathers versions.

You have to set the properties you want to use in the schemasGenerator (and any swagger related functionality) before you call app.service app.use.

bravo-kernel commented 4 years ago

Apologies but is that not what is happening here? If you un-collapse the app.js code you can see the SchemasGenerator is configured before app.configure(services).

If I am overlooking something obvious please let me know, otherwise this is exactly what I mean (this app.js works on v3, not on v4).

Mairu commented 4 years ago

Ok sorry, I wrote stupid things, not the app.configure but the app.use is the call that triggers the mixins that are used by the feathers-swagger package.

In the service, you used for feathers version 4 you have

 // Initialize our service with any options it requires
  app.use('/projects', new Projects(options, app));

  // Get our initialized service so that we can register hooks
  const service = app.service('projects');

  // generate OpenAPI 3.0 model schema as required by feathers-swagger
  service.model = app
    .get('jsonSchemaManager')
    .generate(Model, app.get('openApi3Strategy'));

There you fetch the initialized service from feathers, but this is different from the service that will be used when registering a service. Properties you want to use in feathers-sagger have to be set before calling app.use.

 const service = new Projects(options, app);

  // generate OpenAPI 3.0 model schema as required by feathers-swagger
  service.model = app
    .get('jsonSchemaManager')
    .generate(Model, app.get('openApi3Strategy'));

 // Initialize our service with any options it requires
  app.use('/projects', service);

Should do the trick. You can also do the service.model = ... stuff in the constructor of the Projects class.

bravo-kernel commented 4 years ago

Aha, much appreciated. I will give this a try and will report back to confirm, will probably be tomorrow.

Thanks 💯

Mairu commented 4 years ago

You could btw also use the Model from the options directly in the schemasGenerator. service.options.Model should be available there without custom additions, so the call to your jsonSchemaManager could be done in the schemasGenerator directly.

Or even better extend the Service from feathers-sequelize and do in the constructor.

bravo-kernel commented 4 years ago

OK, I will try all options tomorrow and see where I end up. After that I will update the instructions I made for Feathers. Appreciate the time you're taking.

bravo-kernel commented 4 years ago

That last one sounds pretty cool to me. However, there are model-specific options that can be passed to the SchemaGenerator as well so that must continue to function.

Mairu commented 4 years ago

You could add additional properties to options if needed.

bravo-kernel commented 4 years ago

This is a lot of information to take in, lol but I will give it serious time, highly appreciated

bravo-kernel commented 4 years ago

@Mairu first of all, thanks for the input, priceless. After testing some variations I think I will adopt the following as the advized/documented solution:

  1. generate model schema inside "your" SchemaGenerator
  2. configure model specific overrides in the sequelize model (options.jsonSchema)
  3. then configure swagger like shown below:
schemasGenerator(service, model, modelName) {
  const modelSchema = app
    .get('jsonSchemaManager')
    .generate(
      service.options.Model,
      app.get('openApi3Strategy'),
      service.options.Model.options.jsonSchema,
    );

  return {
    [model]: modelSchema,
    [`${model}_list`]: {
      title: `${modelName} list`,
      type: 'array',
      items: { $ref: `#/components/schemas/${model}` },
    },
  };
},

Do you agree this looks good too? Docs have been updated https://github.com/alt3/sequelize-to-json-schemas/issues/17

Mairu commented 4 years ago

Well there could also be services that are not sequelize services, so I would add a check for that.

const { Service: SequelizeService } = require('feathers-sequelize');

// ...
schemasGenerator(service, model, modelName) {
  if (!service instanceof SequelizeService) {
    return {};
  }

  //...
}
bravo-kernel commented 4 years ago

@mairu awesome, I will add that check and be done with it. Thanks for your support, highly appreciated. Happy to see that my design wasn't flawed as well to be honest, the two libs work great together 💃

bravo-kernel commented 4 years ago

Hope I'm not driving you crazy here but I want to prevent users from hitting errors. Your example causes the following eslint error.

Unexpected negating the left operand of 'instanceof' operator: no-unsafe-negation

Their explanation says this will always be false. However, changing it as they suggest to:

if (!(service instanceof SequelizeService)) {

the outcome is always false. Very strange.

Mairu commented 4 years ago

Mhh it was more a theoretical thing, can only check it this evening. Alternatively you can check for service.options.Model.

bravo-kernel commented 4 years ago

Aha, that figures, I took it literally haha. What a day 💃

Mairu commented 4 years ago

So yeah instanceOf is not working, so checking the model seems to be the way to go.

if (!service.options || !service.options.Model || !(service.options.Model instanceof SequelizeModel)) {
   return {};
}
bravo-kernel commented 4 years ago

Is this hypothetical as well, just checking ;) Because my quick test didn't not work I will have to put this on my todo-list until I find some time. Thanks for all your support, highly appreciated, keep up the good job!

const Sequelize = require('sequelize');

if ( (!(service.options)) || (!(service.options.Model)) || (!(service.options.Model instanceof Sequelize.Model))) {
    // always false
Mairu commented 4 years ago

Yes it was theoretically for sequelize, I just tested it with with NeDB, there it was working.

Mairu commented 4 years ago

So in case you have not figured it out yourself in the meantime and for the sake of completeness: for sequelize it should be:

if (!(service.options && service.options.Model && service.options.Model.sequelize)) {
   return {};
}
Barbapapazes commented 4 years ago

Hello, I'm sorry but after doing every thing you do @bravo-kernel , I have an error.

TypeError: Provided model does not match expected format. Are you sure this is a Sequelize model?

from this

  service.model = app
    .get('jsonSchemaManager')
    .generate(Model, app.get('openApi3Strategy'));

How do you handle this ?

bravo-kernel commented 4 years ago

@Barbapapazes I do not perform this (hypothetical) check at all actually. I am just using https://github.com/alt3/sequelize-to-json-schemas/issues/17

Barbapapazes commented 4 years ago

And can you explain how to use sequelize to have a correct format from a mongoose model ?

bravo-kernel commented 4 years ago

Never used mongoose

Mairu commented 4 years ago

For mongoose you could use https://github.com/giddyinc/mongoose-to-swagger I guess.

Barbapapazes commented 4 years ago

Ho thanks, I will take a look at it. And I finally succeeded to use Swagger using this https://github.com/DScheglov/mongoose-schema-jsonschema