Support for multiple versions

[hekike]

Have you ever considered to add version support to find-my-way?
If I understand correctly it could be a new child node in a Radix Tree below the path node.

Something like this:

router.on('GET', '/', '1.2.3', (req, res, params) => {
  res.end('{"message":"hello world 1"}')
})
router.on('GET', '/', '2.0.0', (req, res, params) => {
  res.end('{"message":"hello world 2"}')
})

request({
  method: 'GET',
  uri: '/',
  headers: { 'accept-version': '~1.2'  }
}) => 'hello world 2'

request({
  method: 'GET',
  uri: '/',
  headers: { 'accept-version': '~3.1'  }
}) => 400, ~2.1 is not supported by GET /

Lookup could happen by Accept-Version header with finding the biggest version that satisfies via semver.

[mcollina]

This would be a good addition, as long as it does not slow things down. My worry is if we need to add a check for this in all the other nodes.

[delvedor]

I'll check if I can implement it without compromising the performances.
This router should be as light as possible.

[hekike]

@delvedor thanks!

[i5ting]

@hekike maybe use store is a good choice!

router.on('GET', '/example', (req, res, params, store) => {
  assert.equal(store, { message: 'hello world' })
  ....
  store.versions[req.query.version](req, res)
}, { 
  versions: {
    '1.2.3': function(req, res) {},
    '1.2.4': function(req, res) {}
  } 
})

[delvedor]

@i5ting it could be a solution, but the problem is that we should use semver, which as far as I know uses regular expressions massively.

To lower the friction that this change will introduce we should find a way (lol) to store the different handlers in the node and define which one use inside the find algorithm, based on the presence of the Accept-Version header.

I'm working on this.

[delvedor]

@hekike I'm wondering if we want to support a semver range in the declaration:

router.on('GET', '/', '>=2.5.0', (req, res, params) => {
  res.end('{"message":"hello world 1"}')
})

[hekike]

How about that routes have a fix version number like 1.0.0, 1.1.0 and 2.0.0 and the client tells it's expectation through accept-version header with semver ranges like ~1.0.0, ^1.0.0 or 2.x and the router finds the handler with the greatest fitting version.

For example:

router.on('GET', '/', '1.1.0', (req, res, params) => {
  res.end('hello world 1.1.0')
})
router.on('GET', '/', '1.2.0', (req, res, params) => {
  res.end('hello world 1.2.0')
})

// 'accept-version': '^1.1.0' => 'hello world 1.2.0'

Example for finding the greatest version from node-restify:

var semver = require('semver');
...
var reqVersion = req.version();  // accept-version header
var maxVersion;
var maxVersionIndex;

candidates.forEach(function forEach(candidate, idx) {
    var version = semver.maxSatisfying(
        candidate.version,
        reqVersion
    );

    if (version) {
        // Is this version greater?
        if (!maxVersion || semver.gt(version, maxVersion)) {
            maxVersion = version;
            maxVersionIndex = idx;
        }
    }
});

// No version found
if (_.isUndefined(maxVersionIndex)) {
    next(
        new InvalidVersionError(
            '%s is not supported by %s %s',
            req.version() || '?',
            req.method,
            req.path()
        )
    );
    return;
}

candidates[maxVersionIndex].handler(req, res, next);

[delvedor]

I thought about this and there is not an easy way to insert it without compromising the performances and the structure of the tree.
Because you should add a check every time you call lookup for the presence of the header and change the Node structure to handle multiple pair method:path with different versions.

The easiest solution without compromising the performances is to build a wrapper around find-my-way, like the following:
https://gist.github.com/delvedor/0cbb4aadefc5384ae17ad8305ea567e8

If you have some better solution, please make a proposal! :)

[hekike]

Okay, thanks for putting effort into it.
Your solution looks good, with a couple of versions it should be performant as well.

For now, we will handle route versioning with a middleware that sits after find-my-way:
https://github.com/restify/node-restify/blob/9a017158ca077b0ab4a4e0cad0e4709a79e2b74d/lib/plugins/conditionalHandler.js#L38

[mcollina]

Awesome. Closing this then.

[delvedor]

#79