-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix request creation when headers are the default #198
Changes from all commits
e97d966
bc7a8fd
297089e
84fbd34
fae4b7e
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 |
---|---|---|
|
@@ -42,16 +42,15 @@ def __init__(self, domain, telemetry=True): | |
|
||
def post(self, url, data=None, headers=None): | ||
request_headers = self.base_headers.copy() | ||
request_headers.update(headers) | ||
response = requests.post(url=url, data=json.dumps(data), | ||
headers=request_headers) | ||
request_headers.update(headers or {}) | ||
response = requests.post(url=url, json=data, headers=request_headers) | ||
return self._process_response(response) | ||
|
||
def get(self, url, params=None, headers=None): | ||
request_headers = self.base_headers.copy() | ||
request_headers.update(headers) | ||
request_headers.update(headers or {}) | ||
response = requests.get(url=url, params=params, headers=request_headers) | ||
return response.text | ||
return self._process_response(response) | ||
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. Some response formats where not taken into account. This was the only method not calling |
||
|
||
def _process_response(self, response): | ||
return self._parse(response).content() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,7 @@ def login(self, client_id, username, password, connection, id_token=None, | |
'device': device, | ||
'grant_type': grant_type, | ||
'scope': scope, | ||
}, | ||
headers={'Content-Type': 'application/json'} | ||
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. All these args were removed because they match the default behavior https://github.com/auth0/auth0-python/blob/master/auth0/v3/authentication/base.py#L24 |
||
} | ||
) | ||
|
||
def signup(self, client_id, email, password, connection, username=None, | ||
|
@@ -67,8 +66,7 @@ def signup(self, client_id, email, password, connection, username=None, | |
|
||
return self.post( | ||
'https://{}/dbconnections/signup'.format(self.domain), | ||
data=body, | ||
headers={'Content-Type': 'application/json'} | ||
data=body | ||
) | ||
|
||
def change_password(self, client_id, email, connection, password=None): | ||
|
@@ -82,6 +80,5 @@ def change_password(self, client_id, email, connection, password=None): | |
'email': email, | ||
'password': password, | ||
'connection': connection, | ||
}, | ||
headers={'Content-Type': 'application/json'} | ||
} | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ def get(self, url, params=None): | |
def post(self, url, data=None): | ||
headers = self.base_headers.copy() | ||
|
||
response = requests.post(url, data=json.dumps(data or {}), headers=headers) | ||
response = requests.post(url, json=data, headers=headers) | ||
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. No longer need to explicitly convert python dictionaries to "JSON text" |
||
return self._process_response(response) | ||
|
||
def file_post(self, url, data=None, files=None): | ||
|
@@ -59,13 +59,13 @@ def file_post(self, url, data=None, files=None): | |
def patch(self, url, data=None): | ||
headers = self.base_headers.copy() | ||
|
||
response = requests.patch(url, data=json.dumps(data or {}), headers=headers) | ||
response = requests.patch(url, json=data, headers=headers) | ||
return self._process_response(response) | ||
|
||
def put(self, url, data=None): | ||
headers = self.base_headers.copy() | ||
|
||
response = requests.put(url, data=json.dumps(data or {}), headers=headers) | ||
response = requests.put(url, json=data, headers=headers) | ||
return self._process_response(response) | ||
|
||
def delete(self, url, params=None): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,10 +50,25 @@ def test_post(self, mock_post): | |
|
||
data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'}) | ||
|
||
mock_post.assert_called_with(url='the-url', data='{"a": "b"}', | ||
mock_post.assert_called_with(url='the-url', json={'a': 'b'}, | ||
headers={'c': 'd', 'Content-Type': 'application/json'}) | ||
|
||
self.assertEqual(data, {'x': 'y'}) | ||
|
||
@mock.patch('requests.post') | ||
def test_post_with_defaults(self, mock_post): | ||
ab = AuthenticationBase('auth0.com', telemetry=False) | ||
|
||
mock_post.return_value.status_code = 200 | ||
mock_post.return_value.text = '{"x": "y"}' | ||
|
||
# Only required params are passed | ||
data = ab.post('the-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. no |
||
|
||
mock_post.assert_called_with(url='the-url', json=None, | ||
headers={'Content-Type': 'application/json'}) | ||
|
||
self.assertEqual(data, {'x': 'y'}) | ||
|
||
@mock.patch('requests.post') | ||
def test_post_includes_telemetry(self, mock_post): | ||
|
@@ -67,7 +82,7 @@ def test_post_includes_telemetry(self, mock_post): | |
self.assertEqual(mock_post.call_count, 1) | ||
call_kwargs = mock_post.call_args[1] | ||
self.assertEqual(call_kwargs['url'], 'the-url') | ||
self.assertEqual(call_kwargs['data'], '{"a": "b"}') | ||
self.assertEqual(call_kwargs['json'], {'a': 'b'}) | ||
headers = call_kwargs['headers'] | ||
self.assertEqual(headers['c'], 'd') | ||
self.assertEqual(headers['Content-Type'], 'application/json') | ||
|
@@ -157,6 +172,35 @@ def test_post_error_with_no_response_text(self, mock_post): | |
'a0.sdk.internal.unknown') | ||
self.assertEqual(context.exception.message, '') | ||
|
||
@mock.patch('requests.get') | ||
def test_get(self, mock_get): | ||
ab = AuthenticationBase('auth0.com', telemetry=False) | ||
|
||
mock_get.return_value.status_code = 200 | ||
mock_get.return_value.text = '{"x": "y"}' | ||
|
||
data = ab.get('the-url', params={'a': 'b'}, headers={'c': 'd'}) | ||
|
||
mock_get.assert_called_with(url='the-url', params={'a': 'b'}, | ||
headers={'c': 'd', 'Content-Type': 'application/json'}) | ||
|
||
self.assertEqual(data, {'x': 'y'}) | ||
|
||
@mock.patch('requests.get') | ||
def test_get_with_defaults(self, mock_get): | ||
ab = AuthenticationBase('auth0.com', telemetry=False) | ||
|
||
mock_get.return_value.status_code = 200 | ||
mock_get.return_value.text = '{"x": "y"}' | ||
|
||
# Only required params are passed | ||
data = ab.get('the-url') | ||
|
||
mock_get.assert_called_with(url='the-url', params=None, | ||
headers={'Content-Type': 'application/json'}) | ||
|
||
self.assertEqual(data, {'x': 'y'}) | ||
|
||
@mock.patch('requests.get') | ||
def test_get_includes_telemetry(self, mock_get): | ||
ab = AuthenticationBase('auth0.com') | ||
|
@@ -176,4 +220,4 @@ def test_get_includes_telemetry(self, mock_get): | |
self.assertIn('User-Agent', headers) | ||
self.assertIn('Auth0-Client', headers) | ||
|
||
self.assertEqual(data, '{"x": "y"}') | ||
self.assertEqual(data, {"x": "y"}) |
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.
this was the bug. When headers was
None
this call would throw.