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

Allow using wildcard in request URI #36

Closed
daanklijn opened this issue Aug 3, 2020 · 6 comments
Closed

Allow using wildcard in request URI #36

daanklijn opened this issue Aug 3, 2020 · 6 comments

Comments

@daanklijn
Copy link

Currently when using the httpserver.expect_request function, I am only allowed to use the full URI string. It would be nice if it would also accept URI's with wildcards or regex.

For instance, I have an endpoint called GET /users/{USER_ID}/role that will return the role of a user with a given USER_ID. If I would like to mock this endpoint I would have to specify a httpserver.expect_request for each of the users I am using in my tests:
httpserver.expect_request("/users/1/role").respond_with_json("admin")
httpserver.expect_request("/users/2/role").respond_with_json("admin")
httpserver.expect_request("/users/3/role").respond_with_json("admin")
It would be way more convenient if we could just pass in a wildcard instead:
httpserver.expect_request("/users/*/role").respond_with_json("admin")

@csernazs
Copy link
Owner

csernazs commented Aug 3, 2020

Hi,

This is the dup of #34. Please read my answer there.

As it came up from two different people, I think I'll add some basic support for this - however I still do not want to add any routing implementation as it would be a reimplementation of Flask. It means that the handlers will be looked up in a sequential order (in the same order as there were defined) with no respect of how narrow is the match.

What I mean by "basic" is the following:

  • accepting a regexp object (eg the object returned by re.compile()), in such case the match will be performed.
  • accepting an URIPattern object, which would be defined as an abstract class, and it would have a match() method of it (eg. it would be handled in the same way as the regexp object).

The library itself would have no implementation of the URIPattern class, it would be up to the developer to implement something (eg prefix/suffix matches, glob matching, etc).

@daanklijn
Copy link
Author

daanklijn commented Aug 3, 2020

Thanks for you answer! Just read your answer in #34 and it's great that we are able to do this 👍. But it might indeed be good to also implement a way to also accept regex as that seems a bit more user-friendly. If you need any help I'll be happy to assist implementing it 👌

csernazs added a commit that referenced this issue Aug 3, 2020
Extend URI matching functionality which now accepts:

* a URIPattern object, whose match() method will be called
* a compiled regexp returned by re.compile(). In this case the
  URI will be matched against the regexp.

In both cases the URI will be an absolute path starting with a slash.

Fixes #34 and #36.
csernazs added a commit that referenced this issue Aug 3, 2020
Extend URI matching functionality which now accepts:

* a URIPattern object, whose match() method will be called
* a compiled regexp returned by re.compile(). In this case the
  URI will be matched against the regexp.

In both cases the URI will be an absolute path starting with a slash.

Fixes #34 and #36.
@csernazs
Copy link
Owner

csernazs commented Aug 3, 2020

cc: @arturpat

I've submitted the PR #37 to implement pattern matching. Could you please have a look at it?
Obviously I accept any comments on the implementation and also I'm interested if this would make your life easier regarding this library. Having an isinstance on the object returned by re.compile is quite awkward but I could not find any better.

The key points are:

  • regexps can be provided by re.compile (this is good for performance, but also to distinguish string from regexp)
  • URIPattern is defined as abstract class, so it cannot be instantiated directly, but if you create a subclass and define the match() method, it will be used to match the URI. I was thinking about allowing providing of any callable, but this URIPattern can be parametrized (which will be true in 99% of cases, such as prefix match or glob match), and it is more type safe (in python term: easier to specify a pattern hint, better IDE support, etc)

I also kept the possibility to specify any object whose __eq__ is properly written as I suggested in #34, as I don't want to break any code (unless it is absolutely necessary).

csernazs added a commit that referenced this issue Aug 5, 2020
Extend URI matching functionality which now accepts:

* a URIPattern object, whose match() method will be called
* a compiled regexp returned by re.compile(). In this case the
  URI will be matched against the regexp.

In both cases the URI will be an absolute path starting with a slash.

Fixes #34 and #36.
csernazs added a commit that referenced this issue Aug 5, 2020
Extend URI matching functionality which now accepts:

* a URIPattern object, whose match() method will be called
* a compiled regexp returned by re.compile(). In this case the
  URI will be matched against the regexp.

In both cases the URI will be an absolute path starting with a slash.

Fixes #34 and #36.
csernazs added a commit that referenced this issue Aug 5, 2020
Extend URI matching functionality which now accepts:

* a URIPattern object, whose match() method will be called
* a compiled regexp returned by re.compile(). In this case the
  URI will be matched against the regexp.

In both cases the URI will be an absolute path starting with a slash.

Fixes #34 and #36.
@csernazs
Copy link
Owner

csernazs commented Aug 5, 2020

I've merged the PR request, thanks for your comments!
I think I'll do a release later today.

@csernazs
Copy link
Owner

csernazs commented Aug 5, 2020

I've released 0.3.5.
I'm closing this issue in the hope that packages on pypi are also usable (I've already checked, but who knows..). :) If you see any problem, feel free to open a new issue.

Thanks for using this library!

@csernazs csernazs closed this as completed Aug 5, 2020
@arturpat
Copy link

arturpat commented Aug 6, 2020

Hey,
thanks for getting back to this, that's cool! I will probably try to upgrade and battle-test your change in upcoming months :)

Thanks for developing this great library,
Artur

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants