hoxy icon indicating copy to clipboard operation
hoxy copied to clipboard

Pass variables gained from route-pattern to request object

Open editedredx opened this issue 10 years ago • 10 comments
trafficstars

It would be nice to be able to use the route variables mentioned in the url/fullUrl interception parameters. Something in the line of this:

proxy.intercept({
  phase: 'request',
  method: 'GET',
  url: '/dir/:page'
}, function(req, resp, cycle) {
  console.log(req.route.namedParams.page);
});

editedredx avatar Sep 01 '15 13:09 editedredx

It might also be nice to also support regex capturing groups:

proxy.intercept({
  phase: 'request',
  url: /bar\-(\d+)/
}, function(req) {
  console.log(req.urlData[1]);
});
proxy.intercept({
  phase: 'request',
  url: '/dir/:page'
}, function(req) {
  console.log(req.urlData.namedParams.page);
});

Thoughts?

greim avatar Sep 02 '15 00:09 greim

I assume that ...namedParams.name is used because route-patterns already provides it as this? Mixing the namedParams and matches might be tricky if someone uses (for whatever reason!):

url: /bar\-(?P<namedParams>\d+)/

If mixing is the way to go, I'd find

req.matches[1]
req.matches.page

more intuitive and cleaner. Which makes me think if it'll be of use to match regexes on other request/response fields too. If so naming it urlData or alike would be safer indeed.

ysf avatar Sep 02 '15 10:09 ysf

Another way to prevent mixing problems could be to use a getter:

proxy.intercept({
  phase: 'request',
  url: /bar\-(\d+)\/(?P<foo>\d+)/
}, function(req) {
  var urlData = req.getUrlData();
  console.log(urlData[0]);
  console.log(urlData.foo);
});
proxy.intercept({
  phase: 'request',
  url: '/dir/:page'
}, function(req) {
  var urlData = req.getUrlData();
  console.log(urlData.namedParams.page);
});

What getUrlData returns depends on the datatype used, regex or (route)string.

This is more in line with the getFullUrl method. It could also prevent problems when using a url and a fullUrl to intercept (although I'm not sure why you would do that) by also providing a getFullUrlData method.

EDIT Yes, namedParams was used because route-patterns already provides it that way.

editedredx avatar Sep 02 '15 11:09 editedredx

Which makes me think if it'll be of use to match regexes on other request/response fields too

Interesting. What about a more general approach where, for any filtering option, req.matches.thing is always just whatever the matching operation returned for the "thing" option. Same for resp.matches.thing. It would always be truthy, but otherwise it could be any of:

  • simple string matches: boolean (not too useful)
  • regex matches: whatever RegExp#exec() returns
  • function matches: whatever the function returns
  • url matches: whatever RoutePattern#match() returns
proxy.intercept({
  phase: 'request',
  hostname: hostname => hostname.split('-')[1]
}, function(req) {
  console.log(req.matches.hostname); // "bar" for hostname "foo-bar"
});

greim avatar Sep 02 '15 16:09 greim

I like the general approach. I'm pretty new to node/io.js and english is not my mother tongue. So excuse me asking further: I don't see yet why the hostname example makes a big difference with beeing there and not in the callback. AFAIK the main reason was to DRY code, because if you filter with a capture on something in the intercept-parameter, the chance is high that you would've to to this again in the handling callback. How I understand the API is that I could provide a regex/string/function to filter on multiple headerfields and if all those match/are equal to/return true on something then the callbackfunction handling the request/response would be called, is this right? If so, how would this behave:

proxy.intercept({
  hostname: hostname => hostname.split('-')[1]
  url: '/user/:username/',
}, function(req) {
  console.log(req.matches.hostname);
  console.log(req.matches.url.username);
});

Will it be called only when the url starts with /user/ and hostname.split('-')[1] is actually something? Or would hostname be 'undefined' on a hostname like 'nodashinhere'? The latter one would be unintuitive or breaking the current API.

ysf avatar Sep 02 '15 23:09 ysf

It's buried in the docs, but yes, filtering options are logically ANDed together, and would definitely remain so if this new feature is added.

proxy.intercept({
  hostname: hostname => hostname.split('-')[1]
  url: '/user/:username/',
}, function(req) {
  // if this interceptor callback is called, then...
  console.log(req.matches.hostname); // this will always be truthy...
  console.log(req.matches.url.username); // this will always be truthy...
});

Does that answer the question?

greim avatar Sep 03 '15 19:09 greim

Although, now that I think of it:

proxy.intercept({
  hostname: hostname => hostname.split('-')[1]
  url: '/user/:username/',
}, function(req) {
  // if this interceptor callback is called, then...
  console.log(req.matches.hostname); // this will always be truthy...
  console.log(req.matches.url.username); // this will always be truthy...

  // what about this:
  console.log(req.matches.fullUrl); // should this be undefined?
});

greim avatar Sep 03 '15 20:09 greim

I know that they are ANDed but here is "hostname: hostname => hostname.split('-')[1]" like the other settings a condition that has to me met? Or only a utility syntax to provide preprocessed data on headers. Means, using the examples given, the callback will only be called if the hostname contains a dash? Or will it be called anyway without magic matches for "hostname". Hope that I can explain my point somehow. Thanks for your time and work!

ysf avatar Sep 03 '15 21:09 ysf

a condition that has to me met?

Correct. The intent would be that hostname => hostname.split('-')[1] is just a conditional. It was probably a bad example. Maybe a better one would be:

var myHosts = new Set();
myHosts.add('foo.com');
myHosts.add('bar.com');

proxy.intercept({
  hostname: name => myHosts.has(name)
}, function(req) {
  console.log(req.matches.hostname); // true
}); 

greim avatar Sep 03 '15 21:09 greim

Ok, yeah, that sounds nice to me.

ysf avatar Sep 04 '15 21:09 ysf