bardzusny / swaggerize-restify

Design-driven apis with swagger 2.0 and restify.
14 stars 5 forks source link

versioning - allow swaggerize() to be called multiple times #2

Open chintan1287 opened 9 years ago

chintan1287 commented 9 years ago

Allow swaggerize() to call multiple times for different versions

bardzusny commented 9 years ago

@chintan1287: Isn't second commit of this PR cancelling out most of the appropriate changes?

Overall, even despite involuntary hesitance to implement any features not already present in swaggerize-express, this one makes a lot of sense from the angle of restify providing us with its semantic route versioning. Here's how I envision its usage:

swaggerize(server, {
  api: path.resolve('./config/spec.json'),
  handlers: path.resolve('./handlers'),
  version: "1.0.0" // version to register routes under
});

version parameter being completely optional - by default we would read it from info object within provided Swagger spec document.

The only real issue I can see (as I already mentioned in #1 ) is making swagger-ui include proper Accept-Version header in its requests to the API. I'll look into it some more when I get the time (sorry for delayed answer, by the way).

bardzusny commented 9 years ago

(Moving outside of comment thread to first post)

@bardzusny made swagger configurable and writable. This will allow to call swaggerize multiple times.

Oops, yes, now I see what you mean. Does it actually give us anything in the absence of route versioning?

chintan1287 commented 9 years ago

@bardzusny yes, it helps in making multiple swaggerize() calls. One can have separate swagger object for each version registered on the same restify version.

versions.forEach(function(version) {
    var swaggerJson = require('./swaggerJson.js')(version);
    var swaggerHandler = require('./swaggerHandler.js')(version);

    swaggerize(server, {
        api: swaggerJson,
        handlers: swaggerHandler,
        docspath: '/api-docs'
    });

    server['swagger'+version] = server.swagger;
    delete server.swagger;
});

// can initialize individual swagger object
server.listen(port, 'localhost', function () {
    app.swaggerv1.api.host = server.address().address + ':' + server.address().port;
    app.swaggerv2.api.host = server.address().address + ':' + server.address().port;
});

Although not ideal way of handling versioning, but helps to achieve what you mentioned here - https://github.com/bardzusny/swaggerize-restify/issues/1

bardzusny commented 9 years ago

Sorry for letting it sit quietly for so long! Seriously lacking in time lately.

What restify lacks and express has is ability to be used as Connect.JS middleware - mounting different servers under different mount paths.

Here's the adequate node-restify issue: https://github.com/restify/node-restify/issues/89 Here's a sensible workaround (but still - workaround): https://gist.github.com/jmibanez/2140974

I believe this is something you should try when trying to mount multiple restify server under different path prefixes. The solution you proposed would work, but is/seems quite hackish - I'm mostly talking about manually assigning multiple server.swagger objects by hand.

What I had in mind when it comes to versioning is to not depend on path prefixes for different versions (which you can still achieve though, with multiple restify servers, using gist I linked to), but to resort to natively supported by restify semantic versioning, namely: using Accept-Version header.

Ability to call swaggerize() on the same server multiple times is the smallest part of the problem. There are two bigger issues that I can see so far:

swaggerize(server, {
  // first call - 1.0.0
});
swaggerize(server, {
  // second call - 2.0.0
});

server.swagger['1.0.0'] // 1.0.0
server.swagger['2.0.0'] // 2.0.0
server.swagger['latest'] // 2.0.0 - ???

// or, possibly, guarding backwards compatibility
server['swagger-1.0.0'] // 1.0.0
server['swagger-2.0.0'] // 2.0.0
server.swagger // 2.0.0 - last/newest registered version

If you're up for implementing this approach, by all means go ahead - otherwise I'll try to do it myself when I find some time. Let me know what you think.

bardzusny commented 8 years ago

This is quite old; any news here @chintan1287 ? Otherwise I'd close this PR, and just note down this idea for a feature.