From 15bb689dfe878b550f7f8940ce31508f40f6ac89 Mon Sep 17 00:00:00 2001 From: Kacper Kubkowski Date: Tue, 28 Aug 2018 23:49:53 +0200 Subject: [PATCH 1/4] Add from_json and from_data methods to JSON RPC Responses + jsonrpc/exceptions.py: add JSONRPCInvalidResponseException class; + jsonrpc/jsonrpc.py: add JSONRPCResponse class; * jsonrpc/jsonrpc1.py: update JSONRPC10Response.error setter, add from_json and from_data class methods; * jsonrpc/jsonrpc2.py: add JSONRPC20Response.result and error deleters, update error setter, add from_json and from_data class methods; + jsonrpc/tests/test_jsonrpc1.py, jsonrpc/tests/test_jsonrpc2.py: add functions testing new features; + added self to AUTHORS. Signed-off-by: Kacper Kubkowski --- AUTHORS | 1 + jsonrpc/exceptions.py | 7 +++ jsonrpc/jsonrpc.py | 17 +++++++ jsonrpc/jsonrpc1.py | 34 +++++++++++-- jsonrpc/jsonrpc2.py | 67 +++++++++++++++++++++++-- jsonrpc/tests/test_jsonrpc1.py | 84 ++++++++++++++++++++++++++++++- jsonrpc/tests/test_jsonrpc2.py | 92 +++++++++++++++++++++++++++++++++- 7 files changed, 294 insertions(+), 8 deletions(-) diff --git a/AUTHORS b/AUTHORS index 23e3ba4..064e9d4 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,3 +24,4 @@ Laurent Mazuel (@lmazuel) Igor Melnyk (@liminspace) Ghislain Antony Vaillant (@ghisvail) Chris Jerdonek (@cjerdonek) +Kacper Kubkowski (@kacper-ka) diff --git a/jsonrpc/exceptions.py b/jsonrpc/exceptions.py index 175ba92..1a7bffa 100644 --- a/jsonrpc/exceptions.py +++ b/jsonrpc/exceptions.py @@ -172,6 +172,13 @@ class JSONRPCInvalidRequestException(JSONRPCException): pass +class JSONRPCInvalidResponseException(JSONRPCException): + + """ Response is not valid.""" + + pass + + class JSONRPCDispatchException(JSONRPCException): """ JSON-RPC Dispatch Exception. diff --git a/jsonrpc/jsonrpc.py b/jsonrpc/jsonrpc.py index f02126e..92c11a9 100644 --- a/jsonrpc/jsonrpc.py +++ b/jsonrpc/jsonrpc.py @@ -26,3 +26,20 @@ def from_data(cls, data): return JSONRPC10Request.from_data(data) else: return JSONRPC20Request.from_data(data) + + +class JSONRPCResponse(JSONSerializable): + + """ JSONRPC Response.""" + + @classmethod + def from_json(cls, json_str): + data = cls.deserialize(json_str) + return cls.from_data(data) + + @classmethod + def from_data(cls, data): + if isinstance(data, dict) and 'jsonrpc' not in data: + return JSONRPC10Response.from_data(data) + else: + return JSONRPC20Response.from_data(data) diff --git a/jsonrpc/jsonrpc1.py b/jsonrpc/jsonrpc1.py index 3fcbe9b..c883d24 100644 --- a/jsonrpc/jsonrpc1.py +++ b/jsonrpc/jsonrpc1.py @@ -1,7 +1,7 @@ from . import six from .base import JSONRPCBaseRequest, JSONRPCBaseResponse -from .exceptions import JSONRPCInvalidRequestException, JSONRPCError +from .exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException, JSONRPCError class JSONRPC10Request(JSONRPCBaseRequest): @@ -105,6 +105,8 @@ def from_data(cls, data): class JSONRPC10Response(JSONRPCBaseResponse): JSONRPC_VERSION = "1.0" + REQUIRED_FIELDS = {"id", "result", "error"} + POSSIBLE_FIELDS = {"id", "result", "error"} @property def data(self): @@ -134,11 +136,14 @@ def error(self): @error.setter def error(self, value): - self._data.pop('value', None) if value: - self._data["error"] = value + if self.result is not None: + raise ValueError("Either result or error should be used") # Test error JSONRPCError(**value) + self._data["error"] = value + else: + self._data["error"] = None @property def _id(self): @@ -149,3 +154,26 @@ def _id(self, value): if value is None: raise ValueError("id could not be null for JSON-RPC1.0 Response") self._data["id"] = value + + @classmethod + def from_json(cls, json_str): + data = cls.deserialize(json_str) + return cls.from_data(data) + + @classmethod + def from_data(cls, data): + if not isinstance(data, dict): + raise ValueError("data should be dict") + + if cls.REQUIRED_FIELDS <= set(data.keys()) <= cls.POSSIBLE_FIELDS: + try: + return cls( + _id=data["id"], result=data["result"], error=data["error"] + ) + except ValueError as e: + raise JSONRPCInvalidResponseException(str(e)) + else: + extra = set(data.keys()) - cls.POSSIBLE_FIELDS + missed = cls.REQUIRED_FIELDS - set(data.keys()) + msg = "Invalid response. Extra fields: {0}, Missed fields: {1}" + raise JSONRPCInvalidResponseException(msg.format(extra, missed)) diff --git a/jsonrpc/jsonrpc2.py b/jsonrpc/jsonrpc2.py index 66ca451..cfa5be0 100644 --- a/jsonrpc/jsonrpc2.py +++ b/jsonrpc/jsonrpc2.py @@ -1,7 +1,7 @@ from . import six import json -from .exceptions import JSONRPCError, JSONRPCInvalidRequestException +from .exceptions import JSONRPCError, JSONRPCInvalidRequestException, JSONRPCInvalidResponseException from .base import JSONRPCBaseRequest, JSONRPCBaseResponse @@ -199,6 +199,8 @@ class JSONRPC20Response(JSONRPCBaseResponse): """ JSONRPC_VERSION = "2.0" + REQUIRED_FIELDS = {"jsonrpc", "id"} + POSSIBLE_FIELDS = {"jsonrpc", "id", "result", "error"} @property def data(self): @@ -222,17 +224,35 @@ def result(self, value): raise ValueError("Either result or error should be used") self._data["result"] = value + @result.deleter + def result(self): + try: + del self._data["result"] + except KeyError: + pass + @property def error(self): return self._data.get("error") @error.setter def error(self, value): - self._data.pop('value', None) if value: - self._data["error"] = value + if self.result is not None: + raise ValueError("Either result or error should be used") + del self.result # Test error JSONRPCError(**value) + self._data["error"] = value + else: + del self.error + + @error.deleter + def error(self): + try: + del self._data["error"] + except KeyError: + pass @property def _id(self): @@ -246,6 +266,47 @@ def _id(self, value): self._data["id"] = value + @classmethod + def from_json(cls, json_str): + data = cls.deserialize(json_str) + return cls.from_data(data) + + @classmethod + def from_data(cls, data): + is_batch = isinstance(data, list) + data = data if is_batch else [data] + + if not data: + raise JSONRPCInvalidResponseException("[] value is not accepted") + + if not all(isinstance(d, dict) for d in data): + raise JSONRPCInvalidResponseException( + "Each response should be an object (dict)") + + result = [] + for d in data: + if not cls.REQUIRED_FIELDS <= set(d.keys()) <= cls.POSSIBLE_FIELDS: + extra = set(d.keys()) - cls.POSSIBLE_FIELDS + missed = cls.REQUIRED_FIELDS - set(d.keys()) + msg = "Invalid response. Extra fields: {0}, Missed fields: {1}" + raise JSONRPCInvalidResponseException(msg.format(extra, missed)) + s = {'result', 'error'} & set(d.keys()) + if len(s) != 1: + if len(s) == 2: + msg = "Invalid response. Either result or error may be present, not both." + else: + msg = "Invalid response. Neither result nor error present." + raise JSONRPCInvalidResponseException(msg) + + try: + result.append(JSONRPC20Response( + _id=d.get("id"), result=d.get("result"), error=d.get("error") + )) + except ValueError as e: + raise JSONRPCInvalidResponseException(str(e)) + + return JSONRPC20BatchResponse(*result) if is_batch else result[0] + class JSONRPC20BatchResponse(object): diff --git a/jsonrpc/tests/test_jsonrpc1.py b/jsonrpc/tests/test_jsonrpc1.py index c66f045..3984631 100644 --- a/jsonrpc/tests/test_jsonrpc1.py +++ b/jsonrpc/tests/test_jsonrpc1.py @@ -1,7 +1,7 @@ import json import sys -from ..exceptions import JSONRPCInvalidRequestException +from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException, JSONRPCParseError from ..jsonrpc1 import ( JSONRPC10Request, JSONRPC10Response, @@ -427,3 +427,85 @@ def test_data_setter(self): def test_validation_id(self): response = JSONRPC10Response(**self.response_success_params) self.assertEqual(response._id, self.response_success_params["_id"]) + + def test_from_json_invalid_response_result(self): + str_json = json.dumps({ + "error": {"code": -32700, "message": "Parse error"}, + "id": 0, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC10Response.from_json(str_json) + + def test_from_json_invalid_response_error(self): + str_json = json.dumps({ + "result": "", + "id": 0, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC10Response.from_json(str_json) + + def test_from_json_invalid_response_id(self): + str_json = json.dumps({ + "result": "", + "error": None, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC10Response.from_json(str_json) + + def test_from_json_invalid_response_both_result_and_error(self): + str_json = json.dumps({ + "result": "", + "error": {"code": -32700, "message": "Parse error"}, + "id": 0, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC10Response.from_json(str_json) + + def test_from_json_invalid_response_extra_data(self): + str_json = json.dumps({ + "result": "", + "error": None, + "id": 0, + "is_notification": True, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC10Response.from_json(str_json) + + def test_from_json_response_result(self): + str_json = json.dumps({ + "result": "abc", + "error": None, + "id": 0, + }) + + response = JSONRPC10Response.from_json(str_json) + self.assertTrue(isinstance(response, JSONRPC10Response)) + self.assertEqual(response.result, "abc") + self.assertIsNone(response.error) + self.assertEqual(response._id, 0) + + def test_from_json_response_error(self): + err = JSONRPCParseError() + str_json = json.dumps({ + "result": None, + "error": err._data, + "id": 0, + }) + + response = JSONRPC10Response.from_json(str_json) + self.assertTrue(isinstance(response, JSONRPC10Response)) + self.assertIsNone(response.result) + self.assertEqual(response.error, err._data) + self.assertEqual(response._id, 0) + + def test_from_json_string_not_dict(self): + with self.assertRaises(ValueError): + JSONRPC10Response.from_json("[]") + + with self.assertRaises(ValueError): + JSONRPC10Response.from_json("0") \ No newline at end of file diff --git a/jsonrpc/tests/test_jsonrpc2.py b/jsonrpc/tests/test_jsonrpc2.py index 32c1639..5a27bee 100644 --- a/jsonrpc/tests/test_jsonrpc2.py +++ b/jsonrpc/tests/test_jsonrpc2.py @@ -1,7 +1,7 @@ import json import sys -from ..exceptions import JSONRPCInvalidRequestException +from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException, JSONRPCParseError from ..jsonrpc2 import ( JSONRPC20Request, JSONRPC20BatchRequest, @@ -691,6 +691,96 @@ def test_data_setter(self): with self.assertRaises(ValueError): response.data = None + def test_from_json_invalid_response_jsonrpc(self): + str_json = json.dumps({ + "result": None, + "id": 0, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC20Response.from_json(str_json) + + def test_from_json_invalid_response_id(self): + str_json = json.dumps({ + "result": None, + "jsonrpc": "2.0", + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC20Response.from_json(str_json) + + def test_from_json_invalid_response_no_result_error(self): + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC20Response.from_json(str_json) + + def test_from_json_invalid_response_result_and_error(self): + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + "result": None, + "error": {"code": 1, "message": ""} + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC20Response.from_json(str_json) + + def test_from_json_invalid_response_extra_data(self): + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + "result": None, + "error": {"code": 1, "message": ""}, + "extra-data": "" + }) + + with self.assertRaises(JSONRPCInvalidResponseException): + JSONRPC20Response.from_json(str_json) + + def test_from_json_response_result_null(self): + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + "result": None, + }) + + response = JSONRPC20Response.from_json(str_json) + self.assertIsInstance(response, JSONRPC20Response) + self.assertIsNone(response.result) + self.assertIsNone(response.error) + self.assertEqual(response._id, 0) + + def test_from_json_response_result(self): + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + "result": [1, 2, 3], + }) + + response = JSONRPC20Response.from_json(str_json) + self.assertIsInstance(response, JSONRPC20Response) + self.assertEqual(response.result, [1, 2, 3]) + self.assertIsNone(response.error) + self.assertEqual(response._id, 0) + + def test_from_json_response_error(self): + err = JSONRPCParseError() + str_json = json.dumps({ + "jsonrpc": "2.0", + "id": 0, + "error": err._data, + }) + + response = JSONRPC20Response.from_json(str_json) + self.assertIsInstance(response, JSONRPC20Response) + self.assertIsNone(response.result) + self.assertEqual(response.error, err._data) + self.assertEqual(response._id, 0) + class TestJSONRPC20BatchResponse(unittest.TestCase): From 6311cccb296fdce63c14929349567afd42a697ba Mon Sep 17 00:00:00 2001 From: Kacper Kubkowski Date: Wed, 29 Aug 2018 09:44:39 +0200 Subject: [PATCH 2/4] Update tests to use simple JSON RPC error objects --- jsonrpc/tests/test_jsonrpc1.py | 8 ++++---- jsonrpc/tests/test_jsonrpc2.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jsonrpc/tests/test_jsonrpc1.py b/jsonrpc/tests/test_jsonrpc1.py index 3984631..b7d166f 100644 --- a/jsonrpc/tests/test_jsonrpc1.py +++ b/jsonrpc/tests/test_jsonrpc1.py @@ -1,7 +1,7 @@ import json import sys -from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException, JSONRPCParseError +from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException from ..jsonrpc1 import ( JSONRPC10Request, JSONRPC10Response, @@ -490,17 +490,17 @@ def test_from_json_response_result(self): self.assertEqual(response._id, 0) def test_from_json_response_error(self): - err = JSONRPCParseError() + error = {'code': 1, 'message': ''} str_json = json.dumps({ "result": None, - "error": err._data, + "error": error, "id": 0, }) response = JSONRPC10Response.from_json(str_json) self.assertTrue(isinstance(response, JSONRPC10Response)) self.assertIsNone(response.result) - self.assertEqual(response.error, err._data) + self.assertEqual(response.error, error) self.assertEqual(response._id, 0) def test_from_json_string_not_dict(self): diff --git a/jsonrpc/tests/test_jsonrpc2.py b/jsonrpc/tests/test_jsonrpc2.py index 5a27bee..8c9e946 100644 --- a/jsonrpc/tests/test_jsonrpc2.py +++ b/jsonrpc/tests/test_jsonrpc2.py @@ -1,7 +1,7 @@ import json import sys -from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException, JSONRPCParseError +from ..exceptions import JSONRPCInvalidRequestException, JSONRPCInvalidResponseException from ..jsonrpc2 import ( JSONRPC20Request, JSONRPC20BatchRequest, @@ -768,17 +768,17 @@ def test_from_json_response_result(self): self.assertEqual(response._id, 0) def test_from_json_response_error(self): - err = JSONRPCParseError() + error = {'code': 1, 'message': ''} str_json = json.dumps({ "jsonrpc": "2.0", "id": 0, - "error": err._data, + "error": error, }) response = JSONRPC20Response.from_json(str_json) self.assertIsInstance(response, JSONRPC20Response) self.assertIsNone(response.result) - self.assertEqual(response.error, err._data) + self.assertEqual(response.error, error) self.assertEqual(response._id, 0) From 6436ea6f5926b289ce072884a0ad09a4a07644fa Mon Sep 17 00:00:00 2001 From: Kacper Kubkowski Date: Wed, 29 Aug 2018 10:35:17 +0200 Subject: [PATCH 3/4] Fix set syntax causing Python 2.6 tests fail --- jsonrpc/jsonrpc1.py | 4 ++-- jsonrpc/jsonrpc2.py | 4 ++-- jsonrpc/tests/test_jsonrpc1.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/jsonrpc/jsonrpc1.py b/jsonrpc/jsonrpc1.py index c883d24..1a96ce6 100644 --- a/jsonrpc/jsonrpc1.py +++ b/jsonrpc/jsonrpc1.py @@ -105,8 +105,8 @@ def from_data(cls, data): class JSONRPC10Response(JSONRPCBaseResponse): JSONRPC_VERSION = "1.0" - REQUIRED_FIELDS = {"id", "result", "error"} - POSSIBLE_FIELDS = {"id", "result", "error"} + REQUIRED_FIELDS = set(["id", "result", "error"]) + POSSIBLE_FIELDS = set(["id", "result", "error"]) @property def data(self): diff --git a/jsonrpc/jsonrpc2.py b/jsonrpc/jsonrpc2.py index cfa5be0..9509dfd 100644 --- a/jsonrpc/jsonrpc2.py +++ b/jsonrpc/jsonrpc2.py @@ -199,8 +199,8 @@ class JSONRPC20Response(JSONRPCBaseResponse): """ JSONRPC_VERSION = "2.0" - REQUIRED_FIELDS = {"jsonrpc", "id"} - POSSIBLE_FIELDS = {"jsonrpc", "id", "result", "error"} + REQUIRED_FIELDS = set(["jsonrpc", "id"]) + POSSIBLE_FIELDS = set(["jsonrpc", "id", "result", "error"]) @property def data(self): diff --git a/jsonrpc/tests/test_jsonrpc1.py b/jsonrpc/tests/test_jsonrpc1.py index b7d166f..c0cfa81 100644 --- a/jsonrpc/tests/test_jsonrpc1.py +++ b/jsonrpc/tests/test_jsonrpc1.py @@ -508,4 +508,4 @@ def test_from_json_string_not_dict(self): JSONRPC10Response.from_json("[]") with self.assertRaises(ValueError): - JSONRPC10Response.from_json("0") \ No newline at end of file + JSONRPC10Response.from_json("0") From 28115f722010a1af519fa4f111486b28544a928d Mon Sep 17 00:00:00 2001 From: Kacper Kubkowski Date: Wed, 29 Aug 2018 10:37:58 +0200 Subject: [PATCH 4/4] Fix usage of sets causing syntax error in Python 2.6 --- jsonrpc/jsonrpc2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonrpc/jsonrpc2.py b/jsonrpc/jsonrpc2.py index 9509dfd..7b34f3f 100644 --- a/jsonrpc/jsonrpc2.py +++ b/jsonrpc/jsonrpc2.py @@ -290,7 +290,7 @@ def from_data(cls, data): missed = cls.REQUIRED_FIELDS - set(d.keys()) msg = "Invalid response. Extra fields: {0}, Missed fields: {1}" raise JSONRPCInvalidResponseException(msg.format(extra, missed)) - s = {'result', 'error'} & set(d.keys()) + s = set(['result', 'error']) & set(d.keys()) if len(s) != 1: if len(s) == 2: msg = "Invalid response. Either result or error may be present, not both."