Skip to content

Commit 6e4cf58

Browse files
committed
perf: Fuse required with properties validator
Signed-off-by: Dmitry Dygalo <dmitry@dygalo.dev>
1 parent a741634 commit 6e4cf58

File tree

2 files changed

+288
-10
lines changed

2 files changed

+288
-10
lines changed

crates/jsonschema/src/keywords/properties.rs

Lines changed: 273 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ pub(crate) struct BigPropertiesValidator {
2424
pub(crate) properties: AHashMap<String, SchemaNode>,
2525
}
2626

27+
/// Fused validator for `properties` + `required: [2 items]` (no `additionalProperties: false`).
28+
/// Eliminates separate required validation pass and duplicate `BTreeMap` lookups.
29+
pub(crate) struct SmallPropertiesWithRequired2Validator {
30+
pub(crate) properties: Vec<(String, SchemaNode)>,
31+
first: String,
32+
second: String,
33+
required_location: Location,
34+
}
35+
2736
impl SmallPropertiesValidator {
2837
#[inline]
2938
pub(crate) fn compile<'a>(
@@ -62,6 +71,33 @@ impl BigPropertiesValidator {
6271
}
6372
}
6473

74+
impl SmallPropertiesWithRequired2Validator {
75+
#[inline]
76+
pub(crate) fn compile<'a>(
77+
ctx: &compiler::Context,
78+
map: &'a Map<String, Value>,
79+
first: String,
80+
second: String,
81+
) -> CompilationResult<'a> {
82+
let pctx = ctx.new_at_location("properties");
83+
let mut properties = Vec::with_capacity(map.len());
84+
for (key, subschema) in map {
85+
let kctx = pctx.new_at_location(key.as_str());
86+
properties.push((
87+
key.clone(),
88+
compiler::compile(&kctx, kctx.as_resource_ref(subschema))?,
89+
));
90+
}
91+
let required_location = ctx.location().join("required");
92+
Ok(Box::new(SmallPropertiesWithRequired2Validator {
93+
properties,
94+
first,
95+
second,
96+
required_location,
97+
}))
98+
}
99+
}
100+
65101
impl Validate for SmallPropertiesValidator {
66102
fn is_valid(&self, instance: &Value, ctx: &mut ValidationContext) -> bool {
67103
if let Value::Object(item) = instance {
@@ -143,6 +179,139 @@ impl Validate for SmallPropertiesValidator {
143179
}
144180
}
145181

182+
impl Validate for SmallPropertiesWithRequired2Validator {
183+
fn is_valid(&self, instance: &Value, ctx: &mut ValidationContext) -> bool {
184+
if let Value::Object(item) = instance {
185+
// Check required first (fast fail)
186+
if item.len() < 2 || !item.contains_key(&self.first) || !item.contains_key(&self.second)
187+
{
188+
return false;
189+
}
190+
// Validate properties
191+
for (name, node) in &self.properties {
192+
if let Some(prop) = item.get(name) {
193+
if !node.is_valid(prop, ctx) {
194+
return false;
195+
}
196+
}
197+
}
198+
true
199+
} else {
200+
true
201+
}
202+
}
203+
204+
fn validate<'i>(
205+
&self,
206+
instance: &'i Value,
207+
location: &LazyLocation,
208+
tracker: Option<&RefTracker>,
209+
ctx: &mut ValidationContext,
210+
) -> Result<(), ValidationError<'i>> {
211+
if let Value::Object(item) = instance {
212+
// Check required first
213+
if !item.contains_key(&self.first) {
214+
return Err(ValidationError::required(
215+
self.required_location.clone(),
216+
crate::paths::capture_evaluation_path(tracker, &self.required_location),
217+
location.into(),
218+
instance,
219+
Value::String(self.first.clone()),
220+
));
221+
}
222+
if !item.contains_key(&self.second) {
223+
return Err(ValidationError::required(
224+
self.required_location.clone(),
225+
crate::paths::capture_evaluation_path(tracker, &self.required_location),
226+
location.into(),
227+
instance,
228+
Value::String(self.second.clone()),
229+
));
230+
}
231+
// Validate properties
232+
for (name, node) in &self.properties {
233+
if let Some(prop) = item.get(name) {
234+
node.validate(prop, &location.push(name), tracker, ctx)?;
235+
}
236+
}
237+
}
238+
Ok(())
239+
}
240+
241+
#[allow(clippy::needless_collect)]
242+
fn iter_errors<'i>(
243+
&self,
244+
instance: &'i Value,
245+
location: &LazyLocation,
246+
tracker: Option<&RefTracker>,
247+
ctx: &mut ValidationContext,
248+
) -> ErrorIterator<'i> {
249+
if let Value::Object(item) = instance {
250+
let mut errors = Vec::new();
251+
// Check required
252+
let eval_path = crate::paths::capture_evaluation_path(tracker, &self.required_location);
253+
if !item.contains_key(&self.first) {
254+
errors.push(ValidationError::required(
255+
self.required_location.clone(),
256+
eval_path.clone(),
257+
location.into(),
258+
instance,
259+
Value::String(self.first.clone()),
260+
));
261+
}
262+
if !item.contains_key(&self.second) {
263+
errors.push(ValidationError::required(
264+
self.required_location.clone(),
265+
eval_path,
266+
location.into(),
267+
instance,
268+
Value::String(self.second.clone()),
269+
));
270+
}
271+
// Validate properties
272+
for (name, node) in &self.properties {
273+
if let Some(prop) = item.get(name) {
274+
let instance_path = location.push(name.as_str());
275+
errors.extend(node.iter_errors(prop, &instance_path, tracker, ctx));
276+
}
277+
}
278+
if !errors.is_empty() {
279+
return ErrorIterator::from_iterator(errors.into_iter());
280+
}
281+
}
282+
no_error()
283+
}
284+
285+
fn evaluate(
286+
&self,
287+
instance: &Value,
288+
location: &LazyLocation,
289+
tracker: Option<&RefTracker>,
290+
ctx: &mut ValidationContext,
291+
) -> EvaluationResult {
292+
if let Value::Object(props) = instance {
293+
// Check required first
294+
if !props.contains_key(&self.first) || !props.contains_key(&self.second) {
295+
return EvaluationResult::invalid_empty(Vec::new());
296+
}
297+
let mut matched_props = Vec::with_capacity(props.len());
298+
let mut children = Vec::new();
299+
for (prop_name, node) in &self.properties {
300+
if let Some(prop) = props.get(prop_name) {
301+
let path = location.push(prop_name.as_str());
302+
matched_props.push(prop_name.clone());
303+
children.push(node.evaluate_instance(prop, &path, tracker, ctx));
304+
}
305+
}
306+
let mut application = EvaluationResult::from_children(children);
307+
application.annotate(Annotations::new(Value::from(matched_props)));
308+
application
309+
} else {
310+
EvaluationResult::valid_empty()
311+
}
312+
}
313+
}
314+
146315
impl Validate for BigPropertiesValidator {
147316
fn is_valid(&self, instance: &Value, ctx: &mut ValidationContext) -> bool {
148317
if let Value::Object(item) = instance {
@@ -225,6 +394,25 @@ impl Validate for BigPropertiesValidator {
225394
}
226395
}
227396

397+
/// Check if we can use fused properties+required validator.
398+
/// Conditions: properties < threshold, required: [2 strings], no patternProperties.
399+
fn extract_required2(parent: &Map<String, Value>) -> Option<(String, String)> {
400+
// No patternProperties (uses separate validator paths)
401+
if parent.contains_key("patternProperties") {
402+
return None;
403+
}
404+
if let Some(Value::Array(items)) = parent.get("required") {
405+
if items.len() == 2 {
406+
if let (Some(Value::String(first)), Some(Value::String(second))) =
407+
(items.first(), items.get(1))
408+
{
409+
return Some((first.clone(), second.clone()));
410+
}
411+
}
412+
}
413+
None
414+
}
415+
228416
#[inline]
229417
pub(crate) fn compile<'a>(
230418
ctx: &compiler::Context,
@@ -237,7 +425,14 @@ pub(crate) fn compile<'a>(
237425
_ => {
238426
if let Value::Object(map) = schema {
239427
if map.len() < HASHMAP_THRESHOLD {
240-
Some(SmallPropertiesValidator::compile(ctx, map))
428+
// Try fused validator for properties + required: [2 items]
429+
if let Some((first, second)) = extract_required2(parent) {
430+
Some(SmallPropertiesWithRequired2Validator::compile(
431+
ctx, map, first, second,
432+
))
433+
} else {
434+
Some(SmallPropertiesValidator::compile(ctx, map))
435+
}
241436
} else {
242437
Some(BigPropertiesValidator::compile(ctx, map))
243438
}
@@ -258,7 +453,8 @@ pub(crate) fn compile<'a>(
258453
#[cfg(test)]
259454
mod tests {
260455
use crate::tests_util;
261-
use serde_json::json;
456+
use serde_json::{json, Value};
457+
use test_case::test_case;
262458

263459
#[test]
264460
fn location() {
@@ -268,4 +464,79 @@ mod tests {
268464
"/properties/foo/properties/bar/required",
269465
);
270466
}
467+
468+
// SmallPropertiesWithRequired2Validator tests
469+
fn fused_schema() -> Value {
470+
// No additionalProperties: false, so uses SmallPropertiesWithRequired2Validator
471+
json!({
472+
"properties": {
473+
"a": {"type": "integer"},
474+
"b": {"type": "string"}
475+
},
476+
"required": ["a", "b"]
477+
})
478+
}
479+
480+
#[test_case(&json!({"a": 1, "b": "x"}), true)]
481+
#[test_case(&json!({"a": 1, "b": "x", "c": 3}), true)]
482+
#[test_case(&json!({"a": 1}), false)] // missing b
483+
#[test_case(&json!({"b": "x"}), false)] // missing a
484+
#[test_case(&json!({}), false)]
485+
#[test_case(&json!("string"), true)] // non-object passes
486+
fn fused_properties_required2_is_valid(instance: &Value, expected: bool) {
487+
let validator = crate::validator_for(&fused_schema()).unwrap();
488+
assert_eq!(validator.is_valid(instance), expected);
489+
}
490+
491+
#[test]
492+
fn fused_properties_required2_validate_missing_first() {
493+
let validator = crate::validator_for(&fused_schema()).unwrap();
494+
let instance = json!({"b": "x"});
495+
let result = validator.validate(&instance);
496+
assert!(result.is_err());
497+
let err = result.unwrap_err();
498+
assert!(err.to_string().contains("required"));
499+
}
500+
501+
#[test]
502+
fn fused_properties_required2_validate_missing_second() {
503+
let validator = crate::validator_for(&fused_schema()).unwrap();
504+
let instance = json!({"a": 1});
505+
let result = validator.validate(&instance);
506+
assert!(result.is_err());
507+
let err = result.unwrap_err();
508+
assert!(err.to_string().contains("required"));
509+
}
510+
511+
#[test]
512+
fn fused_properties_required2_iter_errors_missing_both() {
513+
let validator = crate::validator_for(&fused_schema()).unwrap();
514+
let instance = json!({});
515+
let errors: Vec<_> = validator.iter_errors(&instance).collect();
516+
assert_eq!(errors.len(), 2);
517+
}
518+
519+
#[test]
520+
fn fused_properties_required2_iter_errors_missing_first() {
521+
let validator = crate::validator_for(&fused_schema()).unwrap();
522+
let instance = json!({"b": "x"});
523+
let errors: Vec<_> = validator.iter_errors(&instance).collect();
524+
assert_eq!(errors.len(), 1);
525+
}
526+
527+
#[test]
528+
fn fused_properties_required2_iter_errors_missing_second() {
529+
let validator = crate::validator_for(&fused_schema()).unwrap();
530+
let instance = json!({"a": 1});
531+
let errors: Vec<_> = validator.iter_errors(&instance).collect();
532+
assert_eq!(errors.len(), 1);
533+
}
534+
535+
#[test]
536+
fn fused_properties_required2_iter_errors_valid() {
537+
let validator = crate::validator_for(&fused_schema()).unwrap();
538+
let instance = json!({"a": 1, "b": "x"});
539+
let errors: Vec<_> = validator.iter_errors(&instance).collect();
540+
assert!(errors.is_empty());
541+
}
271542
}

crates/jsonschema/src/keywords/required.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -375,16 +375,23 @@ pub(crate) fn compile<'a>(
375375
parent: &'a Map<String, Value>,
376376
schema: &'a Value,
377377
) -> Option<CompilationResult<'a>> {
378-
// Check if fused validator in additional_properties handles this case:
379-
// properties + additionalProperties: false + required: [single_item]
380-
// Also check there's no patternProperties (which uses different validators)
378+
// Check if fused validators handle this case
381379
if let Value::Array(items) = schema {
382-
if items.len() == 1
383-
&& matches!(parent.get("additionalProperties"), Some(Value::Bool(false)))
384-
&& parent.contains_key("properties")
385-
&& !parent.contains_key("patternProperties")
380+
let has_properties = parent.contains_key("properties");
381+
let has_pattern_properties = parent.contains_key("patternProperties");
382+
let additional_props_false =
383+
matches!(parent.get("additionalProperties"), Some(Value::Bool(false)));
384+
385+
// Case 1: properties + additionalProperties: false + required: [1 item], no patternProperties
386+
// Handled by AdditionalPropertiesNotEmptyFalseWithRequired1Validator
387+
if items.len() == 1 && additional_props_false && has_properties && !has_pattern_properties {
388+
return None;
389+
}
390+
391+
// Case 2: properties + required: [2 items], no additionalProperties: false, no patternProperties
392+
// Handled by SmallPropertiesWithRequired2Validator
393+
if items.len() == 2 && has_properties && !additional_props_false && !has_pattern_properties
386394
{
387-
// Fused validator handles this - skip separate required validation
388395
return None;
389396
}
390397
}

0 commit comments

Comments
 (0)