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

Add params-in-body support to DELETE #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pytumblr/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ def _send_post(self, blogname, params):

return self.send_api_request("post", url, params, valid_options)

def send_api_request(self, method, url, params={}, valid_parameters=[], needs_api_key=False):
def send_api_request(self, method, url, params={}, valid_parameters=[], needs_api_key=False, use_body=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while i'm not exactly sure what to do here, this feels a bit weird for a few reasons:

  1. this is a parameter that is only really available when method == "delete"
  2. this means that we cannot use both query parameters and body parameters

i think the ideal here would be to have two separate arguments, query_params and body_params so we have the ability to send both. its not following any best practices to go that route, but it does cover all the bases so we can handle oddities that exist in various legacy api routes.

"""
Sends the url with parameters to the requested url, validating them
to make sure that they are what we expect to have passed to us
Expand All @@ -557,6 +557,7 @@ def send_api_request(self, method, url, params={}, valid_parameters=[], needs_ap
:param params: a dict, the parameters used for the API request
:param valid_parameters: a list, the list of valid parameters
:param needs_api_key: a boolean, whether or not your request needs an api key injected
:param use_body: a boolean, put params in request body rather than the URL

:returns: a dict parsed from the JSON response
"""
Expand All @@ -577,6 +578,6 @@ def send_api_request(self, method, url, params={}, valid_parameters=[], needs_ap
if method == "get":
return self.request.get(url, params)
elif method == "delete":
return self.request.delete(url, params)
return self.request.delete(url, params, use_body)
else:
return self.request.post(url, params, files)
18 changes: 14 additions & 4 deletions pytumblr/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,33 @@ def post(self, url, params={}, files=[]):
except HTTPError as e:
return self.json_parse(e.response)

def delete(self, url, params):
def delete(self, url, params, use_body=False):
"""
Issues a DELETE request against the API, properly formatting the params

:param url: a string, the url you are requesting
:param params: a dict, the key-value of all the paramaters needed
in the request
:param use_body: a boolean, put params in request body rather than the URL

:returns: a dict parsed of the JSON response
"""
url = self.host + url
if params:
url = url + "?" + urllib.parse.urlencode(params)

try:
resp = requests.delete(url, allow_redirects=False, headers=self.headers, auth=self.oauth)
if use_body:
data = urllib.parse.urlencode(params)
if not PY3:
data = str(data)
resp = requests.delete(url, data=data, headers=self.headers, auth=self.oauth)
else:
if params:
url = url + "?" + urllib.parse.urlencode(params)
resp = requests.delete(url, allow_redirects=False, headers=self.headers, auth=self.oauth)
except TooManyRedirects as e:
resp = e.response
except HTTPError as e:
resp = e.response

return self.json_parse(resp)

Expand Down
8 changes: 8 additions & 0 deletions tests/test_pytumblr.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,14 @@ def test_api_delete(self, mock_delete):
response = self.client.send_api_request('delete', api_url, {'param1': 'foo', 'param2': 'bar'}, ['param1', 'param2'], False)
assert response == []

@mock.patch('requests.delete')
def test_api_delete_body(self, mock_delete):
mock_delete.side_effect = wrap_response('{"meta": {"status": 200, "msg": "OK"}, "response": []}')

api_url = '/v2/some/api'
response = self.client.send_api_request('delete', api_url, {'param1': 'foo', 'param2': 'bar'}, ['param1', 'param2'], False, use_body=True)
assert response == []


if __name__ == "__main__":
unittest.main()