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.

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

query strings and ordering #82

Closed
em-cliqz opened this issue Dec 3, 2012 · 23 comments
Closed

query strings and ordering #82

em-cliqz opened this issue Dec 3, 2012 · 23 comments

Comments

@em-cliqz
Copy link

em-cliqz commented Dec 3, 2012

I'm having some trouble with query strings. I wish to intercept a path like '/path/load' which can accept multiple parameters, like '?b=123&a=456'. The trouble is that "a" "b" could be in any order (since it'd still be a valid URL). Is there some way I could do parameter matching like the POST request using an object map instead of constructing this string?

@pgte
Copy link
Member

pgte commented Dec 3, 2012

You can use a path filtering regexp or function that will be called on every request by using .filteringPath():
https://github.com/flatiron/nock#path-filtering

You can then take that chance to reorder the query string if you will.

@em-cliqz
Copy link
Author

em-cliqz commented Dec 3, 2012

That is what I ended up doing. I created a filtering function like below.

function normalize_get_path( str ) {
    var u = url.parse( str );
    var query = querystring.parse( u.query );

    var params = [];
    for( var param in query )
        params.push( param );
    params.sort();

    var oqs = '';
    for( var param in params ) {
        if( oqs.length )
            oqs += '&';

        oqs += encodeURIComponent( params[param] );
        oqs += "=";
        oqs += encodeURIComponent( query[params[param]] );
    }

    var c = url.format( { pathname: u.pathname, search: oqs } ); //NODE: 'query' is ignored, must use search
    return c;
}

@pgte
Copy link
Member

pgte commented Dec 7, 2012

Closing #82.

@jtremback
Copy link

Shouldn't this be in the library? Query strings are pretty common things.

@wejendorp
Copy link

Agree with @jtremback.

@pgte pgte reopened this Jul 23, 2014
@wejendorp
Copy link

Something like the body matcher code would be awesome for query params as well.

@samccone
Copy link

👍 to this idea / problem

@samccone
Copy link

@pgte would you be open to something like

.get("/x/x")
.allowAnyQueryString()
.reply(..............)

If so I can do this

@djensen47
Copy link

👍

1 similar comment
@mtford90
Copy link

mtford90 commented Dec 6, 2014

👍

@LoicMahieu
Copy link

I'm mocking an external ugly API which has a lot of query string stuffs...
At this time we write that:

var data = { foo: 'bar' };
nock.get('/some/path' + querystring.stringify(data))

which is quite ugly but now, ordering is the problem.

As @wejendorp said, marcher like body would be great! 👍

@doublerebel
Copy link

👍

@floatdrop
Copy link

💯 Any plans on this?

@pgte
Copy link
Member

pgte commented Feb 12, 2015

Nock is currently not query-string aware, but I agree this is annoying.

We can either
a) make a scope query-string aware by defau (making it match the query object instead of the string). Here an option would be needed to switch this off.
b) make it easy for the programmer to create a query string filter that they can activate with something like this:

scope.filteringPath(nock.orderQueryString);

I'm inclined to support the first option since it's more friendly and I suspect covers most of the cases.
@ierceg @svnlto thoughts?

@svnlto
Copy link
Member

svnlto commented Feb 12, 2015

I also think the first option fits nicer into nock's api.

@ierceg
Copy link
Contributor

ierceg commented Feb 12, 2015

@pgte I agree.

@doublerebel
Copy link

The way I was imagining, was an extension to the API. Similar to the above proposal for .allowAnyQueryString():

.query() // Allow any query string
.query(true) // Allow any query string
.query(false) // Disallow any query string
.query({key: "value"}) // Match query object

This matches up with the superagent syntax used in nock's test suite.

In the best case, we could parse the query string from the existing path, and treat that path as a shorthand for .query, enabling backwards compatibility. i.e.:

.get('/users/1?password=XXX')
.get('/users/1').query({password: "XXX"}) // same result

And also imagining filteringQuery() as in:

.filteringQuery(function(queryObj) {
    if (queryObj.password == ...);
});

EDIT: added .query() and .query(false)

@kevinhodges
Copy link

Has anyone started a branch for this? I'm happy to give it a crack, from a first look at the codebase we'd want to extend the scope => intercept function with a .query(), but then it should probably use a lot of the .match() functionality, any thoughts/ideas on the best solution let me know.

In terms of the interface, the best solutions are not gonna be backwards compatible, something like

var google = nock('http://www.google.com')
.get( '/', {
  body: {
    foo: 'bar'
  },
  query: {
    foo: 'bar'
  }
}

feels quite nice.

@pgte
Copy link
Member

pgte commented Mar 4, 2015

@kevinhodges I wouldn't want to break backwards compatibility.

One way I see this working is if, instead of a string with the path you provide an object like this:

var google = nock('http://www.google.com')
.get({
  path:  '/',
  query: {
    foo: 'bar',
    bar: 'foo'
  }
});

this should also work:

var google = nock('http://www.google.com')
.get({
  path:  '/',
  query: 'foo=bar&bar=foo'
});

Care to give it a go?

@jezeniel
Copy link

@pgte what is the news about this issue? Is this implemented?

@pgte
Copy link
Member

pgte commented Jun 26, 2015

It's not implemented. Would appreciate some help here.

@pgte
Copy link
Member

pgte commented Jul 2, 2015

Fixed by #341

@pgte pgte closed this as completed Jul 2, 2015
@lock
Copy link

lock bot commented Sep 13, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests