-
Notifications
You must be signed in to change notification settings - Fork 11
Create class to handle absolute urls #81
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
Changes from 18 commits
c24c6b1
63bbd28
ac3bc46
483d65d
8d9fd88
a965bb5
2856e82
4e95d12
96994da
be82f1f
d5fc124
896f6ac
55b0921
917932d
311965f
5911f2f
8b935ca
2d05029
8e4897d
9f052a8
1af0e33
e608a79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,3 +161,29 @@ class PlainRequest(BaseAnvilHttpRequest): | |
|
||
def get_url(self): | ||
return f"{self.API_HOST}/{self.API_BASE}" | ||
|
||
|
||
class FullyQualifiedRequest(BaseAnvilHttpRequest): | ||
"""A request class that validates URLs point to Anvil domains.""" | ||
|
||
VALID_HOSTS = [ | ||
"https://app.useanvil.com", | ||
# Future Anvil specific URLs | ||
] | ||
|
||
def get_url(self): | ||
return "" # Not used since we expect full URLs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to define this? the base of this func throws an error, prob can keep if we don't want people to use this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is intentionally defined as an empty string, otherwise request_rest tries to append whatever URL is passed in to whatever is returned in get_url There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if that's the case then yeah def, but doesn't this function get inherited from here? I'm not a python guy so idk how inheritance works here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it does get inherited from there but we are overriding it here by returning an empty string |
||
|
||
def _validate_url(self, url): | ||
if not any(url.startswith(host) for host in self.VALID_HOSTS): | ||
raise ValueError( | ||
f"URL must start with one of: {', '.join(self.VALID_HOSTS)}" | ||
) | ||
|
||
def get(self, url, params=None, **kwargs): | ||
self._validate_url(url) | ||
return super().get(url, params, **kwargs) | ||
|
||
def post(self, url, data=None, **kwargs): | ||
self._validate_url(url) | ||
return super().post(url, data, **kwargs) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -396,3 +396,61 @@ def test_minimum_valid_data_submission(m_request_post, anvil): | |
anvil.forge_submit(payload=payload) | ||
assert m_request_post.call_count == 1 | ||
assert _expected_data in m_request_post.call_args | ||
|
||
def describe_rest_request_absolute_url_behavior(): | ||
@pytest.mark.parametrize( | ||
"url, should_raise", | ||
[ | ||
("some/relative/path", True), | ||
("https://external.example.com/full/path/file.pdf", True), | ||
("https://app.useanvil.com/api/v1/some-endpoint", False), | ||
], | ||
) | ||
@mock.patch("python_anvil.api_resources.requests.AnvilRequest._request") | ||
def test_get_behavior(mock_request, anvil, url, should_raise): | ||
mock_request.return_value = (b"fake_content", 200, {}) | ||
rest_client = anvil.request_fully_qualified() | ||
|
||
if should_raise: | ||
with pytest.raises( | ||
ValueError, | ||
match="URL must start with one of: https://app.useanvil.com", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally this uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
): | ||
rest_client.get(url) | ||
else: | ||
rest_client.get(url) | ||
mock_request.assert_called_once_with( | ||
"GET", | ||
url, | ||
params=None, | ||
retry=True, | ||
) | ||
|
||
@pytest.mark.parametrize( | ||
"url, should_raise", | ||
[ | ||
("some/relative/path", True), | ||
("https://external.example.com/full/path/file.pdf", True), | ||
("https://app.useanvil.com/api/v1/some-endpoint", False), | ||
], | ||
) | ||
@mock.patch("python_anvil.api_resources.requests.AnvilRequest._request") | ||
def test_post_behavior(mock_request, anvil, url, should_raise): | ||
mock_request.return_value = (b"fake_content", 200, {}) | ||
rest_client = anvil.request_fully_qualified() | ||
|
||
if should_raise: | ||
with pytest.raises( | ||
ValueError, | ||
match="URL must start with one of: https://app.useanvil.com", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally this uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
): | ||
rest_client.post(url, data={}) | ||
else: | ||
rest_client.post(url, data={}) | ||
mock_request.assert_called_once_with( | ||
"POST", | ||
url, | ||
json={}, | ||
retry=True, | ||
params=None, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idt this is actually used anywhere - looks like the func above is used here. a couple other placess too, probably should use this new version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, request_rest also doesnt seem to be called anywhere. I guess someone could use these if they wanted to when making other requests to Anvil