From 1ce6662a9d7115348788972afce62a5c45199021 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Sat, 24 Nov 2018 20:02:00 +0530 Subject: [PATCH 01/12] Implement Request subclass for json requests --- scrapy/http/__init__.py | 1 + scrapy/http/request/json_request.py | 28 +++++++++++++++ tests/test_http_request.py | 55 ++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 scrapy/http/request/json_request.py diff --git a/scrapy/http/__init__.py b/scrapy/http/__init__.py index f04a9d3e5..4b2f7b33f 100644 --- a/scrapy/http/__init__.py +++ b/scrapy/http/__init__.py @@ -10,6 +10,7 @@ from scrapy.http.headers import Headers from scrapy.http.request import Request from scrapy.http.request.form import FormRequest from scrapy.http.request.rpc import XmlRpcRequest +from scrapy.http.request.json_request import JSONRequest from scrapy.http.response import Response from scrapy.http.response.html import HtmlResponse diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py new file mode 100644 index 000000000..0fdd2ddf1 --- /dev/null +++ b/scrapy/http/request/json_request.py @@ -0,0 +1,28 @@ +""" +This module implements the JSONRequest class which is a more convenient class +(than Request) to generate JSON Requests. + +See documentation in docs/topics/request-response.rst +""" + +import json + +from scrapy.http.request import Request + + +class JSONRequest(Request): + def __init__(self, *args, **kwargs): + if 'method' not in kwargs: + kwargs['method'] = 'POST' + + data = kwargs.pop('data', {}) + kwargs['body'] = json.dumps(data) + super(JSONRequest, self).__init__(*args, **kwargs) + self.headers.setdefault(b'Content-Type', b'application/json') + + def replace(self, *args, **kwargs): + """ Create a new Request with the same attributes except for those + given new values. """ + + kwargs.pop('body', None) + return super(JSONRequest, self).replace(*args, **kwargs) diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 58326a384..3f2e4f521 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -2,6 +2,7 @@ import cgi import unittest import re +import json import six from six.moves import xmlrpc_client as xmlrpclib @@ -9,7 +10,7 @@ from six.moves.urllib.parse import urlparse, parse_qs, unquote if six.PY3: from urllib.parse import unquote_to_bytes -from scrapy.http import Request, FormRequest, XmlRpcRequest, Headers, HtmlResponse +from scrapy.http import Request, FormRequest, XmlRpcRequest, JSONRequest, Headers, HtmlResponse from scrapy.utils.python import to_bytes, to_native_str @@ -1147,5 +1148,57 @@ class XmlRpcRequestTest(RequestTest): self._test_request(params=(u'pas£',), encoding='latin1') +class JSONRequestTest(RequestTest): + request_class = JSONRequest + default_method = 'POST' + default_headers = {b'Content-Type': [b'application/json']} + + def test_body(self): + r1 = self.request_class(url="http://www.example.com/") + self.assertEqual(r1.body, '{}') + + r2 = self.request_class(url="http://www.example.com/", body=b"") + self.assertEqual(r2.body, '{}') + + data = { + 'name': 'value', + } + r3 = self.request_class(url="http://www.example.com/", data=data) + self.assertEqual(r3.body, json.dumps(data)) + + r4 = self.request_class(url="http://www.example.com/", body='body1', data=data) + self.assertEqual(r3.body, json.dumps(data)) + + def test_replace(self): + """Test Request.replace() method""" + r1 = self.request_class("http://www.example.com") + hdrs = Headers(r1.headers) + hdrs[b'key'] = b'value' + r2 = r1.replace(body="New body", headers=hdrs) + + # body will not be replaced + self.assertEqual(r1.body, r2.body) + self.assertEqual(r1.url, r2.url) + self.assertEqual((r1.headers, r2.headers), (self.default_headers, hdrs)) + + # Empty attributes (which may fail if not compared properly) + r3 = self.request_class("http://www.example.com", meta={'a': 1}, dont_filter=True) + r4 = r3.replace(url="http://www.example.com/2", meta={}, dont_filter=False) + self.assertEqual(r4.url, "http://www.example.com/2") + self.assertEqual(r4.meta, {}) + assert r4.dont_filter is False + + data1 = { + 'name': 'value1', + } + data2 = { + 'name': 'value2', + } + r5 = self.request_class("http://www.example.com", data=data1) + r6 = r5.replace(url="http://www.example.com/2", data=data2) + self.assertNotEqual(r5.body, r6.body) + self.assertEqual((r5.body, r6.body), (json.dumps(data1), json.dumps(data2))) + + if __name__ == "__main__": unittest.main() From 1b2b8b4bf0c73b4ad143f943584545702d66cbb7 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Tue, 27 Nov 2018 08:57:44 +0530 Subject: [PATCH 02/12] fix tests under py3 --- tests/test_http_request.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 3f2e4f521..a2021bd65 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -1155,19 +1155,19 @@ class JSONRequestTest(RequestTest): def test_body(self): r1 = self.request_class(url="http://www.example.com/") - self.assertEqual(r1.body, '{}') + self.assertEqual(r1.body, b'{}') r2 = self.request_class(url="http://www.example.com/", body=b"") - self.assertEqual(r2.body, '{}') + self.assertEqual(r2.body, b'{}') data = { 'name': 'value', } r3 = self.request_class(url="http://www.example.com/", data=data) - self.assertEqual(r3.body, json.dumps(data)) + self.assertEqual(r3.body, to_bytes(json.dumps(data))) r4 = self.request_class(url="http://www.example.com/", body='body1', data=data) - self.assertEqual(r3.body, json.dumps(data)) + self.assertEqual(r3.body, to_bytes(json.dumps(data))) def test_replace(self): """Test Request.replace() method""" @@ -1197,7 +1197,7 @@ class JSONRequestTest(RequestTest): r5 = self.request_class("http://www.example.com", data=data1) r6 = r5.replace(url="http://www.example.com/2", data=data2) self.assertNotEqual(r5.body, r6.body) - self.assertEqual((r5.body, r6.body), (json.dumps(data1), json.dumps(data2))) + self.assertEqual((r5.body, r6.body), (to_bytes(json.dumps(data1)), to_bytes(json.dumps(data2)))) if __name__ == "__main__": From cd619c1d4f3810c96af0ff5c5735c1856dfac95a Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Sat, 8 Dec 2018 22:10:45 +0530 Subject: [PATCH 03/12] removed overriden replace method --- scrapy/http/request/json_request.py | 20 +++++------ tests/test_http_request.py | 51 ++++++++--------------------- 2 files changed, 21 insertions(+), 50 deletions(-) diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 0fdd2ddf1..03a0ab061 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -12,17 +12,13 @@ from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): - if 'method' not in kwargs: - kwargs['method'] = 'POST' + data = kwargs.pop('data', None) + if data: + kwargs['body'] = json.dumps(data) + + if 'method' not in kwargs: + kwargs['method'] = 'POST' - data = kwargs.pop('data', {}) - kwargs['body'] = json.dumps(data) super(JSONRequest, self).__init__(*args, **kwargs) - self.headers.setdefault(b'Content-Type', b'application/json') - - def replace(self, *args, **kwargs): - """ Create a new Request with the same attributes except for those - given new values. """ - - kwargs.pop('body', None) - return super(JSONRequest, self).replace(*args, **kwargs) + self.headers.setdefault('Content-Type', 'application/json') + self.headers.setdefault('Accept', 'application/json, text/javascript, */*; q=0.01') diff --git a/tests/test_http_request.py b/tests/test_http_request.py index a2021bd65..793a583bc 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -1150,54 +1150,29 @@ class XmlRpcRequestTest(RequestTest): class JSONRequestTest(RequestTest): request_class = JSONRequest - default_method = 'POST' - default_headers = {b'Content-Type': [b'application/json']} + default_method = 'GET' + default_headers = {b'Content-Type': [b'application/json'], b'Accept': [b'application/json, text/javascript, */*; q=0.01']} - def test_body(self): + def test_data(self): r1 = self.request_class(url="http://www.example.com/") - self.assertEqual(r1.body, b'{}') + self.assertEqual(r1.body, b'') + self.assertEqual(r1.method, 'GET') - r2 = self.request_class(url="http://www.example.com/", body=b"") - self.assertEqual(r2.body, b'{}') + body = b'body' + r2 = self.request_class(url="http://www.example.com/", body=body) + self.assertEqual(r2.body, body) + self.assertEqual(r2.method, 'GET') data = { 'name': 'value', } r3 = self.request_class(url="http://www.example.com/", data=data) self.assertEqual(r3.body, to_bytes(json.dumps(data))) + self.assertEqual(r3.method, 'POST') - r4 = self.request_class(url="http://www.example.com/", body='body1', data=data) - self.assertEqual(r3.body, to_bytes(json.dumps(data))) - - def test_replace(self): - """Test Request.replace() method""" - r1 = self.request_class("http://www.example.com") - hdrs = Headers(r1.headers) - hdrs[b'key'] = b'value' - r2 = r1.replace(body="New body", headers=hdrs) - - # body will not be replaced - self.assertEqual(r1.body, r2.body) - self.assertEqual(r1.url, r2.url) - self.assertEqual((r1.headers, r2.headers), (self.default_headers, hdrs)) - - # Empty attributes (which may fail if not compared properly) - r3 = self.request_class("http://www.example.com", meta={'a': 1}, dont_filter=True) - r4 = r3.replace(url="http://www.example.com/2", meta={}, dont_filter=False) - self.assertEqual(r4.url, "http://www.example.com/2") - self.assertEqual(r4.meta, {}) - assert r4.dont_filter is False - - data1 = { - 'name': 'value1', - } - data2 = { - 'name': 'value2', - } - r5 = self.request_class("http://www.example.com", data=data1) - r6 = r5.replace(url="http://www.example.com/2", data=data2) - self.assertNotEqual(r5.body, r6.body) - self.assertEqual((r5.body, r6.body), (to_bytes(json.dumps(data1)), to_bytes(json.dumps(data2)))) + r4 = self.request_class(url="http://www.example.com/", body=body, data=data) + self.assertEqual(r4.body, to_bytes(json.dumps(data))) + self.assertEqual(r4.method, 'POST') if __name__ == "__main__": From c347acbff6545c428aa2c965cd03f03db6bae1bf Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Sun, 9 Dec 2018 11:27:09 +0530 Subject: [PATCH 04/12] warning if body and data are provided --- scrapy/http/request/json_request.py | 7 ++++++- tests/test_http_request.py | 25 ++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 03a0ab061..3b791eda3 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -6,14 +6,19 @@ See documentation in docs/topics/request-response.rst """ import json +import warnings from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): + body_passed = 'body' in kwargs data = kwargs.pop('data', None) - if data: + if body_passed and data: + warnings.warn('Both body and data passed. data will be ignored') + + elif not body_passed and data: kwargs['body'] = json.dumps(data) if 'method' not in kwargs: diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 793a583bc..e5a85e6fc 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -3,6 +3,7 @@ import cgi import unittest import re import json +import warnings import six from six.moves import xmlrpc_client as xmlrpclib @@ -1153,6 +1154,10 @@ class JSONRequestTest(RequestTest): default_method = 'GET' default_headers = {b'Content-Type': [b'application/json'], b'Accept': [b'application/json, text/javascript, */*; q=0.01']} + def setUp(self): + warnings.simplefilter("always") + super(JSONRequestTest, self).setUp() + def test_data(self): r1 = self.request_class(url="http://www.example.com/") self.assertEqual(r1.body, b'') @@ -1170,9 +1175,23 @@ class JSONRequestTest(RequestTest): self.assertEqual(r3.body, to_bytes(json.dumps(data))) self.assertEqual(r3.method, 'POST') - r4 = self.request_class(url="http://www.example.com/", body=body, data=data) - self.assertEqual(r4.body, to_bytes(json.dumps(data))) - self.assertEqual(r4.method, 'POST') + with warnings.catch_warnings(record=True) as _warnings: + r4 = self.request_class(url="http://www.example.com/", body=body, data=data) + self.assertEqual(r4.body, body) + self.assertEqual(r4.method, 'GET') + self.assertEqual(len(_warnings), 1) + self.assertIn('data will be ignored', str(_warnings[0].message)) + + with warnings.catch_warnings(record=True) as _warnings: + r5 = self.request_class(url="http://www.example.com/", body=b'', data=data) + self.assertEqual(r5.body, b'') + self.assertEqual(r5.method, 'GET') + self.assertEqual(len(_warnings), 1) + self.assertIn('data will be ignored', str(_warnings[0].message)) + + def tearDown(self): + warnings.resetwarnings() + super(JSONRequestTest, self).tearDown() if __name__ == "__main__": From 3c981bf204c739fa77e205b9747d2aff446c99d5 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Sun, 9 Dec 2018 12:56:12 +0530 Subject: [PATCH 05/12] add documentation --- docs/topics/request-response.rst | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index e29914dbf..d957915e7 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -508,6 +508,38 @@ method for this job. Here's an example spider which uses it:: # continue scraping with authenticated session... +JSONRequest +----------- + +The JSONRequest class extends the base :class:`Request` class with functionality for +dealing with JSON requests. + +.. class:: JSONRequest(url, [data, ...]) + + The :class:`JSONRequest` class adds a new argument to the constructor called data. The + remaining arguments are the same as for the :class:`Request` class and are + not documented here. + + Using the :class:`JSONRequest` will set the `Content-Type` header to `application/json` + and `Accept` header to `application/json, text/javascript, */*; q=0.01` + + :param data: is any JSON serializable object that needs to be JSON encoded and assigned to body. + if :attr:`Request.body` argument is provided this parameter will be ignored. + if :attr:`Request.body` argument is not provided and data argument is provided :attr:`Request.method` will be + set to POST automatically. + :type data: JSON serializable object + +JSONRequest usage example +------------------------- + +Sending a JSON POST request with a JSON payload:: + + data = { + 'name1': 'value1', + 'name2': 'value2', + } + yield JSONRequest(url='http://www.example.com/post/action', data=data) + Response objects ================ From ecda69130e97629b15d3b09b1e588cb6777ee94d Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Mon, 10 Dec 2018 22:34:49 +0530 Subject: [PATCH 06/12] allow to send empty data values and docs changes --- docs/topics/request-response.rst | 6 +++--- scrapy/http/request/json_request.py | 8 +++++--- tests/test_http_request.py | 27 +++++++++++++++++++++------ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index d957915e7..02b853fc0 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -520,13 +520,13 @@ dealing with JSON requests. remaining arguments are the same as for the :class:`Request` class and are not documented here. - Using the :class:`JSONRequest` will set the `Content-Type` header to `application/json` - and `Accept` header to `application/json, text/javascript, */*; q=0.01` + Using the :class:`JSONRequest` will set the ``Content-Type`` header to ``application/json`` + and ``Accept`` header to ``application/json, text/javascript, */*; q=0.01`` :param data: is any JSON serializable object that needs to be JSON encoded and assigned to body. if :attr:`Request.body` argument is provided this parameter will be ignored. if :attr:`Request.body` argument is not provided and data argument is provided :attr:`Request.method` will be - set to POST automatically. + set to ``'POST'`` automatically. :type data: JSON serializable object JSONRequest usage example diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 3b791eda3..593dfdcb0 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -13,12 +13,14 @@ from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): - body_passed = 'body' in kwargs + body_passed = kwargs.get('body', None) is not None data = kwargs.pop('data', None) - if body_passed and data: + data_passed = data is not None + + if body_passed and data_passed: warnings.warn('Both body and data passed. data will be ignored') - elif not body_passed and data: + elif not body_passed and data_passed: kwargs['body'] = json.dumps(data) if 'method' not in kwargs: diff --git a/tests/test_http_request.py b/tests/test_http_request.py index e5a85e6fc..5eb655c12 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -1175,20 +1175,35 @@ class JSONRequestTest(RequestTest): self.assertEqual(r3.body, to_bytes(json.dumps(data))) self.assertEqual(r3.method, 'POST') + r4 = self.request_class(url="http://www.example.com/", data=[]) + self.assertEqual(r4.body, to_bytes(json.dumps([]))) + self.assertEqual(r4.method, 'POST') + with warnings.catch_warnings(record=True) as _warnings: - r4 = self.request_class(url="http://www.example.com/", body=body, data=data) - self.assertEqual(r4.body, body) - self.assertEqual(r4.method, 'GET') + r5 = self.request_class(url="http://www.example.com/", body=body, data=data) + self.assertEqual(r5.body, body) + self.assertEqual(r5.method, 'GET') self.assertEqual(len(_warnings), 1) self.assertIn('data will be ignored', str(_warnings[0].message)) with warnings.catch_warnings(record=True) as _warnings: - r5 = self.request_class(url="http://www.example.com/", body=b'', data=data) - self.assertEqual(r5.body, b'') - self.assertEqual(r5.method, 'GET') + r6 = self.request_class(url="http://www.example.com/", body=b'', data=data) + self.assertEqual(r6.body, b'') + self.assertEqual(r6.method, 'GET') self.assertEqual(len(_warnings), 1) self.assertIn('data will be ignored', str(_warnings[0].message)) + with warnings.catch_warnings(record=True) as _warnings: + r7 = self.request_class(url="http://www.example.com/", body=None, data=data) + self.assertEqual(r7.body, to_bytes(json.dumps(data))) + self.assertEqual(r7.method, 'POST') + self.assertEqual(len(_warnings), 0) + + with warnings.catch_warnings(record=True) as _warnings: + r8 = self.request_class(url="http://www.example.com/", body=None, data=None) + self.assertEqual(r8.method, 'GET') + self.assertEqual(len(_warnings), 0) + def tearDown(self): warnings.resetwarnings() super(JSONRequestTest, self).tearDown() From 71ef321b68d2fd202de145d0c580387ee59cd2e2 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Wed, 12 Dec 2018 11:12:48 +0530 Subject: [PATCH 07/12] sort_keys while serializing to json --- scrapy/http/request/json_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 593dfdcb0..afc4356a3 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -21,7 +21,7 @@ class JSONRequest(Request): warnings.warn('Both body and data passed. data will be ignored') elif not body_passed and data_passed: - kwargs['body'] = json.dumps(data) + kwargs['body'] = json.dumps(data, sort_keys=True) if 'method' not in kwargs: kwargs['method'] = 'POST' From 8f1507a4a5de2ed55cb0fda198265845a047fedb Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Mon, 17 Dec 2018 23:14:06 +0530 Subject: [PATCH 08/12] dumps_kwargs --- docs/topics/request-response.rst | 10 ++- scrapy/http/request/json_request.py | 21 ++++- tests/test_http_request.py | 114 +++++++++++++++++++++++++++- 3 files changed, 138 insertions(+), 7 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 02b853fc0..4e6f00bb0 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -514,9 +514,9 @@ JSONRequest The JSONRequest class extends the base :class:`Request` class with functionality for dealing with JSON requests. -.. class:: JSONRequest(url, [data, ...]) +.. class:: JSONRequest(url, [... data]) - The :class:`JSONRequest` class adds a new argument to the constructor called data. The + The :class:`JSONRequest` class adds two new argument to the constructor. The remaining arguments are the same as for the :class:`Request` class and are not documented here. @@ -529,6 +529,12 @@ dealing with JSON requests. set to ``'POST'`` automatically. :type data: JSON serializable object + :param dumps_kwargs: Parameters that will be passed to underlying `json.dumps`_ method which is used to serialize data + into JSON format. + :type dumps_kwargs: dict + +.. _json.dumps: https://docs.python.org/3/library/json.html#json.dumps + JSONRequest usage example ------------------------- diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index afc4356a3..7499610b9 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -13,6 +13,7 @@ from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): + dumps_kwargs = kwargs.pop('dumps_kwargs', {}) body_passed = kwargs.get('body', None) is not None data = kwargs.pop('data', None) data_passed = data is not None @@ -21,7 +22,7 @@ class JSONRequest(Request): warnings.warn('Both body and data passed. data will be ignored') elif not body_passed and data_passed: - kwargs['body'] = json.dumps(data, sort_keys=True) + kwargs['body'] = self.dump(data, **dumps_kwargs) if 'method' not in kwargs: kwargs['method'] = 'POST' @@ -29,3 +30,21 @@ class JSONRequest(Request): super(JSONRequest, self).__init__(*args, **kwargs) self.headers.setdefault('Content-Type', 'application/json') self.headers.setdefault('Accept', 'application/json, text/javascript, */*; q=0.01') + self._dumps_kwargs = dumps_kwargs + + def replace(self, *args, **kwargs): + body_passed = kwargs.get('body', None) is not None + data = kwargs.pop('data', None) + data_passed = data is not None + + if body_passed and data_passed: + warnings.warn('Both body and data passed. data will be ignored') + + elif not body_passed and data_passed: + kwargs['body'] = self.dump(data, **self._dumps_kwargs) + + return super(JSONRequest, self).replace(*args, **kwargs) + + def dump(self, data, **kwargs): + """Convert to JSON """ + return json.dumps(data, sort_keys=True, **kwargs) diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 5eb655c12..6dcfa25da 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -14,6 +14,8 @@ if six.PY3: from scrapy.http import Request, FormRequest, XmlRpcRequest, JSONRequest, Headers, HtmlResponse from scrapy.utils.python import to_bytes, to_native_str +from tests import mock + class RequestTest(unittest.TestCase): @@ -1161,24 +1163,49 @@ class JSONRequestTest(RequestTest): def test_data(self): r1 = self.request_class(url="http://www.example.com/") self.assertEqual(r1.body, b'') - self.assertEqual(r1.method, 'GET') body = b'body' r2 = self.request_class(url="http://www.example.com/", body=body) self.assertEqual(r2.body, body) - self.assertEqual(r2.method, 'GET') data = { 'name': 'value', } r3 = self.request_class(url="http://www.example.com/", data=data) self.assertEqual(r3.body, to_bytes(json.dumps(data))) - self.assertEqual(r3.method, 'POST') + # empty data r4 = self.request_class(url="http://www.example.com/", data=[]) self.assertEqual(r4.body, to_bytes(json.dumps([]))) - self.assertEqual(r4.method, 'POST') + def test_data_method(self): + # data is not passed + r1 = self.request_class(url="http://www.example.com/") + self.assertEqual(r1.method, 'GET') + + body = b'body' + r2 = self.request_class(url="http://www.example.com/", body=body) + self.assertEqual(r2.method, 'GET') + + data = { + 'name': 'value', + } + r3 = self.request_class(url="http://www.example.com/", data=data) + self.assertEqual(r3.method, 'POST') + + # method passed explicitly + r4 = self.request_class(url="http://www.example.com/", data=data, method='GET') + self.assertEqual(r4.method, 'GET') + + r5 = self.request_class(url="http://www.example.com/", data=[]) + self.assertEqual(r5.method, 'POST') + + def test_body_data(self): + """ passing both body and data should result a warning """ + body = b'body' + data = { + 'name': 'value', + } with warnings.catch_warnings(record=True) as _warnings: r5 = self.request_class(url="http://www.example.com/", body=body, data=data) self.assertEqual(r5.body, body) @@ -1186,6 +1213,11 @@ class JSONRequestTest(RequestTest): self.assertEqual(len(_warnings), 1) self.assertIn('data will be ignored', str(_warnings[0].message)) + def test_empty_body_data(self): + """ passing any body value and data should result a warning """ + data = { + 'name': 'value', + } with warnings.catch_warnings(record=True) as _warnings: r6 = self.request_class(url="http://www.example.com/", body=b'', data=data) self.assertEqual(r6.body, b'') @@ -1193,17 +1225,91 @@ class JSONRequestTest(RequestTest): self.assertEqual(len(_warnings), 1) self.assertIn('data will be ignored', str(_warnings[0].message)) + def test_body_none_data(self): + data = { + 'name': 'value', + } with warnings.catch_warnings(record=True) as _warnings: r7 = self.request_class(url="http://www.example.com/", body=None, data=data) self.assertEqual(r7.body, to_bytes(json.dumps(data))) self.assertEqual(r7.method, 'POST') self.assertEqual(len(_warnings), 0) + def test_body_data_none(self): with warnings.catch_warnings(record=True) as _warnings: r8 = self.request_class(url="http://www.example.com/", body=None, data=None) self.assertEqual(r8.method, 'GET') self.assertEqual(len(_warnings), 0) + def test_dumps_sort_keys(self): + """ Test that sort_keys=True is passed to json.dumps by default """ + data = { + 'name': 'value', + } + with mock.patch('json.dumps', return_value=b'') as mock_dumps: + self.request_class(url="http://www.example.com/", data=data) + kwargs = mock_dumps.call_args[1] + self.assertEqual(kwargs['sort_keys'], True) + + def test_dumps_kwargs(self): + """ Test that dumps_kwargs are passed to json.dumps """ + data = { + 'name': 'value', + } + dumps_kwargs = { + 'ensure_ascii': True, + 'allow_nan': True, + } + with mock.patch('json.dumps', return_value=b'') as mock_dumps: + self.request_class(url="http://www.example.com/", data=data, dumps_kwargs=dumps_kwargs) + kwargs = mock_dumps.call_args[1] + self.assertEqual(kwargs['ensure_ascii'], True) + self.assertEqual(kwargs['allow_nan'], True) + + def test_replace_data(self): + data1 = { + 'name1': 'value1', + } + data2 = { + 'name2': 'value2', + } + r1 = self.request_class(url="http://www.example.com/", data=data1) + r2 = r1.replace(data=data2) + self.assertEqual(r2.body, to_bytes(json.dumps(data2))) + + def test_replace_sort_keys(self): + """ Test that replace provides sort_keys=True to json.dumps """ + data1 = { + 'name1': 'value1', + } + data2 = { + 'name2': 'value2', + } + r1 = self.request_class(url="http://www.example.com/", data=data1) + with mock.patch('json.dumps', return_value=b'') as mock_dumps: + r1.replace(data=data2) + kwargs = mock_dumps.call_args[1] + self.assertEqual(kwargs['sort_keys'], True) + + def test_replace_dumps_kwargs(self): + """ Test that dumps_kwargs are provided json.dumps when replace is called """ + data1 = { + 'name1': 'value1', + } + data2 = { + 'name2': 'value2', + } + dumps_kwargs = { + 'ensure_ascii': True, + 'allow_nan': True, + } + r1 = self.request_class(url="http://www.example.com/", data=data1, dumps_kwargs=dumps_kwargs) + with mock.patch('json.dumps', return_value=b'') as mock_dumps: + r1.replace(data=data2) + kwargs = mock_dumps.call_args[1] + self.assertEqual(kwargs['ensure_ascii'], True) + self.assertEqual(kwargs['allow_nan'], True) + def tearDown(self): warnings.resetwarnings() super(JSONRequestTest, self).tearDown() From 12ad06b7ac57dd022a4add16259ee8fd64d5ede2 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Mon, 17 Dec 2018 23:17:13 +0530 Subject: [PATCH 09/12] docs change --- docs/topics/request-response.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 4e6f00bb0..6758269b1 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -529,8 +529,8 @@ dealing with JSON requests. set to ``'POST'`` automatically. :type data: JSON serializable object - :param dumps_kwargs: Parameters that will be passed to underlying `json.dumps`_ method which is used to serialize data - into JSON format. + :param dumps_kwargs: Parameters that will be passed to underlying `json.dumps`_ method which is used to serialize + data into JSON format. :type dumps_kwargs: dict .. _json.dumps: https://docs.python.org/3/library/json.html#json.dumps From 24acc50d1894b6566e427f1dfea14e2aa647077e Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Tue, 18 Dec 2018 23:16:14 +0530 Subject: [PATCH 10/12] dumps_kwargs parameter in docs --- docs/topics/request-response.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 6758269b1..37b73edd1 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -514,7 +514,7 @@ JSONRequest The JSONRequest class extends the base :class:`Request` class with functionality for dealing with JSON requests. -.. class:: JSONRequest(url, [... data]) +.. class:: JSONRequest(url, [... data, dumps_kwargs]) The :class:`JSONRequest` class adds two new argument to the constructor. The remaining arguments are the same as for the :class:`Request` class and are From 3f914f6d8c369a18e1f856c01b7d1ad2a63f6e49 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Mon, 14 Jan 2019 23:03:14 +0530 Subject: [PATCH 11/12] made jsonrequest dump into private method --- scrapy/http/request/json_request.py | 15 +++++++++------ tests/test_http_request.py | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 7499610b9..1e2c6b0c6 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -5,6 +5,7 @@ This module implements the JSONRequest class which is a more convenient class See documentation in docs/topics/request-response.rst """ +import copy import json import warnings @@ -13,7 +14,10 @@ from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): - dumps_kwargs = kwargs.pop('dumps_kwargs', {}) + dumps_kwargs = copy.deepcopy(kwargs.pop('dumps_kwargs', {})) + dumps_kwargs['sort_keys'] = True + self._dumps_kwargs = dumps_kwargs + body_passed = kwargs.get('body', None) is not None data = kwargs.pop('data', None) data_passed = data is not None @@ -22,7 +26,7 @@ class JSONRequest(Request): warnings.warn('Both body and data passed. data will be ignored') elif not body_passed and data_passed: - kwargs['body'] = self.dump(data, **dumps_kwargs) + kwargs['body'] = self._dumps(data) if 'method' not in kwargs: kwargs['method'] = 'POST' @@ -30,7 +34,6 @@ class JSONRequest(Request): super(JSONRequest, self).__init__(*args, **kwargs) self.headers.setdefault('Content-Type', 'application/json') self.headers.setdefault('Accept', 'application/json, text/javascript, */*; q=0.01') - self._dumps_kwargs = dumps_kwargs def replace(self, *args, **kwargs): body_passed = kwargs.get('body', None) is not None @@ -41,10 +44,10 @@ class JSONRequest(Request): warnings.warn('Both body and data passed. data will be ignored') elif not body_passed and data_passed: - kwargs['body'] = self.dump(data, **self._dumps_kwargs) + kwargs['body'] = self._dumps(data) return super(JSONRequest, self).replace(*args, **kwargs) - def dump(self, data, **kwargs): + def _dumps(self, data): """Convert to JSON """ - return json.dumps(data, sort_keys=True, **kwargs) + return json.dumps(data, **self._dumps_kwargs) diff --git a/tests/test_http_request.py b/tests/test_http_request.py index 6dcfa25da..49f148016 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -1292,7 +1292,7 @@ class JSONRequestTest(RequestTest): self.assertEqual(kwargs['sort_keys'], True) def test_replace_dumps_kwargs(self): - """ Test that dumps_kwargs are provided json.dumps when replace is called """ + """ Test that dumps_kwargs are provided to json.dumps when replace is called """ data1 = { 'name1': 'value1', } From d9aa5391327dd34f8d840e7ce2bca1eb8583d932 Mon Sep 17 00:00:00 2001 From: kasun Herath Date: Fri, 25 Jan 2019 21:26:28 +0530 Subject: [PATCH 12/12] enabled sort keys only if not provided --- scrapy/http/request/json_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/request/json_request.py b/scrapy/http/request/json_request.py index 1e2c6b0c6..8f7a61a6d 100644 --- a/scrapy/http/request/json_request.py +++ b/scrapy/http/request/json_request.py @@ -15,7 +15,7 @@ from scrapy.http.request import Request class JSONRequest(Request): def __init__(self, *args, **kwargs): dumps_kwargs = copy.deepcopy(kwargs.pop('dumps_kwargs', {})) - dumps_kwargs['sort_keys'] = True + dumps_kwargs.setdefault('sort_keys', True) self._dumps_kwargs = dumps_kwargs body_passed = kwargs.get('body', None) is not None