Conversation
mikkoseppa
commented
Oct 9, 2018
mikkoseppa
commented
- Refactor Route to support multiple handlers and api versioning.
- Add HeaderApiVersioning and RouteApiVersioning features.
- Refactor Router to forward apiVersioningConfig from Router's config to Route.
- Also resolve feature name in Router to the actual versioning feature.
- Add tests for Router and both versioning features.
3 similar comments
mikkoseppa
force-pushed
the
ms-add-support-for-api-versioning
branch
2 times, most recently
from
f8ef394 to
300c279
Compare
|
We could remove older than 6 node versions from travis TypeError: availableApiVersions.includes is not a function |
header-api-versioning/index.js
Outdated
| * @param {Object} req Epxress request object | ||
| * @returns {number} Api version | ||
| */ | ||
| findApiVersionFromRequest: function(req) { |
There was a problem hiding this comment.
If this function would get actually handler (req) => { return apiVersionOrUndefined; } callback as parameter, it could work for any kind of method for fetching API version from request so we wouldn't have to hardcode any parameter / header names.
There was a problem hiding this comment.
Sorry I didn't quite get the point. Could you give me a small code example how this would work?
There was a problem hiding this comment.
So in configuration could be like this
config = {
parseCallback: (req) => {
let headerString = req.headers['accept'];
let pattern = 'application\/vnd.someapplication\.v(\\d+)\\+[A-Za-z]+';
let results = header.match(new RegExp(pattern));
return results ? parseInt(results) : undefined;
};
That way one could decide with complete freedom how API version number is extracted anywhere from request.
There was a problem hiding this comment.
PR updated to support generateRoutePathHandler and findApiVersionHandler in config.
|
|
||
| it('should be able to register a versioned handled for two different versions', function () { | ||
| router[method]('/some/path') | ||
| .public() |
There was a problem hiding this comment.
All api version handlers share the same authentication code? Would be good to have also tests, which uses default authenticator and custom authentication handlers.
There was a problem hiding this comment.
Added apiVersioning+auth and apiVersioning+middleware tests to PR.
There was a problem hiding this comment.
I still couldn't find the case where handlers would be called with defaultAuthHandler included... all the routes seemed having .public() at start, which removes the default handler.
mikkoseppa
force-pushed
the
ms-add-support-for-api-versioning
branch
2 times, most recently
from
0a6419f to
6b4b8d1
Compare
- Refactor Route to support multiple handlers and api versioning. - Add HeaderApiVersioning and RouteApiVersioning features. - Refactor Router to forward apiVersioningConfig from Router's config to Route. - Also resolve feature name in Router to the actual versioning feature. - Add tests for Router and both versioning features.
mikkoseppa
force-pushed
the
ms-add-support-for-api-versioning
branch
from
6b4b8d1 to
76770fb
Compare
… request but handler is not found. - Add fallback functionality to Route findHandler_ function. - Add tests.
|
Could you still drop node .12 from .travis.yml file to make tests pass? |
| }); | ||
| }); | ||
|
|
||
| describe.only('API versioning', function() { |
3 participants
