Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use app.restApiRoot #10

Merged
merged 3 commits into from
Jan 14, 2014
Merged

Use app.restApiRoot #10

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c4e780
Replace strong-remoting ext/swagger with app.docs
Jan 7, 2014
8b6fc55
Use `app.get('restApiRoot')` as default basePath
Jan 7, 2014
2d15b9c
Bump up loopback min version to 1.5
Jan 14, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
* Adds dynamically-updated docs as /explorer
*/
var path = require('path');
var extend = require('util')._extend;
var loopback = require('loopback');
var swagger = requireLoopbackDependency('strong-remoting/ext/swagger');
var express = requireLoopbackDependency('express');
var STATIC_ROOT = path.join(__dirname, 'public');

Expand All @@ -13,13 +13,14 @@ module.exports = explorer;
* Example usage:
*
* var explorer = require('loopback-explorer');
* app.use('/explorer', explorer(app));
* app.use('/explorer', explorer(app, options));
*/

function explorer(loopbackApplication, options) {
var options = options || {};
var remotes = loopbackApplication.remotes();
swagger(remotes, options);
options = extend({}, options);
options.basePath = options.basePath || loopbackApplication.get('restApiRoot');

loopbackApplication.docs(options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to deprecate app.docs in the core application in favor of using swagger directly here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it better to access swagger directly? Things I don't like about that approach: The consumer has to know intimate details about how loopback is composed to be able to find strong-remoting and the swagger extension (requireLoopbackDependency('strong-remoting/ext/swagger') is a serious code smell). It also has to know that loopback is using strong-remoting and the swagger builder requires a list of remotable methods (as opposed to e.g. list of shared classes). If we later decided to extend swagger descriptors with e.g. model definitions and add another parameter to swagger() function, then we will break compatibility of new strong-remoting/loopback with the old loopback-explorer. It wouldn't happen with app.docs() approach, because it's abstracting away all these details.

If we want to deprecate app.docs then I would prefer to either deprecate strong-remoting/ext/swagger too and move the code to this project, or expose strong-remoting/ext/swagger in loopback exports. E.g. loopback.remoting.ext.swagger, where loopback.remoting is require('strong-remoting') and the rest is filled by strong-remoting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should deprecate strong-remoting/ext/swagger and move that code into this project. That sounds like a project on its own, so depending on app.docs for this change is reasonable.


var app = express();
app.get('/config.json', function(req, res) {
Expand Down
55 changes: 39 additions & 16 deletions test/explorer.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('explorer', function() {
done();
});
});
})
});

describe('with custom baseUrl', function() {
beforeEach(givenLoopBackAppWithExplorer('/api'));
Expand All @@ -60,25 +60,48 @@ describe('explorer', function() {
});
});

describe('with custom app.restApiRoot', function() {
it('should serve correct swagger-ui config', function(done) {
var app = loopback();
app.set('restApiRoot', '/rest-api-root');
configureRestApiAndExplorer(app);

request(app)
.get('/explorer/config.json')
.expect(200)
.end(function(err, res) {
if (err) return done(err);
expect(res.body).to
.have.property('discoveryUrl', '/rest-api-root/swagger/resources');
done();
});
});
});

function givenLoopBackAppWithExplorer(restUrlBase) {
return function(done) {
var app = this.app = loopback();
var Product = loopback.Model.extend('product');
Product.attachTo(loopback.memory());
app.model(Product);

if (restUrlBase) {
app.use(restUrlBase, loopback.rest());
app.use('/explorer', explorer(app, { basePath: restUrlBase }));
} else {
// LoopBack REST adapter owns the whole URL space and does not
// let other middleware handle same URLs.
// It's possible to circumvent this measure by installing
// the explorer middleware before the REST middleware.
app.use('/explorer', explorer(app));
app.use(loopback.rest());
}
configureRestApiAndExplorer(app, restUrlBase);
done();
};
}

function configureRestApiAndExplorer(app, restUrlBase) {
var Product = loopback.Model.extend('product');
Product.attachTo(loopback.memory());
app.model(Product);

if (restUrlBase) {
app.use(restUrlBase, loopback.rest());
app.use('/explorer', explorer(app, { basePath: restUrlBase }));
} else {
// LoopBack REST adapter owns the whole URL space and does not
// let other middleware handle same URLs.
// It's possible to circumvent this measure by installing
// the explorer middleware before the REST middleware.
// This way we can acess `/explorer` even when REST is mounted at `/`
app.use('/explorer', explorer(app));
app.use(app.get('restApiRoot') || '/', loopback.rest());
}
}
});