-
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 all 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 |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| from typing import Any, Dict | ||
|
|
||
| from python_anvil.constants import VALID_HOSTS | ||
| from python_anvil.http import HTTPClient | ||
|
|
||
|
|
||
|
|
@@ -161,3 +162,22 @@ 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.""" | ||
|
|
||
| def get_url(self): | ||
| return "" # Not used since we expect full URLs | ||
|
Contributor
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
Contributor
Author
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
Contributor
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
Contributor
Author
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 VALID_HOSTS): | ||
| raise ValueError(f"URL must start with one of: {', '.join(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 |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| CreateEtchPacketPayload, | ||
| ForgeSubmitPayload, | ||
| ) | ||
| from python_anvil.constants import VALID_HOSTS | ||
|
|
||
| from ..api_resources.payload import FillPDFPayload | ||
| from . import payloads | ||
|
|
@@ -396,3 +397,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), | ||
| *[(host + "/some-endpoint", False) for host in VALID_HOSTS], | ||
| ], | ||
| ) | ||
| @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", | ||
|
Contributor
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
Contributor
Author
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), | ||
| *[(host + "/some-endpoint", False) for host in VALID_HOSTS], | ||
| ], | ||
| ) | ||
| @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", | ||
|
Contributor
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
Contributor
Author
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