Skip to content

Commit

Permalink
monkey_patches an issue with Pydantic causing PageLinks validation to…
Browse files Browse the repository at this point in the history
… fail (#2298)

* refactoring test

* added missing refactor

* adding breaking test

* added monkeypatch to fix issues with urls

* extended test suite to work with new type of urls

* fix pylint

* removed unused

* moved to take effect inside webserver

* using more reliable version parsing

* trying to see if CI is broken because of this patch

* if pyndatic is missing will skip patching

* adding message to disable patching

* skip module import if not required and print a message for later usage

* this pattern should avoid locking the CI

Co-authored-by: Andrei Neagu <[email protected]>
  • Loading branch information
GitHK and Andrei Neagu authored May 1, 2021
1 parent e72a398 commit 0af11b5
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 15 deletions.
42 changes: 42 additions & 0 deletions packages/service-library/src/servicelib/rest_pagination_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,48 @@
from yarl import URL


def monkey_patch_pydantic_url_regex() -> None:
# waiting for PR https://github.com/samuelcolvin/pydantic/pull/2512 to be released into
# pydantic main codebase
import pydantic

if pydantic.VERSION > "1.8.1":
raise RuntimeError(
(
"Please check that PR https://github.com/samuelcolvin/pydantic/pull/2512 "
"was merged. If already present in this version, remove this monkey_patch"
)
)

from typing import Pattern
from pydantic import networks
import re

def url_regex() -> Pattern[str]:
_url_regex_cache = networks._url_regex_cache # pylint: disable=protected-access
if _url_regex_cache is None:
_url_regex_cache = re.compile(
r"(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?" # scheme https://tools.ietf.org/html/rfc3986#appendix-A
r"(?:(?P<user>[^\s:/]*)(?::(?P<password>[^\s/]*))?@)?" # user info
r"(?:"
r"(?P<ipv4>(?:\d{1,3}\.){3}\d{1,3})(?=$|[/:#?])|" # ipv4
r"(?P<ipv6>\[[A-F0-9]*:[A-F0-9:]+\])(?=$|[/:#?])|" # ipv6
r"(?P<domain>[^\s/:?#]+)" # domain, validation occurs later
r")?"
r"(?::(?P<port>\d+))?" # port
r"(?P<path>/[^\s?#]*)?" # path
r"(?:\?(?P<query>[^\s#]+))?" # query
r"(?:#(?P<fragment>\S+))?", # fragment
re.IGNORECASE,
)
return _url_regex_cache

networks.url_regex = url_regex


monkey_patch_pydantic_url_regex()


class PageMetaInfoLimitOffset(BaseModel):
limit: PositiveInt
total: NonNegativeInt
Expand Down
97 changes: 82 additions & 15 deletions packages/service-library/tests/test_rest_pagination_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,28 @@ def test_empty_data_is_converted_to_list():
assert model_instance.data == []


def test_paginating_data():
@pytest.mark.parametrize(
"base_url",
[
"http://site.com",
"http://site.com/",
"http://some/random/url.com",
"http://some/random/url.com/",
"http://s.s.s.s.subsite.site.com",
"http://s.s.s.s.subsite.site.com/",
"http://10.0.0.1.nip.io/",
"http://10.0.0.1.nip.io:8091/",
"http://10.0.0.1.nip.io",
"http://10.0.0.1.nip.io:8091",
],
)
def test_paginating_data(base_url):
# create random data
total_number_of_data = 29
limit = 9
offset = 0
partial_data = [range(9)]
request_url = URL("http://some/random/url.com?some=1&random=4&query=true")
request_url = URL(f"{base_url}?some=1&random=4&query=true")

# first "call"
model_instance: PageResponseLimitOffset = PageResponseLimitOffset.paginate_data(
Expand All @@ -75,11 +90,27 @@ def test_paginating_data():
total=total_number_of_data, count=len(partial_data), limit=limit, offset=offset
)
assert model_instance.links == PageLinks(
self=f"http://some/random/url.com?some=1&random=4&query=true&offset={offset}&limit={limit}",
first=f"http://some/random/url.com?some=1&random=4&query=true&offset=0&limit={limit}",
self=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset={offset}&limit={limit}"
)
),
first=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=0&limit={limit}"
)
),
prev=None,
next=f"http://some/random/url.com?some=1&random=4&query=true&offset=9&limit={limit}",
last=f"http://some/random/url.com?some=1&random=4&query=true&offset=27&limit={limit}",
next=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=9&limit={limit}"
)
),
last=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=27&limit={limit}"
)
),
)

# next "call"s
Expand All @@ -100,11 +131,31 @@ def test_paginating_data():
offset=offset + i * limit,
)
assert model_instance.links == PageLinks(
self=f"http://some/random/url.com?some=1&random=4&query=true&offset={offset + i*limit}&limit={limit}",
first=f"http://some/random/url.com?some=1&random=4&query=true&offset=0&limit={limit}",
prev=f"http://some/random/url.com?some=1&random=4&query=true&offset={offset + i*limit-limit}&limit={limit}",
next=f"http://some/random/url.com?some=1&random=4&query=true&offset={offset + i*limit+limit}&limit={limit}",
last=f"http://some/random/url.com?some=1&random=4&query=true&offset=27&limit={limit}",
self=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset={offset + i*limit}&limit={limit}"
)
),
first=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=0&limit={limit}"
)
),
prev=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset={offset + i*limit-limit}&limit={limit}"
)
),
next=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset={offset + i*limit+limit}&limit={limit}"
)
),
last=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=27&limit={limit}"
)
),
)

# last "call"
Expand All @@ -124,9 +175,25 @@ def test_paginating_data():
offset=offset + 3 * limit,
)
assert model_instance.links == PageLinks(
self=f"http://some/random/url.com?some=1&random=4&query=true&offset={offset+3*limit}&limit={limit}",
first=f"http://some/random/url.com?some=1&random=4&query=true&offset=0&limit={limit}",
prev=f"http://some/random/url.com?some=1&random=4&query=true&offset=18&limit={limit}",
self=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset={offset+3*limit}&limit={limit}"
)
),
first=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=0&limit={limit}"
)
),
prev=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=18&limit={limit}"
)
),
next=None,
last=f"http://some/random/url.com?some=1&random=4&query=true&offset=27&limit={limit}",
last=str(
URL(base_url).with_query(
f"some=1&random=4&query=true&offset=27&limit={limit}"
)
),
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

from aiohttp import web
from servicelib.application import create_safe_application
from servicelib.rest_pagination_utils import monkey_patch_pydantic_url_regex

monkey_patch_pydantic_url_regex()

from ._meta import WELCOME_MSG
from .activity import setup_activity
Expand Down

0 comments on commit 0af11b5

Please sign in to comment.