cleanup and document slots better

Author: samuelcolvin

src/validators/mod.rs

@@ -67,7 +67,6 @@ impl SchemaValidator {
 
         let mut build_context = BuildContext::default();
         let (mut validator, _) = build_validator(schema, config, &mut build_context)?;
-        build_context.complete_validators()?;
         validator.complete(&build_context)?;
         let slots = build_context.into_slots()?;
         let title = validator.get_name().into_py(py);
@@ -459,65 +458,68 @@ pub trait Validator: Send + Sync + Clone + Debug {
     }
 }
 
+/// `BuildContext` is used to store extra information while building validators,
+/// currently it just holds a vec "slots" which holds validators need to be accessed from multiple other validators
+/// and therefore can't be owned by them directly.
 #[derive(Default, Clone)]
 pub struct BuildContext {
     slots: Vec<(String, Option<CombinedValidator>)>,
 }
 
 impl BuildContext {
+    /// First of two part process to add a new validator slot, we add the `slot_ref` to the array, but not the
+    /// actual `validator`, we can't add the validator until it's build.
+    /// We need the `id` to build the validator, hence this two-step process.
     pub fn prepare_slot(&mut self, slot_ref: String) -> PyResult<usize> {
         let id = self.slots.len();
         self.slots.push((slot_ref, None));
         Ok(id)
     }
 
+    /// Second part of adding a validator - we update the slot to include a validator
     pub fn complete_slot(&mut self, slot_id: usize, validator: CombinedValidator) -> PyResult<()> {
         match self.slots.get(slot_id) {
             Some((val_ref, _)) => {
                 self.slots[slot_id] = (val_ref.clone(), Some(validator));
                 Ok(())
             }
-            None => py_error!("Recursive reference error: slot {} not found", slot_id),
+            None => py_error!("Slots Error: slot {} not found", slot_id),
         }
     }
 
-    pub fn find_slot_id(&self, val_ref: &str) -> PyResult<usize> {
-        let is_match = |(slot_ref, _): &(String, Option<CombinedValidator>)| slot_ref == val_ref;
+    /// find a slot by `slot_ref` - iterate over the slots until we find a matching reference - return the index
+    pub fn find_slot_id(&self, slot_ref: &str) -> PyResult<usize> {
+        let is_match = |(match_sr, _): &(String, Option<CombinedValidator>)| match_sr == slot_ref;
         match self.slots.iter().position(is_match) {
             Some(id) => Ok(id),
-            None => py_error!("Recursive reference error: ref '{}' not found", val_ref),
+            None => py_error!("Slots Error: ref '{}' not found", slot_ref),
         }
     }
 
-    pub fn get_validator(&self, slot_id: usize) -> PyResult<&CombinedValidator> {
+    /// find a validator by `slot_id` - this used in `Validator.complete`, specifically `RecursiveRefValidator`
+    /// to set its name
+    pub fn find_validator(&self, slot_id: usize) -> PyResult<&CombinedValidator> {
         match self.slots.get(slot_id) {
             Some((_, op_validator)) => match op_validator {
                 Some(ref validator) => Ok(validator),
-                None => py_error!("Recursive reference error: slot {} not yet filled", slot_id),
+                None => py_error!("Slots Error: slot {} not yet filled", slot_id),
             },
-            None => py_error!("Recursive reference error: slot {} not found", slot_id),
+            None => py_error!("Slots Error: slot {} not found", slot_id),
         }
     }
 
-    pub fn complete_validators(&mut self) -> PyResult<()> {
-        let self_clone = self.clone();
-        for (_, op_validator) in self.slots.iter_mut() {
-            match op_validator {
-                Some(ref mut validator) => {
-                    validator.complete(&self_clone)?;
-                }
-                None => return py_error!("Recursive reference error: slot not yet filled"),
-            }
-        }
-        Ok(())
-    }
-
+    /// Move validators into a new vec which maintains the order of slots, `complete` is called on each validator
+    /// at the same time.
     pub fn into_slots(self) -> PyResult<Vec<CombinedValidator>> {
+        let self_clone = self.clone();
         self.slots
             .into_iter()
             .map(|(_, opt_validator)| match opt_validator {
-                Some(validator) => Ok(validator),
-                None => py_error!("Recursive schema build error: slot not yet filled"),
+                Some(mut validator) => {
+                    validator.complete(&self_clone)?;
+                    Ok(validator)
+                }
+                None => py_error!("Slots Error: slot not yet filled"),
             })
             .collect()
     }

src/validators/recursive.rs

@@ -88,7 +88,7 @@ impl Validator for RecursiveRefValidator {
 
     /// don't need to call complete on the inner validator here, complete_validators takes care of that.
     fn complete(&mut self, build_context: &BuildContext) -> PyResult<()> {
-        let validator = build_context.get_validator(self.validator_id)?;
+        let validator = build_context.find_validator(self.validator_id)?;
         self.inner_name = validator.get_name().to_string();
         Ok(())
     }

tests/validators/test_recursive.py

@@ -200,7 +200,7 @@ class Branch:
 
 
 def test_invalid_schema():
-    with pytest.raises(SchemaError, match="Recursive reference error: ref 'Branch' not found"):
+    with pytest.raises(SchemaError, match="Slots Error: ref 'Branch' not found"):
         SchemaValidator(
             {
                 'type': 'list',