Skip to content

Commit f892755

Browse files
authored
avoid getting default values from defaultdict (#1853)
1 parent 3359241 commit f892755

File tree

8 files changed

+119
-133
lines changed

8 files changed

+119
-133
lines changed

src/argument_markers.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,9 @@ impl PydanticUndefinedType {
6060
}
6161

6262
#[staticmethod]
63-
pub fn new(py: Python) -> Py<Self> {
64-
UNDEFINED_CELL
65-
.get_or_init(py, || Py::new(py, PydanticUndefinedType {}).unwrap())
66-
.clone_ref(py)
63+
#[pyo3(name = "new")]
64+
pub fn get(py: Python<'_>) -> &Py<Self> {
65+
UNDEFINED_CELL.get_or_init(py, || Py::new(py, PydanticUndefinedType {}).unwrap())
6766
}
6867

6968
fn __repr__(&self) -> &'static str {

src/input/input_python.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ impl<'py> KeywordArgs<'py> for PyKwargs<'py> {
833833
&self,
834834
key: &'k crate::lookup_key::LookupKey,
835835
) -> ValResult<Option<(&'k crate::lookup_key::LookupPath, Self::Item<'_>)>> {
836-
key.py_get_dict_item(&self.0)
836+
key.py_get_dict_item(&self.0).map_err(Into::into)
837837
}
838838

839839
fn iter(&self) -> impl Iterator<Item = ValResult<(Self::Key<'_>, Self::Item<'_>)>> {
@@ -864,8 +864,8 @@ impl<'py> ValidatedDict<'py> for GenericPyMapping<'_, 'py> {
864864
key: &'k crate::lookup_key::LookupKey,
865865
) -> ValResult<Option<(&'k crate::lookup_key::LookupPath, Self::Item<'_>)>> {
866866
match self {
867-
Self::Dict(dict) => key.py_get_dict_item(dict),
868-
Self::Mapping(mapping) => key.py_get_mapping_item(mapping),
867+
Self::Dict(dict) => key.py_get_dict_item(dict).map_err(Into::into),
868+
Self::Mapping(mapping) => key.py_get_mapping_item(mapping).map_err(Into::into),
869869
Self::GetAttr(obj, dict) => key.py_get_attr(obj, dict.as_ref()),
870870
}
871871
}

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub mod _pydantic_core {
125125
m.add("build_profile", env!("PROFILE"))?;
126126
m.add("build_info", build_info())?;
127127
m.add("_recursion_limit", recursion_guard::RECURSION_GUARD_LIMIT)?;
128-
m.add("PydanticUndefined", PydanticUndefinedType::new(m.py()))?;
128+
m.add("PydanticUndefined", PydanticUndefinedType::get(m.py()))?;
129129
Ok(())
130130
}
131131
}

src/lookup_key.rs

Lines changed: 69 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use jiter::{JsonObject, JsonValue};
1111
use crate::build_tools::py_schema_err;
1212
use crate::errors::{py_err_string, ErrorType, LocItem, Location, ToErrorValue, ValError, ValLineError, ValResult};
1313
use crate::input::StringMapping;
14-
use crate::tools::{extract_i64, py_err};
14+
use crate::tools::{extract_i64, mapping_get, py_err};
1515

1616
/// Used for getting items from python dicts, python objects, or JSON objects, in different ways
1717
#[derive(Debug)]
@@ -89,44 +89,12 @@ impl LookupKey {
8989
pub fn py_get_dict_item<'py, 's>(
9090
&'s self,
9191
dict: &Bound<'py, PyDict>,
92-
) -> ValResult<Option<(&'s LookupPath, Bound<'py, PyAny>)>> {
93-
match self {
94-
Self::Simple(path) => match dict.get_item(&path.first_item.py_key)? {
95-
Some(value) => {
96-
debug_assert!(path.rest.is_empty());
97-
Ok(Some((path, value)))
98-
}
99-
None => Ok(None),
100-
},
101-
Self::Choice { path1, path2, .. } => match dict.get_item(&path1.first_item.py_key)? {
102-
Some(value) => {
103-
debug_assert!(path1.rest.is_empty());
104-
Ok(Some((path1, value)))
105-
}
106-
None => match dict.get_item(&path2.first_item.py_key)? {
107-
Some(value) => {
108-
debug_assert!(path2.rest.is_empty());
109-
Ok(Some((path2, value)))
110-
}
111-
None => Ok(None),
112-
},
113-
},
114-
Self::PathChoices(path_choices) => {
115-
for path in path_choices {
116-
let Some(first_value) = dict.get_item(&path.first_item.py_key)? else {
117-
continue;
118-
};
119-
// iterate over the path and plug each value into the py_any from the last step,
120-
// this could just be a loop but should be somewhat faster with a functional design
121-
if let Some(v) = path.rest.iter().try_fold(first_value, |d, loc| loc.py_get_item(&d)) {
122-
// Successfully found an item, return it
123-
return Ok(Some((path, v)));
124-
}
125-
}
126-
// got to the end of path_choices, without a match, return None
127-
Ok(None)
128-
}
129-
}
92+
) -> PyResult<Option<(&'s LookupPath, Bound<'py, PyAny>)>> {
93+
self.get_impl(
94+
dict,
95+
|dict, path| dict.get_item(&path.py_key),
96+
|d, loc| Ok(loc.py_get_item(&d)),
97+
)
13098
}
13199

132100
pub fn py_get_string_mapping_item<'py, 's>(
@@ -144,94 +112,23 @@ impl LookupKey {
144112
pub fn py_get_mapping_item<'py, 's>(
145113
&'s self,
146114
dict: &Bound<'py, PyMapping>,
147-
) -> ValResult<Option<(&'s LookupPath, Bound<'py, PyAny>)>> {
148-
match self {
149-
Self::Simple(path) => match dict.get_item(&path.first_item.py_key) {
150-
Ok(value) => {
151-
debug_assert!(path.rest.is_empty());
152-
Ok(Some((path, value)))
153-
}
154-
_ => Ok(None),
155-
},
156-
Self::Choice { path1, path2, .. } => match dict.get_item(&path1.first_item.py_key) {
157-
Ok(value) => {
158-
debug_assert!(path1.rest.is_empty());
159-
Ok(Some((path1, value)))
160-
}
161-
_ => match dict.get_item(&path2.first_item.py_key) {
162-
Ok(value) => {
163-
debug_assert!(path2.rest.is_empty());
164-
Ok(Some((path2, value)))
165-
}
166-
_ => Ok(None),
167-
},
168-
},
169-
Self::PathChoices(path_choices) => {
170-
for path in path_choices {
171-
let Some(first_value) = dict.get_item(&path.first_item.py_key).ok() else {
172-
continue;
173-
};
174-
// iterate over the path and plug each value into the py_any from the last step,
175-
// this could just be a loop but should be somewhat faster with a functional design
176-
if let Some(v) = path.rest.iter().try_fold(first_value, |d, loc| loc.py_get_item(&d)) {
177-
// Successfully found an item, return it
178-
return Ok(Some((path, v)));
179-
}
180-
}
181-
// got to the end of path_choices, without a match, return None
182-
Ok(None)
183-
}
184-
}
115+
) -> PyResult<Option<(&'s LookupPath, Bound<'py, PyAny>)>> {
116+
self.get_impl(
117+
dict,
118+
|dict, path| mapping_get(dict, &path.py_key),
119+
|d, loc| Ok(loc.py_get_item(&d)),
120+
)
185121
}
186122

187123
pub fn simple_py_get_attr<'py, 's>(
188124
&'s self,
189125
obj: &Bound<'py, PyAny>,
190126
) -> PyResult<Option<(&'s LookupPath, Bound<'py, PyAny>)>> {
191-
match self {
192-
Self::Simple(path) => match py_get_attrs(obj, &path.first_item.py_key)? {
193-
Some(value) => {
194-
debug_assert!(path.rest.is_empty());
195-
Ok(Some((path, value)))
196-
}
197-
None => Ok(None),
198-
},
199-
Self::Choice { path1, path2, .. } => match py_get_attrs(obj, &path1.first_item.py_key)? {
200-
Some(value) => {
201-
debug_assert!(path1.rest.is_empty());
202-
Ok(Some((path1, value)))
203-
}
204-
None => match py_get_attrs(obj, &path2.first_item.py_key)? {
205-
Some(value) => {
206-
debug_assert!(path2.rest.is_empty());
207-
Ok(Some((path2, value)))
208-
}
209-
None => Ok(None),
210-
},
211-
},
212-
Self::PathChoices(path_choices) => {
213-
'outer: for path in path_choices {
214-
// similar to above, but using `py_get_attrs`, we can't use try_fold because of the extra Err
215-
// so we have to loop manually
216-
let Some(mut v) = path.first_item.py_get_attrs(obj)? else {
217-
continue;
218-
};
219-
for loc in &path.rest {
220-
v = match loc.py_get_attrs(&v) {
221-
Ok(Some(v)) => v,
222-
Ok(None) => {
223-
continue 'outer;
224-
}
225-
Err(e) => return Err(e),
226-
}
227-
}
228-
// Successfully found an item, return it
229-
return Ok(Some((path, v)));
230-
}
231-
// got to the end of path_choices, without a match, return None
232-
Ok(None)
233-
}
234-
}
127+
self.get_impl(
128+
obj,
129+
|obj, path| py_get_attrs(obj, &path.py_key),
130+
|d, loc| loc.py_get_attrs(&d),
131+
)
235132
}
236133

237134
pub fn py_get_attr<'py, 's>(
@@ -324,6 +221,57 @@ impl LookupKey {
324221
}
325222
}
326223

224+
fn get_impl<'s, 'a, SourceT, OutputT: 'a>(
225+
&'s self,
226+
source: &'a SourceT,
227+
lookup: impl Fn(&'a SourceT, &'s PathItemString) -> PyResult<Option<OutputT>>,
228+
nested_lookup: impl Fn(OutputT, &'s PathItem) -> PyResult<Option<OutputT>>,
229+
) -> PyResult<Option<(&'s LookupPath, OutputT)>> {
230+
match self {
231+
Self::Simple(path) => match lookup(source, &path.first_item)? {
232+
Some(value) => {
233+
debug_assert!(path.rest.is_empty());
234+
Ok(Some((path, value)))
235+
}
236+
None => Ok(None),
237+
},
238+
Self::Choice { path1, path2, .. } => match lookup(source, &path1.first_item)? {
239+
Some(value) => {
240+
debug_assert!(path1.rest.is_empty());
241+
Ok(Some((path1, value)))
242+
}
243+
None => match lookup(source, &path2.first_item)? {
244+
Some(value) => {
245+
debug_assert!(path2.rest.is_empty());
246+
Ok(Some((path2, value)))
247+
}
248+
None => Ok(None),
249+
},
250+
},
251+
Self::PathChoices(path_choices) => {
252+
'choices: for path in path_choices {
253+
let Some(mut value) = lookup(source, &path.first_item)? else {
254+
continue;
255+
};
256+
257+
// iterate over the path and plug each value into the value from the last step
258+
for loc in &path.rest {
259+
value = match nested_lookup(value, loc) {
260+
Ok(Some(v)) => v,
261+
// this choice did not match, try the next one
262+
Ok(None) => continue 'choices,
263+
Err(e) => return Err(e),
264+
}
265+
}
266+
// Successfully found an item, return it
267+
return Ok(Some((path, value)));
268+
}
269+
// got to the end of path_choices, without a match, return None
270+
Ok(None)
271+
}
272+
}
273+
}
274+
327275
pub fn error(
328276
&self,
329277
error_type: ErrorType,

src/tools.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ use num_bigint::BigInt;
44

55
use pyo3::exceptions::PyKeyError;
66
use pyo3::prelude::*;
7-
use pyo3::types::{PyDict, PyString};
7+
use pyo3::types::{PyDict, PyMapping, PyString};
88
use pyo3::{intern, FromPyObject};
99

1010
use crate::input::Int;
11+
use crate::PydanticUndefinedType;
1112
use jiter::{cached_py_string, StringCacheMode};
1213

1314
pub trait SchemaDict<'py> {
@@ -190,3 +191,14 @@ pub fn write_truncated_to_limited_bytes<F: fmt::Write>(f: &mut F, val: &str, max
190191
write!(f, "{val}")
191192
}
192193
}
194+
195+
/// Implementation of `mapping.get(key, PydanticUndefined)` which returns `None` if the key is not found
196+
pub fn mapping_get<'py>(
197+
mapping: &Bound<'py, PyMapping>,
198+
key: impl IntoPyObject<'py>,
199+
) -> PyResult<Option<Bound<'py, PyAny>>> {
200+
let undefined = PydanticUndefinedType::get(mapping.py());
201+
mapping
202+
.call_method1(intern!(mapping.py(), "get"), (key, undefined))
203+
.map(|value| if value.is(undefined) { None } else { Some(value) })
204+
}

src/validators/model.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl BuildValidator for ModelValidator {
101101
frozen: schema.get_as(intern!(py, "frozen"))?.unwrap_or(false),
102102
custom_init: schema.get_as(intern!(py, "custom_init"))?.unwrap_or(false),
103103
root_model: schema.get_as(intern!(py, "root_model"))?.unwrap_or(false),
104-
undefined: PydanticUndefinedType::new(py).into_any(),
104+
undefined: PydanticUndefinedType::get(py).clone_ref(schema.py()).into_any(),
105105
// Get the class's `__name__`, not using `class.qualname()`
106106
name,
107107
})

src/validators/with_default.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ impl BuildValidator for WithDefaultValidator {
144144
validate_default: schema_or_config_same(schema, config, intern!(py, "validate_default"))?.unwrap_or(false),
145145
copy_default,
146146
name,
147-
undefined: PydanticUndefinedType::new(py).into_any(),
147+
undefined: PydanticUndefinedType::get(py).clone_ref(schema.py()).into_any(),
148148
})
149149
.into())
150150
}
@@ -187,7 +187,7 @@ impl Validator for WithDefaultValidator {
187187
// in an unhelpul error.
188188
let mut err = ValError::new(
189189
ErrorTypeDefaults::DefaultFactoryNotCalled,
190-
PydanticUndefinedType::new(py).into_bound(py).into_any(),
190+
PydanticUndefinedType::get(py).bind(py).clone().into_any(),
191191
);
192192
if let Some(outer_loc) = outer_loc {
193193
err = err.with_outer_location(outer_loc);

tests/validators/test_model.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import re
22
import sys
3+
from collections import defaultdict
34
from copy import deepcopy
45
from decimal import Decimal
56
from typing import Any, Callable, Union
@@ -1359,3 +1360,29 @@ class MyModel:
13591360
v.validate_assignment(m, 'enum_field', Decimal(1))
13601361
v.validate_assignment(m, 'enum_field_2', Decimal(2))
13611362
v.validate_assignment(m, 'enum_field_3', IntWrappable(3))
1363+
1364+
1365+
def test_model_from_defaultdict():
1366+
# https://github.com/pydantic/pydantic/issues/12376
1367+
1368+
class MyModel:
1369+
def __init__(self, **kwargs: Any) -> None:
1370+
self.__dict__.update(kwargs)
1371+
1372+
schema = core_schema.model_schema(
1373+
MyModel, core_schema.model_fields_schema({'field_a': core_schema.model_field(core_schema.int_schema())})
1374+
)
1375+
1376+
v = SchemaValidator(schema)
1377+
with pytest.raises(ValidationError) as exc_info:
1378+
# the defaultdict should not provide default values for missing fields
1379+
v.validate_python(defaultdict(int))
1380+
1381+
assert exc_info.value.errors(include_url=False) == [
1382+
{
1383+
'type': 'missing',
1384+
'loc': ('field_a',),
1385+
'msg': 'Field required',
1386+
'input': defaultdict(int),
1387+
}
1388+
]

0 commit comments

Comments
 (0)