From a5cc8b6f6fa47e74f940d5b9bb02ccd78f51ef0e Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Tue, 2 Nov 2021 10:17:53 +1300 Subject: [PATCH 1/6] fix: convert tuples into lists --- bindings/python/CHANGELOG.md | 1 + bindings/python/src/ser.rs | 33 ++++++++++++++++++++- bindings/python/src/types.rs | 4 ++- bindings/python/tests-py/test_jsonschema.py | 6 ++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/bindings/python/CHANGELOG.md b/bindings/python/CHANGELOG.md index 568d5331..cb284051 100644 --- a/bindings/python/CHANGELOG.md +++ b/bindings/python/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixed - Set `jsonschema_rs.JSONSchema.__module__` to `jsonschema_rs`. +- Convert tuples into lists for validation to fix `ValueError: Unsupported type: 'tuple'` ### Performance diff --git a/bindings/python/src/ser.rs b/bindings/python/src/ser.rs index ba85e4b4..e57e092a 100644 --- a/bindings/python/src/ser.rs +++ b/bindings/python/src/ser.rs @@ -2,7 +2,7 @@ use pyo3::{ exceptions, ffi::{ PyDictObject, PyFloat_AS_DOUBLE, PyList_GET_ITEM, PyList_GET_SIZE, PyLong_AsLongLong, - Py_TYPE, + Py_TYPE, PyTuple_GET_ITEM, PyTuple_GET_SIZE }, prelude::*, types::PyAny, @@ -27,6 +27,7 @@ pub enum ObjectType { Float, List, Dict, + Tuple, Unknown(String), } @@ -81,6 +82,8 @@ pub fn get_object_type(object_type: *mut pyo3::ffi::PyTypeObject) -> ObjectType ObjectType::None } else if object_type == unsafe { types::LIST_TYPE } { ObjectType::List + } else if object_type == unsafe { types::TUPLE_TYPE } { + ObjectType::Tuple } else if object_type == unsafe { types::DICT_TYPE } { ObjectType::Dict } else { @@ -174,6 +177,34 @@ impl Serialize for SerializePyObject { sequence.end() } } + ObjectType::Tuple => { + if self.recursion_depth == RECURSION_LIMIT { + return Err(ser::Error::custom("Recursion limit reached")); + } + let length = unsafe { PyTuple_GET_SIZE(self.object) } as usize; + if length == 0 { + serializer.serialize_seq(Some(0))?.end() + } else { + let mut type_ptr = std::ptr::null_mut(); + let mut ob_type = ObjectType::Str; + let mut sequence = serializer.serialize_seq(Some(length))?; + for i in 0..length { + let elem = unsafe { PyTuple_GET_ITEM(self.object, i as isize) }; + let current_ob_type = unsafe { Py_TYPE(elem) }; + if current_ob_type != type_ptr { + type_ptr = current_ob_type; + ob_type = get_object_type(current_ob_type); + } + #[allow(clippy::integer_arithmetic)] + sequence.serialize_element(&SerializePyObject::with_obtype( + elem, + ob_type.clone(), + self.recursion_depth + 1, + ))?; + } + sequence.end() + } + } ObjectType::Unknown(ref type_name) => Err(ser::Error::custom(format!( "Unsupported type: '{}'", type_name diff --git a/bindings/python/src/types.rs b/bindings/python/src/types.rs index 6103e8e8..6725329f 100644 --- a/bindings/python/src/types.rs +++ b/bindings/python/src/types.rs @@ -1,6 +1,6 @@ use pyo3::ffi::{ PyDict_New, PyFloat_FromDouble, PyList_New, PyLong_FromLongLong, PyTypeObject, PyUnicode_New, - Py_None, Py_TYPE, Py_True, + Py_None, Py_TYPE, Py_True, PyTuple_New }; use std::sync::Once; @@ -13,6 +13,7 @@ pub static mut NONE_TYPE: *mut PyTypeObject = 0 as *mut PyTypeObject; pub static mut FLOAT_TYPE: *mut PyTypeObject = 0 as *mut PyTypeObject; pub static mut LIST_TYPE: *mut PyTypeObject = 0 as *mut PyTypeObject; pub static mut DICT_TYPE: *mut PyTypeObject = 0 as *mut PyTypeObject; +pub static mut TUPLE_TYPE: *mut PyTypeObject = 0 as *mut PyTypeObject; static INIT: Once = Once::new(); @@ -24,6 +25,7 @@ pub fn init() { TRUE = Py_True(); STR_TYPE = Py_TYPE(PyUnicode_New(0, 255)); DICT_TYPE = Py_TYPE(PyDict_New()); + TUPLE_TYPE = Py_TYPE(PyTuple_New(1)); LIST_TYPE = Py_TYPE(PyList_New(0_isize)); NONE_TYPE = Py_TYPE(Py_None()); BOOL_TYPE = Py_TYPE(TRUE); diff --git a/bindings/python/tests-py/test_jsonschema.py b/bindings/python/tests-py/test_jsonschema.py index 755c8c31..dff2d07d 100644 --- a/bindings/python/tests-py/test_jsonschema.py +++ b/bindings/python/tests-py/test_jsonschema.py @@ -65,6 +65,12 @@ def test_from_str_error(): JSONSchema.from_str(42) +def test_tuple(): + schema = {"properties": {"foo": {"type": "array"}}} + instance = {"foo": (1, 2, 3)} + assert is_valid(instance, schema) == True + + def test_recursive_dict(): instance = {} instance["foo"] = instance From a71d0373ae50135e150d3344f7df41fd47967164 Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Tue, 2 Nov 2021 11:40:10 +1300 Subject: [PATCH 2/6] refactor: avoid serialization logic duplication --- bindings/python/src/ser.rs | 47 ++++++++++++------------------------ bindings/python/src/types.rs | 4 +-- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/bindings/python/src/ser.rs b/bindings/python/src/ser.rs index e57e092a..25264279 100644 --- a/bindings/python/src/ser.rs +++ b/bindings/python/src/ser.rs @@ -2,7 +2,7 @@ use pyo3::{ exceptions, ffi::{ PyDictObject, PyFloat_AS_DOUBLE, PyList_GET_ITEM, PyList_GET_SIZE, PyLong_AsLongLong, - Py_TYPE, PyTuple_GET_ITEM, PyTuple_GET_SIZE + PyTuple_GET_ITEM, PyTuple_GET_SIZE, Py_TYPE, }, prelude::*, types::PyAny, @@ -149,39 +149,16 @@ impl Serialize for SerializePyObject { map.end() } } - ObjectType::List => { + ObjectType::Tuple | ObjectType::List => { if self.recursion_depth == RECURSION_LIMIT { return Err(ser::Error::custom("Recursion limit reached")); } - let length = unsafe { PyList_GET_SIZE(self.object) } as usize; - if length == 0 { - serializer.serialize_seq(Some(0))?.end() - } else { - let mut type_ptr = std::ptr::null_mut(); - let mut ob_type = ObjectType::Str; - let mut sequence = serializer.serialize_seq(Some(length))?; - for i in 0..length { - let elem = unsafe { PyList_GET_ITEM(self.object, i as isize) }; - let current_ob_type = unsafe { Py_TYPE(elem) }; - if current_ob_type != type_ptr { - type_ptr = current_ob_type; - ob_type = get_object_type(current_ob_type); - } - #[allow(clippy::integer_arithmetic)] - sequence.serialize_element(&SerializePyObject::with_obtype( - elem, - ob_type.clone(), - self.recursion_depth + 1, - ))?; - } - sequence.end() - } - } - ObjectType::Tuple => { - if self.recursion_depth == RECURSION_LIMIT { - return Err(ser::Error::custom("Recursion limit reached")); - } - let length = unsafe { PyTuple_GET_SIZE(self.object) } as usize; + + let length = match self.object_type { + ObjectType::Tuple => unsafe { PyTuple_GET_SIZE(self.object) as usize }, + ObjectType::List => unsafe { PyList_GET_SIZE(self.object) as usize }, + _ => return Err(ser::Error::custom("Object is not a list or tuple")), + }; if length == 0 { serializer.serialize_seq(Some(0))?.end() } else { @@ -189,7 +166,13 @@ impl Serialize for SerializePyObject { let mut ob_type = ObjectType::Str; let mut sequence = serializer.serialize_seq(Some(length))?; for i in 0..length { - let elem = unsafe { PyTuple_GET_ITEM(self.object, i as isize) }; + let elem = match self.object_type { + ObjectType::Tuple => unsafe { + PyTuple_GET_ITEM(self.object, i as isize) + }, + ObjectType::List => unsafe { PyList_GET_ITEM(self.object, i as isize) }, + _ => return Err(ser::Error::custom("Object is not a list or tuple")), + }; let current_ob_type = unsafe { Py_TYPE(elem) }; if current_ob_type != type_ptr { type_ptr = current_ob_type; diff --git a/bindings/python/src/types.rs b/bindings/python/src/types.rs index 6725329f..c5f94c8c 100644 --- a/bindings/python/src/types.rs +++ b/bindings/python/src/types.rs @@ -1,6 +1,6 @@ use pyo3::ffi::{ - PyDict_New, PyFloat_FromDouble, PyList_New, PyLong_FromLongLong, PyTypeObject, PyUnicode_New, - Py_None, Py_TYPE, Py_True, PyTuple_New + PyDict_New, PyFloat_FromDouble, PyList_New, PyLong_FromLongLong, PyTuple_New, PyTypeObject, + PyUnicode_New, Py_None, Py_TYPE, Py_True, }; use std::sync::Once; From d04280120cc0a47f74f3b848f58a79c6ea2131bc Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Thu, 4 Nov 2021 08:52:56 +1300 Subject: [PATCH 3/6] refactor: convert tuple size to 0 --- bindings/python/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/python/src/types.rs b/bindings/python/src/types.rs index c5f94c8c..a79a9625 100644 --- a/bindings/python/src/types.rs +++ b/bindings/python/src/types.rs @@ -25,7 +25,7 @@ pub fn init() { TRUE = Py_True(); STR_TYPE = Py_TYPE(PyUnicode_New(0, 255)); DICT_TYPE = Py_TYPE(PyDict_New()); - TUPLE_TYPE = Py_TYPE(PyTuple_New(1)); + TUPLE_TYPE = Py_TYPE(PyTuple_New(0_isize)); LIST_TYPE = Py_TYPE(PyList_New(0_isize)); NONE_TYPE = Py_TYPE(Py_None()); BOOL_TYPE = Py_TYPE(TRUE); From 8df27d170c0be1dfa44768d2b7f45d57b1b6ed7c Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Thu, 4 Nov 2021 09:11:21 +1300 Subject: [PATCH 4/6] refactor: expand tests around arrays and tuples --- bindings/python/tests-py/test_jsonschema.py | 24 +++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/bindings/python/tests-py/test_jsonschema.py b/bindings/python/tests-py/test_jsonschema.py index dff2d07d..e88de863 100644 --- a/bindings/python/tests-py/test_jsonschema.py +++ b/bindings/python/tests-py/test_jsonschema.py @@ -65,10 +65,26 @@ def test_from_str_error(): JSONSchema.from_str(42) -def test_tuple(): - schema = {"properties": {"foo": {"type": "array"}}} - instance = {"foo": (1, 2, 3)} - assert is_valid(instance, schema) == True +@pytest.mark.parametrize( + "val", + ( + ("A", "B", "C"), + ["A", "B", "C"], + ), +) +def test_array_tuple(val): + schema = {"type": "array", "items": {"type": "string"}} + validate(schema, val) + + +@pytest.mark.parametrize( + "val", + ((1, 2, 3), [1, 2, 3], {"foo": 1}), +) +def test_array_tuple_invalid(val): + schema = {"type": "array", "items": {"type": "string"}} + with pytest.raises(ValueError): + validate(schema, val) def test_recursive_dict(): From c757370143241e8f6d8135eb9100239489f1d7e3 Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Thu, 4 Nov 2021 09:26:06 +1300 Subject: [PATCH 5/6] test: validate that named tuples error --- bindings/python/tests-py/test_jsonschema.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bindings/python/tests-py/test_jsonschema.py b/bindings/python/tests-py/test_jsonschema.py index e88de863..a363ad9d 100644 --- a/bindings/python/tests-py/test_jsonschema.py +++ b/bindings/python/tests-py/test_jsonschema.py @@ -1,3 +1,4 @@ +from collections import namedtuple from contextlib import suppress from functools import partial @@ -87,6 +88,14 @@ def test_array_tuple_invalid(val): validate(schema, val) +def test_named_tuple(): + Person = namedtuple("Person", "first_name last_name") + person_a = Person("Joe", "Smith") + schema = {"type": "array", "items": {"type": "string"}} + with pytest.raises(ValueError): + validate(schema, person_a) + + def test_recursive_dict(): instance = {} instance["foo"] = instance From 0ada8f37dfb0bd7d7ff9bc11ba390fffa4c1dfe7 Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Fri, 5 Nov 2021 00:06:23 +1300 Subject: [PATCH 6/6] refactor: split list and tuple serialization --- bindings/python/src/ser.rs | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/bindings/python/src/ser.rs b/bindings/python/src/ser.rs index 25264279..afb3c806 100644 --- a/bindings/python/src/ser.rs +++ b/bindings/python/src/ser.rs @@ -149,16 +149,11 @@ impl Serialize for SerializePyObject { map.end() } } - ObjectType::Tuple | ObjectType::List => { + ObjectType::List => { if self.recursion_depth == RECURSION_LIMIT { return Err(ser::Error::custom("Recursion limit reached")); } - - let length = match self.object_type { - ObjectType::Tuple => unsafe { PyTuple_GET_SIZE(self.object) as usize }, - ObjectType::List => unsafe { PyList_GET_SIZE(self.object) as usize }, - _ => return Err(ser::Error::custom("Object is not a list or tuple")), - }; + let length = unsafe { PyList_GET_SIZE(self.object) as usize }; if length == 0 { serializer.serialize_seq(Some(0))?.end() } else { @@ -166,13 +161,35 @@ impl Serialize for SerializePyObject { let mut ob_type = ObjectType::Str; let mut sequence = serializer.serialize_seq(Some(length))?; for i in 0..length { - let elem = match self.object_type { - ObjectType::Tuple => unsafe { - PyTuple_GET_ITEM(self.object, i as isize) - }, - ObjectType::List => unsafe { PyList_GET_ITEM(self.object, i as isize) }, - _ => return Err(ser::Error::custom("Object is not a list or tuple")), - }; + let elem = unsafe { PyList_GET_ITEM(self.object, i as isize) }; + let current_ob_type = unsafe { Py_TYPE(elem) }; + if current_ob_type != type_ptr { + type_ptr = current_ob_type; + ob_type = get_object_type(current_ob_type); + } + #[allow(clippy::integer_arithmetic)] + sequence.serialize_element(&SerializePyObject::with_obtype( + elem, + ob_type.clone(), + self.recursion_depth + 1, + ))?; + } + sequence.end() + } + } + ObjectType::Tuple => { + if self.recursion_depth == RECURSION_LIMIT { + return Err(ser::Error::custom("Recursion limit reached")); + } + let length = unsafe { PyTuple_GET_SIZE(self.object) as usize }; + if length == 0 { + serializer.serialize_seq(Some(0))?.end() + } else { + let mut type_ptr = std::ptr::null_mut(); + let mut ob_type = ObjectType::Str; + let mut sequence = serializer.serialize_seq(Some(length))?; + for i in 0..length { + let elem = unsafe { PyTuple_GET_ITEM(self.object, i as isize) }; let current_ob_type = unsafe { Py_TYPE(elem) }; if current_ob_type != type_ptr { type_ptr = current_ob_type;