Skip to content

Commit a47bed9

Browse files
committed
Do not call default factories taking the data argument if a validation error already occurred
1 parent fdccecd commit a47bed9

File tree

6 files changed

+56
-7
lines changed

6 files changed

+56
-7
lines changed

src/errors/types.rs

+4
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,9 @@ error_types! {
196196
class_name: {ctx_type: String, ctx_fn: field_from_context},
197197
},
198198
// ---------------------
199+
// Default factory not called (happens when there's already an error and the factory takes data)
200+
DefaultFactoryNotCalled {},
201+
// ---------------------
199202
// None errors
200203
NoneRequired {},
201204
// ---------------------
@@ -490,6 +493,7 @@ impl ErrorType {
490493
Self::ModelAttributesType {..} => "Input should be a valid dictionary or object to extract fields from",
491494
Self::DataclassType {..} => "Input should be a dictionary or an instance of {class_name}",
492495
Self::DataclassExactType {..} => "Input should be an instance of {class_name}",
496+
Self::DefaultFactoryNotCalled {..} => "The default factory uses validated data, but at least one validation error occurred",
493497
Self::NoneRequired {..} => "Input should be None",
494498
Self::GreaterThan {..} => "Input should be greater than {gt}",
495499
Self::GreaterThanEqual {..} => "Input should be greater than or equal to {ge}",

src/validators/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ pub fn build_validator(
618618
pub struct Extra<'a, 'py> {
619619
/// Validation mode
620620
pub input_type: InputType,
621-
/// This is used as the `data` kwargs to validator functions
621+
/// This is used as the `data` kwargs to validator functions and default factories (if they accept the argument)
622622
pub data: Option<Bound<'py, PyDict>>,
623623
/// whether we're in strict or lax mode
624624
pub strict: Option<bool>,

src/validators/model_fields.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,18 @@ impl Validator for ModelFieldsValidator {
191191
fields_set_vec.push(field.name_py.clone_ref(py));
192192
fields_set_count += 1;
193193
}
194-
Err(ValError::Omit) => continue,
195-
Err(ValError::LineErrors(line_errors)) => {
196-
for err in line_errors {
197-
errors.push(lookup_path.apply_error_loc(err, self.loc_by_alias, &field.name));
194+
Err(e) => {
195+
state.has_field_error = true;
196+
match e {
197+
ValError::Omit => continue,
198+
ValError::LineErrors(line_errors) => {
199+
for err in line_errors {
200+
errors.push(lookup_path.apply_error_loc(err, self.loc_by_alias, &field.name));
201+
}
202+
}
203+
err => return Err(err),
198204
}
199205
}
200-
Err(err) => return Err(err),
201206
}
202207
continue;
203208
}

src/validators/validation_state.rs

+5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ pub struct ValidationState<'a, 'py> {
2525
pub fields_set_count: Option<usize>,
2626
// True if `allow_partial=true` and we're validating the last element of a sequence or mapping.
2727
pub allow_partial: PartialMode,
28+
// Whether at least one field had a validation error. This is used in the context of structured types
29+
// (models, dataclasses, etc), where we need to know if a validation error occurred before calling
30+
// a default factory that takes the validated data.
31+
pub has_field_error: bool,
2832
// deliberately make Extra readonly
2933
extra: Extra<'a, 'py>,
3034
}
@@ -36,6 +40,7 @@ impl<'a, 'py> ValidationState<'a, 'py> {
3640
exactness: None,
3741
fields_set_count: None,
3842
allow_partial,
43+
has_field_error: false,
3944
extra,
4045
}
4146
}

src/validators/with_default.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use pyo3::PyVisit;
99
use super::{build_validator, BuildValidator, CombinedValidator, DefinitionsBuilder, ValidationState, Validator};
1010
use crate::build_tools::py_schema_err;
1111
use crate::build_tools::schema_or_config_same;
12-
use crate::errors::{LocItem, ValError, ValResult};
12+
use crate::errors::{ErrorTypeDefaults, LocItem, ValError, ValResult};
1313
use crate::input::Input;
1414
use crate::py_gc::PyGcTraverse;
1515
use crate::tools::SchemaDict;
@@ -158,6 +158,11 @@ impl Validator for WithDefaultValidator {
158158
state: &mut ValidationState<'_, 'py>,
159159
) -> ValResult<PyObject> {
160160
if input.as_python().is_some_and(|py_input| py_input.is(&self.undefined)) {
161+
if matches!(self.default, DefaultType::DefaultFactory(_, true)) && state.has_field_error {
162+
// The default factory might use data from fields that failed to validate, and this results
163+
// in an unhelpul error.
164+
return Err(ValError::new(ErrorTypeDefaults::DefaultFactoryNotCalled, input));
165+
}
161166
Ok(self.default_value(py, None::<usize>, state)?.unwrap())
162167
} else {
163168
match self.validator.validate(py, input, state) {

tests/validators/test_with_default.py

+30
Original file line numberDiff line numberDiff line change
@@ -818,3 +818,33 @@ def _raise(ex: Exception) -> None:
818818
v.validate_python(input_value)
819819

820820
assert exc_info.value.errors(include_url=False, include_context=False) == expected
821+
822+
823+
def test_default_factory_not_called_if_existing_error() -> None:
824+
class Test:
825+
def __init__(self, a: int, b: int):
826+
self.a = a
827+
self.b = b
828+
829+
schema = core_schema.model_schema(
830+
cls=Test,
831+
schema=core_schema.model_fields_schema(
832+
computed_fields=[],
833+
fields={
834+
'a': core_schema.model_field(
835+
schema=core_schema.int_schema(),
836+
),
837+
'b': core_schema.model_field(
838+
schema=core_schema.with_default_schema(
839+
schema=core_schema.int_schema(),
840+
default_factory=lambda data: data['a'],
841+
default_factory_takes_data=True,
842+
),
843+
),
844+
},
845+
),
846+
)
847+
848+
v = SchemaValidator(schema)
849+
with pytest.raises(ValidationError):
850+
v.validate_python({'a': 'not_an_int'})

0 commit comments

Comments
 (0)