Skip to content

Commit 4088903

Browse files
sydney-runklesydney-runkle
committed
fixing tests, more refactoring
1 parent 37efc68 commit 4088903

File tree

14 files changed

+138
-106
lines changed

14 files changed

+138
-106
lines changed

‎src/serializers/errors.rs‎

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ pub(super) fn se_err_py_err(error: PythonSerializerError) -> PyErr {
4747
let s = error.to_string();
4848
if let Some(msg) = s.strip_prefix(UNEXPECTED_TYPE_SER_MARKER) {
4949
if msg.is_empty() {
50-
PydanticSerializationUnexpectedValue::new_from_msg(None)
50+
PydanticSerializationUnexpectedValue::new_from_msg(None).to_py_err()
5151
} else {
52-
PydanticSerializationUnexpectedValue::new_from_msg(Some(msg.to_string()))
52+
PydanticSerializationUnexpectedValue::new_from_msg(Some(msg.to_string())).to_py_err()
5353
}
5454
} else if let Some(msg) = s.strip_prefix(SERIALIZATION_ERR_MARKER) {
5555
PydanticSerializationError::new_err(msg.to_string())
@@ -102,15 +102,35 @@ pub struct PydanticSerializationUnexpectedValue {
102102
}
103103

104104
impl PydanticSerializationUnexpectedValue {
105-
pub fn new_from_msg(message: Option<String>) -> PyErr {
106-
PyErr::new::<Self, (Option<String>, Option<String>, Option<PyObject>)>((message, None, None))
105+
pub fn new_from_msg(message: Option<String>) -> Self {
106+
Self {
107+
message,
108+
field_type: None,
109+
input_value: None,
110+
}
111+
}
112+
113+
pub fn new_from_parts(field_type: Option<String>, input_value: Option<PyObject>) -> Self {
114+
Self {
115+
message: None,
116+
field_type,
117+
input_value,
118+
}
119+
}
120+
121+
pub fn new(message: Option<String>, field_type: Option<String>, input_value: Option<PyObject>) -> Self {
122+
Self {
123+
message,
124+
field_type,
125+
input_value,
126+
}
107127
}
108128

109-
pub fn new_from_parts(field_type: String, input_value: PyObject) -> PyErr {
129+
pub fn to_py_err(&self) -> PyErr {
110130
PyErr::new::<Self, (Option<String>, Option<String>, Option<PyObject>)>((
111-
None,
112-
Some(field_type),
113-
Some(input_value),
131+
self.message.clone(),
132+
self.field_type.clone(),
133+
self.input_value.clone(),
114134
))
115135
}
116136
}
@@ -127,24 +147,31 @@ impl PydanticSerializationUnexpectedValue {
127147
}
128148
}
129149

130-
fn __str__(&self, py: Python) -> String {
131-
match &self.message {
132-
Some(s) => s.to_string(),
133-
None => match (&self.field_type, &self.input_value) {
134-
(Some(ref field_type), Some(ref input_value)) => {
135-
let bound_input = input_value.bind(py);
136-
137-
let type_name = bound_input
138-
.get_type()
139-
.qualname()
140-
.unwrap_or_else(|_| PyString::new(py, "<unknown python object>"));
141-
142-
let value_str = truncate_safe_repr(bound_input, None);
143-
format!("Expected `{field_type}` but got `{type_name}` with value `{value_str}` - serialized value may not be as expected")
144-
}
145-
_ => "Unexpected value - serialized value may not be as expected".to_string(),
146-
},
150+
pub(crate) fn __str__(&self, py: Python) -> String {
151+
let mut message = self.message.as_deref().unwrap_or("Unexpected Value").to_string();
152+
153+
if let Some(field_type) = &self.field_type {
154+
message.push_str(&format!("Expected `{field_type}`"));
155+
}
156+
157+
if let Some(input_value) = &self.input_value {
158+
let bound_input = input_value.bind(py);
159+
let type_name = bound_input
160+
.get_type()
161+
.name()
162+
.unwrap_or_else(|_| PyString::new(py, "<unknown python object>"))
163+
.to_string();
164+
165+
let value_str = truncate_safe_repr(bound_input, None);
166+
167+
message.push_str(&format!(" but got `{type_name}` with value `{value_str}`"));
147168
}
169+
170+
if self.input_value.is_some() || self.field_type.is_some() {
171+
message.push_str(" - serialized value may not be as expected.");
172+
}
173+
174+
message
148175
}
149176

150177
pub(crate) fn __repr__(&self, py: Python) -> String {

‎src/serializers/extra.rs‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::ffi::CString;
33
use std::fmt;
44
use std::sync::Mutex;
55

6-
use crate::tools::truncate_safe_repr;
76
use pyo3::exceptions::{PyTypeError, PyUserWarning, PyValueError};
87
use pyo3::prelude::*;
98
use pyo3::types::{PyBool, PyString};
@@ -384,7 +383,7 @@ impl From<bool> for WarningsMode {
384383
pub(crate) struct CollectWarnings {
385384
mode: WarningsMode,
386385
// FIXME: mutex is to satisfy PyO3 0.23, we should be able to refactor this away
387-
warnings: Mutex<Vec<String>>,
386+
warnings: Mutex<Vec<PydanticSerializationUnexpectedValue>>,
388387
}
389388

390389
impl Clone for CollectWarnings {
@@ -404,9 +403,9 @@ impl CollectWarnings {
404403
}
405404
}
406405

407-
pub fn custom_warning(&self, warning: String) {
406+
pub fn register_warning(&self, warning: PydanticSerializationUnexpectedValue) {
408407
if self.mode != WarningsMode::None {
409-
self.add_warning(warning);
408+
self.warnings.lock().expect("lock poisoned").push(warning);
410409
}
411410
}
412411

@@ -416,9 +415,10 @@ impl CollectWarnings {
416415
Ok(())
417416
} else if extra.check.enabled() {
418417
Err(PydanticSerializationUnexpectedValue::new_from_parts(
419-
field_type.to_string(),
420-
value.clone().unbind(),
421-
))
418+
Some(field_type.to_string()),
419+
Some(value.clone().unbind()),
420+
)
421+
.to_py_err())
422422
} else {
423423
self.fallback_warning(field_type, value);
424424
Ok(())
@@ -447,23 +447,13 @@ impl CollectWarnings {
447447

448448
fn fallback_warning(&self, field_type: &str, value: &Bound<'_, PyAny>) {
449449
if self.mode != WarningsMode::None {
450-
let type_name = value
451-
.get_type()
452-
.qualname()
453-
.unwrap_or_else(|_| PyString::new(value.py(), "<unknown python object>"));
454-
455-
let value_str = truncate_safe_repr(value, None);
456-
457-
self.add_warning(format!(
458-
"Expected `{field_type}` but got `{type_name}` with value `{value_str}` - serialized value may not be as expected"
450+
self.register_warning(PydanticSerializationUnexpectedValue::new_from_parts(
451+
Some(field_type.to_string()),
452+
Some(value.clone().unbind()),
459453
));
460454
}
461455
}
462456

463-
fn add_warning(&self, message: String) {
464-
self.warnings.lock().expect("lock poisoned").push(message);
465-
}
466-
467457
pub fn final_check(&self, py: Python) -> PyResult<()> {
468458
if self.mode == WarningsMode::None {
469459
return Ok(());
@@ -474,7 +464,9 @@ impl CollectWarnings {
474464
return Ok(());
475465
}
476466

477-
let message = format!("Pydantic serializer warnings:\n {}", warnings.join("\n "));
467+
let formatted_warnings: Vec<String> = warnings.iter().map(|w| w.__repr__(py).to_string()).collect();
468+
469+
let message = format!("Pydantic serializer warnings:\n {}", formatted_warnings.join("\n "));
478470
if self.mode == WarningsMode::Warn {
479471
let user_warning_type = PyUserWarning::type_object(py);
480472
PyErr::warn(py, &user_warning_type, &CString::new(message)?, 0)

‎src/serializers/fields.rs‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use smallvec::SmallVec;
99

1010
use crate::serializers::extra::SerCheck;
1111
use crate::serializers::DuckTypingSerMode;
12-
use crate::tools::truncate_safe_repr;
1312
use crate::PydanticSerializationUnexpectedValue;
1413

1514
use super::computed_fields::ComputedFields;
@@ -201,15 +200,12 @@ impl GeneralFieldsSerializer {
201200
};
202201
output_dict.set_item(key, value)?;
203202
} else if field_extra.check == SerCheck::Strict {
204-
let type_name = field_extra.model_type_name();
205-
return Err(PydanticSerializationUnexpectedValue::new_from_msg(Some(format!(
206-
"Unexpected field `{key}`{for_type_name}",
207-
for_type_name = if let Some(type_name) = type_name {
208-
format!(" for type `{type_name}`")
209-
} else {
210-
String::new()
211-
},
212-
))));
203+
return Err(PydanticSerializationUnexpectedValue::new(
204+
Some(format!("Unexpected field `{key}`")),
205+
field_extra.model_type_name().map(|bound| bound.to_string()),
206+
None,
207+
)
208+
.to_py_err());
213209
}
214210
}
215211
}
@@ -221,16 +217,13 @@ impl GeneralFieldsSerializer {
221217
&& self.required_fields > used_req_fields
222218
{
223219
let required_fields = self.required_fields;
224-
let type_name = extra.model_type_name();
225-
let field_value = match extra.model {
226-
Some(model) => truncate_safe_repr(model, Some(100)),
227-
None => "<unknown python object>".to_string(),
228-
};
229220

230-
Err(PydanticSerializationUnexpectedValue::new_from_msg(Some(format!(
231-
"Expected {required_fields} fields but got {used_req_fields}{for_type_name} with value `{field_value}` - serialized value may not be as expected.",
232-
for_type_name = if let Some(type_name) = type_name { format!(" for type `{type_name}`") } else { String::new() },
233-
))))
221+
Err(PydanticSerializationUnexpectedValue::new(
222+
Some(format!("Expected {required_fields} fields but got {used_req_fields}. ").to_string()),
223+
extra.model_type_name().map(|bound| bound.to_string()),
224+
extra.model.map(|bound| bound.clone().unbind()),
225+
)
226+
.to_py_err())
234227
} else {
235228
Ok(output_dict)
236229
}

‎src/serializers/type_serializers/datetime_etc.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn downcast_date_reject_datetime<'a, 'py>(py_date: &'a Bound<'py, PyAny>) -> PyR
3434
}
3535
}
3636

37-
Err(PydanticSerializationUnexpectedValue::new_from_msg(None))
37+
Err(PydanticSerializationUnexpectedValue::new_from_msg(None).to_py_err())
3838
}
3939

4040
macro_rules! build_serializer {

‎src/serializers/type_serializers/float.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl TypeSerializer for FloatSerializer {
7575
match extra.ob_type_lookup.is_type(value, ObType::Float) {
7676
IsType::Exact => Ok(value.clone().unbind()),
7777
IsType::Subclass => match extra.check {
78-
SerCheck::Strict => Err(PydanticSerializationUnexpectedValue::new_from_msg(None)),
78+
SerCheck::Strict => Err(PydanticSerializationUnexpectedValue::new_from_msg(None).to_py_err()),
7979
SerCheck::Lax | SerCheck::None => match extra.mode {
8080
SerMode::Json => value.extract::<f64>()?.into_py_any(py),
8181
_ => infer_to_python(value, include, exclude, extra),

‎src/serializers/type_serializers/function.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ fn on_error(py: Python, err: PyErr, function_name: &str, extra: &Extra) -> PyRes
194194
if extra.check.enabled() {
195195
Err(err)
196196
} else {
197-
extra.warnings.custom_warning(ser_err.__repr__(py));
197+
extra.warnings.register_warning(ser_err);
198198
Ok(())
199199
}
200200
} else if let Ok(err) = exception.extract::<PydanticSerializationError>() {

‎src/serializers/type_serializers/model.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ impl TypeSerializer for ModelSerializer {
190190
let py = value.py();
191191
let root = value.getattr(intern!(py, ROOT_FIELD)).map_err(|original_err| {
192192
if root_extra.check.enabled() {
193-
PydanticSerializationUnexpectedValue::new_from_msg(None)
193+
PydanticSerializationUnexpectedValue::new_from_msg(None).to_py_err()
194194
} else {
195195
original_err
196196
}

‎src/serializers/type_serializers/simple.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ macro_rules! build_simple_serializer {
123123
match extra.ob_type_lookup.is_type(value, $ob_type) {
124124
IsType::Exact => Ok(value.clone().unbind()),
125125
IsType::Subclass => match extra.check {
126-
SerCheck::Strict => Err(PydanticSerializationUnexpectedValue::new_from_msg(None)),
126+
SerCheck::Strict => Err(PydanticSerializationUnexpectedValue::new_from_msg(None).to_py_err()),
127127
SerCheck::Lax | SerCheck::None => match extra.mode {
128128
SerMode::Json => value.extract::<$rust_type>()?.into_py_any(py),
129129
_ => infer_to_python(value, include, exclude, extra),

‎src/serializers/type_serializers/tuple.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,18 @@ impl TupleSerializer {
218218
"Expected {} items, but got {}",
219219
self.serializers.len(),
220220
n_items
221-
))));
221+
)))
222+
.to_py_err());
222223
} else {
223224
use_serializers!(self.serializers.iter());
224225
let mut warned = false;
225226
for (i, element) in py_tuple_iter.enumerate() {
226227
if !warned {
227228
extra
228229
.warnings
229-
.custom_warning("Unexpected extra items present in tuple".to_string());
230+
.register_warning(PydanticSerializationUnexpectedValue::new_from_msg(Some(
231+
"Unexpected extra items present in tuple".to_string(),
232+
)));
230233
warned = true;
231234
}
232235
let op_next = self

‎src/serializers/type_serializers/union.rs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::build_tools::py_schema_err;
99
use crate::common::union::{Discriminator, SMALL_UNION_THRESHOLD};
1010
use crate::definitions::DefinitionsBuilder;
1111
use crate::serializers::PydanticSerializationUnexpectedValue;
12-
use crate::tools::{truncate_safe_repr, SchemaDict};
12+
use crate::tools::SchemaDict;
1313

1414
use super::{
1515
infer_json_key, infer_serialize, infer_to_python, BuildSerializer, CombinedSerializer, Extra, SerCheck,
@@ -78,6 +78,7 @@ fn union_serialize<S>(
7878
extra: &Extra,
7979
choices: &[CombinedSerializer],
8080
retry_with_lax_check: bool,
81+
py: Python<'_>,
8182
) -> PyResult<Option<S>> {
8283
// try the serializers in left to right order with error_on fallback=true
8384
let mut new_extra = extra.clone();
@@ -104,14 +105,23 @@ fn union_serialize<S>(
104105
// If extra.check is SerCheck::None, we're in a top-level union. We should thus raise the warnings
105106
if extra.check == SerCheck::None {
106107
for err in &errors {
107-
extra.warnings.custom_warning(err.to_string());
108+
if err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
109+
let pydantic_err: PydanticSerializationUnexpectedValue = err.value(py).extract()?;
110+
extra.warnings.register_warning(pydantic_err);
111+
} else {
112+
extra
113+
.warnings
114+
.register_warning(PydanticSerializationUnexpectedValue::new_from_msg(Some(
115+
err.to_string(),
116+
)));
117+
}
108118
}
109119
}
110120
// Otherwise, if we've encountered errors, return them to the parent union, which should take
111121
// care of the formatting for us
112122
else if !errors.is_empty() {
113123
let message = errors.iter().map(ToString::to_string).collect::<Vec<_>>().join("\n");
114-
return Err(PydanticSerializationUnexpectedValue::new_from_msg(Some(message)));
124+
return Err(PydanticSerializationUnexpectedValue::new_from_msg(Some(message)).to_py_err());
115125
}
116126

117127
Ok(None)
@@ -130,6 +140,7 @@ impl TypeSerializer for UnionSerializer {
130140
extra,
131141
&self.choices,
132142
self.retry_with_lax_check(),
143+
value.py(),
133144
)?
134145
.map_or_else(|| infer_to_python(value, include, exclude, extra), Ok)
135146
}
@@ -140,6 +151,7 @@ impl TypeSerializer for UnionSerializer {
140151
extra,
141152
&self.choices,
142153
self.retry_with_lax_check(),
154+
key.py(),
143155
)?
144156
.map_or_else(|| infer_json_key(key, extra), Ok)
145157
}
@@ -157,6 +169,7 @@ impl TypeSerializer for UnionSerializer {
157169
extra,
158170
&self.choices,
159171
self.retry_with_lax_check(),
172+
value.py(),
160173
) {
161174
Ok(Some(v)) => infer_serialize(v.bind(value.py()), serializer, None, None, extra),
162175
Ok(None) => infer_serialize(value, serializer, include, exclude, extra),
@@ -328,17 +341,18 @@ impl TaggedUnionSerializer {
328341
} else if extra.check == SerCheck::None {
329342
// If extra.check is SerCheck::None, we're in a top-level union. We should thus raise
330343
// this warning
331-
let value_str = truncate_safe_repr(value, None);
332-
extra.warnings.custom_warning(
333-
format!(
334-
"Failed to get discriminator value for tagged union serialization with value `{value_str}` - defaulting to left to right union serialization."
344+
extra.warnings.register_warning(
345+
PydanticSerializationUnexpectedValue::new(
346+
Some("Defaulting to left to right union serialization - failed to get discriminator value for tagged union serialization".to_string()),
347+
None,
348+
Some(value.clone().unbind()),
335349
)
336350
);
337351
}
338352

339353
// if we haven't returned at this point, we should fallback to the union serializer
340354
// which preserves the historical expectation that we do our best with serialization
341355
// even if that means we resort to inference
342-
union_serialize(selector, extra, &self.choices, self.retry_with_lax_check())
356+
union_serialize(selector, extra, &self.choices, self.retry_with_lax_check(), value.py())
343357
}
344358
}

0 commit comments

Comments
 (0)