Skip to content

Commit

Permalink
Clean up serialization warnings for invalid data in unions (#1449)
Browse files Browse the repository at this point in the history
  • Loading branch information
sydney-runkle authored Sep 16, 2024
1 parent 16331d7 commit 4a0c332
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 80 deletions.
10 changes: 9 additions & 1 deletion src/serializers/extra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,15 @@ impl CollectWarnings {
if value.is_none() {
Ok(())
} else if extra.check.enabled() {
Err(PydanticSerializationUnexpectedValue::new_err(None))
let type_name = value
.get_type()
.qualname()
.unwrap_or_else(|_| PyString::new_bound(value.py(), "<unknown python object>"));

let value_str = truncate_safe_repr(value, None);
Err(PydanticSerializationUnexpectedValue::new_err(Some(format!(
"Expected `{field_type}` but got `{type_name}` with value `{value_str}` - serialized value may not be as expected"
))))
} else {
self.fallback_warning(field_type, value);
Ok(())
Expand Down
100 changes: 29 additions & 71 deletions src/serializers/type_serializers/union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use std::borrow::Cow;
use crate::build_tools::py_schema_err;
use crate::common::union::{Discriminator, SMALL_UNION_THRESHOLD};
use crate::definitions::DefinitionsBuilder;
use crate::serializers::type_serializers::py_err_se_err;
use crate::tools::{truncate_safe_repr, SchemaDict};
use crate::PydanticSerializationUnexpectedValue;

use super::{
infer_json_key, infer_serialize, infer_to_python, BuildSerializer, CombinedSerializer, Extra, SerCheck,
Expand Down Expand Up @@ -77,7 +75,6 @@ fn to_python(
exclude: Option<&Bound<'_, PyAny>>,
extra: &Extra,
choices: &[CombinedSerializer],
name: &str,
retry_with_lax_check: bool,
) -> PyResult<PyObject> {
// try the serializers in left to right order with error_on fallback=true
Expand All @@ -88,22 +85,15 @@ fn to_python(
for comb_serializer in choices {
match comb_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(value.py()) {
true => (),
false => errors.push(err),
},
Err(err) => errors.push(err),
}
}

if retry_with_lax_check {
new_extra.check = SerCheck::Lax;
for comb_serializer in choices {
match comb_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(value.py()) {
true => (),
false => errors.push(err),
},
if let Ok(v) = comb_serializer.to_python(value, include, exclude, &new_extra) {
return Ok(v);
}
}
}
Expand All @@ -112,15 +102,13 @@ fn to_python(
extra.warnings.custom_warning(err.to_string());
}

extra.warnings.on_fallback_py(name, value, extra)?;
infer_to_python(value, include, exclude, extra)
}

fn json_key<'a>(
key: &'a Bound<'_, PyAny>,
extra: &Extra,
choices: &[CombinedSerializer],
name: &str,
retry_with_lax_check: bool,
) -> PyResult<Cow<'a, str>> {
let mut new_extra = extra.clone();
Expand All @@ -130,22 +118,15 @@ fn json_key<'a>(
for comb_serializer in choices {
match comb_serializer.json_key(key, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(key.py()) {
true => (),
false => errors.push(err),
},
Err(err) => errors.push(err),
}
}

if retry_with_lax_check {
new_extra.check = SerCheck::Lax;
for comb_serializer in choices {
match comb_serializer.json_key(key, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(key.py()) {
true => (),
false => errors.push(err),
},
if let Ok(v) = comb_serializer.json_key(key, &new_extra) {
return Ok(v);
}
}
}
Expand All @@ -154,7 +135,6 @@ fn json_key<'a>(
extra.warnings.custom_warning(err.to_string());
}

extra.warnings.on_fallback_py(name, key, extra)?;
infer_json_key(key, extra)
}

Expand All @@ -166,7 +146,6 @@ fn serde_serialize<S: serde::ser::Serializer>(
exclude: Option<&Bound<'_, PyAny>>,
extra: &Extra,
choices: &[CombinedSerializer],
name: &str,
retry_with_lax_check: bool,
) -> Result<S::Ok, S::Error> {
let py = value.py();
Expand All @@ -177,22 +156,15 @@ fn serde_serialize<S: serde::ser::Serializer>(
for comb_serializer in choices {
match comb_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return infer_serialize(v.bind(py), serializer, None, None, extra),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
true => (),
false => errors.push(err),
},
Err(err) => errors.push(err),
}
}

if retry_with_lax_check {
new_extra.check = SerCheck::Lax;
for comb_serializer in choices {
match comb_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return infer_serialize(v.bind(py), serializer, None, None, extra),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
true => (),
false => errors.push(err),
},
if let Ok(v) = comb_serializer.to_python(value, include, exclude, &new_extra) {
return infer_serialize(v.bind(py), serializer, None, None, extra);
}
}
}
Expand All @@ -201,7 +173,6 @@ fn serde_serialize<S: serde::ser::Serializer>(
extra.warnings.custom_warning(err.to_string());
}

extra.warnings.on_fallback_ser::<S>(name, value, extra)?;
infer_serialize(value, serializer, include, exclude, extra)
}

Expand All @@ -219,13 +190,12 @@ impl TypeSerializer for UnionSerializer {
exclude,
extra,
&self.choices,
self.get_name(),
self.retry_with_lax_check(),
)
}

fn json_key<'a>(&self, key: &'a Bound<'_, PyAny>, extra: &Extra) -> PyResult<Cow<'a, str>> {
json_key(key, extra, &self.choices, self.get_name(), self.retry_with_lax_check())
json_key(key, extra, &self.choices, self.retry_with_lax_check())
}

fn serde_serialize<S: serde::ser::Serializer>(
Expand All @@ -243,7 +213,6 @@ impl TypeSerializer for UnionSerializer {
exclude,
extra,
&self.choices,
self.get_name(),
self.retry_with_lax_check(),
)
}
Expand Down Expand Up @@ -313,8 +282,6 @@ impl TypeSerializer for TaggedUnionSerializer {
exclude: Option<&Bound<'_, PyAny>>,
extra: &Extra,
) -> PyResult<PyObject> {
let py = value.py();

let mut new_extra = extra.clone();
new_extra.check = SerCheck::Strict;

Expand All @@ -325,15 +292,14 @@ impl TypeSerializer for TaggedUnionSerializer {

match serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
true => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
return serializer.to_python(value, include, exclude, &new_extra);
Err(_) => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
if let Ok(v) = serializer.to_python(value, include, exclude, &new_extra) {
return Ok(v);
}
}
false => return Err(err),
},
}
}
}
}
Expand All @@ -344,13 +310,11 @@ impl TypeSerializer for TaggedUnionSerializer {
exclude,
extra,
&self.choices,
self.get_name(),
self.retry_with_lax_check(),
)
}

fn json_key<'a>(&self, key: &'a Bound<'_, PyAny>, extra: &Extra) -> PyResult<Cow<'a, str>> {
let py = key.py();
let mut new_extra = extra.clone();
new_extra.check = SerCheck::Strict;

Expand All @@ -361,20 +325,19 @@ impl TypeSerializer for TaggedUnionSerializer {

match serializer.json_key(key, &new_extra) {
Ok(v) => return Ok(v),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
true => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
return serializer.json_key(key, &new_extra);
Err(_) => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
if let Ok(v) = serializer.json_key(key, &new_extra) {
return Ok(v);
}
}
false => return Err(err),
},
}
}
}
}

json_key(key, extra, &self.choices, self.get_name(), self.retry_with_lax_check())
json_key(key, extra, &self.choices, self.retry_with_lax_check())
}

fn serde_serialize<S: serde::ser::Serializer>(
Expand All @@ -396,18 +359,14 @@ impl TypeSerializer for TaggedUnionSerializer {

match selected_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return infer_serialize(v.bind(py), serializer, None, None, extra),
Err(err) => match err.is_instance_of::<PydanticSerializationUnexpectedValue>(py) {
true => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
match selected_serializer.to_python(value, include, exclude, &new_extra) {
Ok(v) => return infer_serialize(v.bind(py), serializer, None, None, extra),
Err(err) => return Err(py_err_se_err(err)),
}
Err(_) => {
if self.retry_with_lax_check() {
new_extra.check = SerCheck::Lax;
if let Ok(v) = selected_serializer.to_python(value, include, exclude, &new_extra) {
return infer_serialize(v.bind(py), serializer, None, None, extra);
}
}
false => return Err(py_err_se_err(err)),
},
}
}
}
}
Expand All @@ -419,7 +378,6 @@ impl TypeSerializer for TaggedUnionSerializer {
exclude,
extra,
&self.choices,
self.get_name(),
self.retry_with_lax_check(),
)
}
Expand Down
5 changes: 1 addition & 4 deletions tests/serializers/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

from pydantic_core import (
PydanticSerializationError,
PydanticSerializationUnexpectedValue,
SchemaSerializer,
SchemaValidator,
core_schema,
Expand Down Expand Up @@ -1150,8 +1149,6 @@ class BModel(BasicModel): ...
)
)

with pytest.raises(
PydanticSerializationUnexpectedValue, match='Expected 2 fields but got 1 for type `.*AModel` with value `.*`.+'
):
with pytest.warns(UserWarning, match='Expected 2 fields but got 1 for type `.*AModel` with value `.*`.+'):
value = BasicModel(root=AModel(type='a'))
s.to_python(value)
16 changes: 12 additions & 4 deletions tests/serializers/test_union.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import dataclasses
import json
import re
import uuid
import warnings
from decimal import Decimal
from typing import Any, ClassVar, Union

Expand Down Expand Up @@ -32,9 +32,17 @@ def test_union_bool_int(input_value, expected_value, bool_case_label, int_case_l

def test_union_error():
s = SchemaSerializer(core_schema.union_schema([core_schema.bool_schema(), core_schema.int_schema()]))
msg = "Expected `Union[bool, int]` but got `str` with value `'a string'` - serialized value may not be as expected"
with pytest.warns(UserWarning, match=re.escape(msg)):
assert s.to_python('a string') == 'a string'

messages = [
"Expected `bool` but got `str` with value `'a string'` - serialized value may not be as expected",
"Expected `int` but got `str` with value `'a string'` - serialized value may not be as expected",
]

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
s.to_python('a string')
for m in messages:
assert m in str(w[0].message)


class ModelA:
Expand Down

0 comments on commit 4a0c332

Please sign in to comment.