basicallydan / interfake

:computer: Fake APIs for prototypes & automated tests.
Other
805 stars 39 forks source link

Regular expression doesn't work for question symbol #40

Closed AlionaKastsevich closed 8 years ago

AlionaKastsevich commented 9 years ago

I tried to use regular expression in such way: interfake.get(/\/asofdate=[0-9]\?exp/).status(200);

i expect the url to be availiable in such way: /asofdate=1?exp

but because of question mark it is unavailiable.

also i tried it in JSON, like so:

"url": { "pattern" : "/asofdate=[0-9]\?exp", "regexp" : true }

But it doesn't work too.

basicallydan commented 9 years ago

Okay I will be looking at this in the next hour or so, that's no good!

basicallydan commented 9 years ago

Hey @AlionaKastsevich - I've figured out why this is happening. It's because of the way Interfake parses query string parameters. I can fix it but it won't be super-quick.

What I'd say for now is instead do this:

"url": {
  "pattern" : "/asofdate=[0-9]",
  "regexp" : true
},
"query" : {
  "exp": ""
}

...or using the fluent syntax:

interfake.get(/\/asofdate=[0-9]/).query({ exp : '' }).status(200);

That should produce the desired result. You can even make the query string param a regular expression, e.g. exp: /[0-9]?/ if you want to, then ?exp=2 and ?exp=5 will work too.

However I understand your use case and it should be supported. While I'm working on fixing that, above is a workaround/other solution.

Does it work for you?

I should note BTW that using the methods I suggested makes the requirements for query strings less strict about order, which I think in most cases is what people are expecting. Using regular expressions to enforce order in query string params might be overkill. Were you looking to enforce some kind of order in the params? If not, then using the query option is probably better anyway!

Lemme know how you get on - thanks.

P.S. Congrats on your first GitHub issue! I'm totally honoured that it was on my repo :+1:

AlionaKastsevich commented 9 years ago

=) thank you for your grats and fast answer. it would have been very convinient for me to use queries with regexp in JSON better when using .query(). (i didn't say it in first message as i didn't think it matters if it is in query or not, i was concentrated on question symbol). Smth like: "request": { "url": "/myurlgoesHere", "query" : { "asofdate": { "pattern" : "[0-9]", "regexp" : true } }, "method": "get" } Expected string: /myurlgoesHere?asofdate=123 Does interfake has such ability?

The other question is why when i use such notation interfake.get(/myurlgoesHere/).query({asofdate: /[0-9]{1}/}).status(200); the links such as: /myurlgoesHere /myurlgoesHere1 /myurlgoesHere?asofd?anyletters

are still available?

basicallydan commented 9 years ago

Expected string: /myurlgoesHere?asofdate=123 Does interfake has such ability?

Yes, although this won't work if your specification is in a JSON file. It'll only work using JavaScript, either with the fluent notation like you suggested, or with JavaScript objects. In other words, "regexp":true is not supported for querystrings. However, there's no reason why it shouldn't and that would in fact make Interfake a little better. For now, the following should work:

interfake.get(/myurlgoesHere/).query({ asofdate : /[0-9]{1}/}).status(200);

Which will match /myurlgoeshere?asofdate=1 to /myurlgoeshere?asofdate=9. Are you sure you wanted to limit the length of the date to 1 though? I get the impression that /[0-9]+/ would be more appropriate here. Just a thought, since you expected /myurlgoesHere?asofdate=123 to work.

The other question is why when i use such notation

Ah! That's a good question. The regex supplied will be greedy by default. Try putting interfake.get(/myurlgoesHere$/) and that will probably fix it (haven't tested, in a bit of a rush!).

Also don't forget the i flag to make sure it's case-insensitive.

So to summarise, I think you want this:

interfake.get(/myurlgoesHere$/i).query({ asofdate : /[0-9]+/}).status(200);

It wil match:

But it should not match:

Does that help? :)

AlionaKastsevich commented 9 years ago

Yes, it helped=) But it would be a bomb if you'll support regexp in query in JSON Also I have a question, how can I use JSON without hardcoding it in Interfake.get().body({ 'hardcode': 'here' }). I wanna have JSON separately and use regexp for query as I mentioned before.

basicallydan commented 9 years ago

If you use require or import to bring in a JSON file you can put the resulting object into the body. E.g.:

var body = require('bodyfile.json');

interfake.get('/woo').body(body);

Is this what you mean? :smile: On 10 Nov 2015 02:33, "Aliona_Kastsevich" notifications@github.com wrote:

Yes, it helped=) But it would be a bomb if you'll support regexp in query in JSON Also I have a question, how can I use JSON without hardcoding it in Interfake.get().body({ 'hardcode': 'here' }). I wanna have JSON separately and use regexp for query as I mentioned before.

— Reply to this email directly or view it on GitHub https://github.com/basicallydan/interfake/issues/40#issuecomment-155167053 .

AlionaKastsevich commented 9 years ago

yes=) thank you for fast answers)

AlionaKastsevich commented 9 years ago

basicallydan, i think we've found a solution.

Do you mind adding "this.app = app;" into server.js line 280 to return express app instance? Or please provide us any way to get express instance.

basicallydan commented 9 years ago

@AlionaKastsevich Hey,

I was going to suggest using express to do stuff like callback functions in .body, but that'd work too. Are you sure that Express isn't a more appropriate library for what you're trying to do?

You're more than welcome to make pull requests for features as small as this if you think it'd solve your problems, by the way - it'd be quicker than asking me to do it :smile: I'm kinda on a trip at the mo and my Internet access is very unreliable.

samarpanda commented 8 years ago

@AlionaKastsevich Do you still need this change?

//line no. 280
this.app = app;
basicallydan commented 8 years ago

@samarpanda if you have a PR for it go ahead :)

basicallydan commented 8 years ago

@samarpanda @AlionaKastsevich I've this.expressApp = app; into server.js in v1.19. Hopefully this won't be an issue for you anymore :)

samarpanda commented 8 years ago

Thanks @basicallydan. Sorry, was late getting you the PR.

basicallydan commented 8 years ago

No worries @samarpanda I was making a release anyway so it was convenient :)