cleanup

crates/rules/src/rules/has_rule_set_for_role.rs

@@ -11,16 +11,77 @@ pub enum FactorRulesFailure<E: std::fmt::Debug> {
 pub enum FactorRulesFailureBasic {
     #[error("Threshold may not be larger than the number of factors in the threshold list")]
     ThresholdTooLarge,
+
+    #[error("Threshold factors not allowed")]
+    ThresholdFactorsNotAllowed,
+}
+
+pub trait IntoRulesViolation<V: std::fmt::Debug> {
+    fn into_rules_violation(self) -> std::result::Result<(), FactorRulesFailure<V>>;
+}
+
+impl<V: std::fmt::Debug> IntoRulesViolation<V> for FactorBuilderResult<V> {
+    fn into_rules_violation(self) -> std::result::Result<(), FactorRulesFailure<V>> {
+        match self {
+            Ok(()) => Ok(()),
+            Err(FactorRulesViolation::NotYetValid(e)) => Err(FactorRulesFailure::Specific(e)),
+            Err(FactorRulesViolation::ForeverInvalid(e)) => Err(FactorRulesFailure::Specific(e)),
+        }
+    }
+}
+
+pub trait IntoRulesFailure<V: std::fmt::Debug> {
+    fn into_rules_failure(
+        self,
+    ) -> std::result::Result<(), FactorRulesFailure<FactorRulesViolation<V>>>;
+}
+
+impl<V: std::fmt::Debug> IntoRulesFailure<V> for FactorBuilderResult<V> {
+    fn into_rules_failure(
+        self,
+    ) -> std::result::Result<(), FactorRulesFailure<FactorRulesViolation<V>>> {
+        self.map_err(FactorRulesFailure::Specific)
+    }
+}
+
+pub trait BasicFailure {
+    fn basic_failure(failure: FactorRulesFailureBasic) -> Self;
+}
+impl<V: std::fmt::Debug> BasicFailure
+    for std::result::Result<(), FactorRulesFailure<FactorRulesViolation<V>>>
+{
+    fn basic_failure(failure: FactorRulesFailureBasic) -> Self {
+        Self::Err(FactorRulesFailure::Basic(failure))
+    }
+}
+
+impl<N: std::fmt::Debug> CanBeNotYetValid<N>
+    for std::result::Result<(), FactorRulesFailure<FactorRulesViolation<N>>>
+{
+    fn not_yet_valid(violation: N) -> Self {
+        Self::Err(FactorRulesFailure::Specific(
+            FactorRulesViolation::NotYetValid(violation),
+        ))
+    }
+}
+impl<F: std::fmt::Debug> CanBeForeverInvalid<F>
+    for std::result::Result<(), FactorRulesFailure<FactorRulesViolation<F>>>
+{
+    fn forever_invalid(violation: F) -> Self {
+        Self::Err(FactorRulesFailure::Specific(
+            FactorRulesViolation::ForeverInvalid(violation),
+        ))
+    }
 }
 
 pub trait HasRuleSetForRole<F: IsFactor>: Sized + IsRole + Clone {
     type Violation: std::fmt::Debug;
-    type Failure = FactorRulesFailure<Self::Violation>;
-    type Result = std::result::Result<(), Self::Failure>;
 
     /// Neither Recovery nor Confirmation roles can have factors added to their threshold
     /// factor list. Only Primary supports it.
-    fn violation_if_adding_factors_to_threshold() -> Option<Self::Violation>;
+    fn supports_threshold_factors() -> bool {
+        false
+    }
 
     fn validate_device_in_factors_list_of_kind(
         context: &RoleWithFactorsBuilt<F, Self>,
@@ -45,7 +106,7 @@ pub trait HasRuleSetForRole<F: IsFactor>: Sized + IsRole + Clone {
         ) -> FactorBuilderResult<Self::Violation>,
     ) -> FactorBuilderResult<Self::Violation> {
         validation(context, FactorListKind::Override)?;
-        if Self::violation_if_adding_factors_to_threshold().is_none() {
+        if Self::supports_threshold_factors() {
             validation(context, FactorListKind::Threshold)?;
         }
         Ok(())
@@ -69,24 +130,21 @@ pub trait HasRuleSetForRole<F: IsFactor>: Sized + IsRole + Clone {
         Self::_do_validate(context, Self::validate_password_in_factors_list_of_kind)
     }
 
-    fn validate(context: &RoleWithFactorsBuilt<F, Self>) -> Self::Result {
+    fn validate(
+        context: &RoleWithFactorsBuilt<F, Self>,
+    ) -> std::result::Result<(), FactorRulesFailure<FactorRulesViolation<Self::Violation>>> {
         if context.threshold > context.threshold_factors.len() as u8 {
-            // return FactorBuilderResult::not_yet_valid(
-            //     Self::violation_if_adding_factors_to_threshold().unwrap(),
-            // );
-            // return Self::Result::Err(Self::Failure::Basic(
-            //     FactorRulesFailureBasic::ThresholdTooLarge,
-            // ));
-            let basic: FactorBuilderResult<>
-            let failure: Self::Failure =
-                Self::Failure::Basic(FactorRulesFailureBasic::ThresholdTooLarge);
-            let res: Self::Result = Self::Result::Err(failure);
-            return res;
+            return std::result::Result::<
+                (),
+                FactorRulesFailure<FactorRulesViolation<Self::Violation>>,
+            >::Err(FactorRulesFailure::Basic(
+                FactorRulesFailureBasic::ThresholdTooLarge,
+            ));
         }
 
-        // Self::validate_device(context)?;
-        // Self::validate_ledger(context)?;
-        // Self::validate_password(context)?;
-        Self::Result::Ok(())
+        Self::validate_device(context).into_rules_failure()?;
+        Self::validate_ledger(context).into_rules_failure()?;
+        Self::validate_password(context).into_rules_failure()?;
+        Ok(())
     }
 }

crates/rules/src/rules/mod.rs

@@ -1,4 +1,3 @@
-mod error;
 mod factor_rules_violation;
 mod has_rule_set_for_role;
 mod is_role;
@@ -9,12 +8,10 @@ mod role_kind;
 mod role_tags;
 mod sargon_types;
 
-pub use error::*;
 pub use factor_rules_violation::*;
 pub use has_rule_set_for_role::*;
 pub use is_role::*;
 // pub use matrix::*;
-
 #[allow(unused_imports)]
 pub use primary_role_with_factor_sources_builder::*;
 pub use role_builder_or_built::*;

crates/rules/src/rules/primary_role_with_factor_sources_builder.rs

@@ -13,8 +13,8 @@ impl IsRole for PrimaryRoleWithFactorSourceRuleSet {
 impl HasRuleSetForRole<FactorSource> for PrimaryRoleWithFactorSourceRuleSet {
     type Violation = PrimaryRoleViolation;
 
-    fn violation_if_adding_factors_to_threshold() -> Option<Self::Violation> {
-        None
+    fn supports_threshold_factors() -> bool {
+        true
     }
 
     fn validate_device_in_factors_list_of_kind(
@@ -73,46 +73,75 @@ pub type PrimaryRoleWithFactorSourcesBuilder =
 mod tests {
     use super::*;
 
-    #[test]
-    fn single_device_in_threshold_is_ok() {
-        type F = FactorSource;
-        type SUT = PrimaryRoleWithFactorSourcesBuilder;
-        let mut sut = SUT::new();
+    type F = FactorSource;
+    type SUT = PrimaryRoleWithFactorSourcesBuilder;
+    type MutResult = SUT::MutateResult;
+    type E = PrimaryRoleViolation;
 
-        let f = F::sample_device();
-        sut.add_to_threshold(&f).unwrap();
-        let built = sut.build().unwrap();
-        assert_eq!(built.threshold_factors, vec![f])
+    fn forever_invalid(violation: E) -> MutResult {
+        MutResult::forever_invalid(violation)
     }
 
-    #[test]
-    fn threshold_cannot_be_larger_than_threshold_factor_count() {
-        type F = FactorSource;
-        type SUT = PrimaryRoleWithFactorSourcesBuilder;
-        let mut sut = SUT::new();
-
-        let f = F::sample_device();
-        sut.add_to_threshold(&f).unwrap();
-        sut.set_threshold(2);
-        let built = sut.build().unwrap();
-        assert_eq!(built.threshold_factors, vec![f])
+    fn basic_failure(e: FactorRulesFailureBasic) -> MutResult {
+        MutResult::basic_failure(e)
     }
 
+    #[cfg(test)]
+    mod threshold_tests {
+        use super::*;
 
-    #[test]
-    fn add_password_to_override_then_build() {
-        type F = FactorSource;
-        type SUT = PrimaryRoleWithFactorSourcesBuilder;
-        type MutResult = SUT::MutateResult;
+        fn test(threshold: u8, arrange: impl Fn(&mut SUT)) {
+            let mut sut = SUT::new();
+            arrange(&mut sut);
+            assert_eq!(
+                sut.set_threshold(threshold),
+                basic_failure(FactorRulesFailureBasic::ThresholdTooLarge)
+            )
+        }
 
-        let mut sut = SUT::new();
+        #[test]
+        fn threshold_cannot_be_larger_than_threshold_factor_count_1_gt_0() {
+            test(1, |_| {});
+        }
 
-        let f = F::sample_password();
-        assert_eq!(
-            sut.add_to_override(&f),
-            MutResult::forever_invalid(
-                PrimaryRoleViolation::PasswordNotAllowedInOverrideListSinceItWouldBeAlone
-            )
-        );
+        #[test]
+        fn threshold_cannot_be_larger_than_threshold_factor_count_2_gt_1() {
+            test(2, |sut| {
+                sut.add_to_threshold(&F::sample_device()).unwrap();
+            })
+        }
+
+        #[test]
+        fn threshold_cannot_be_larger_than_threshold_factor_count_9_gt_2() {
+            test(9, |sut| {
+                sut.add_to_threshold(&F::sample_device()).unwrap();
+                sut.add_to_threshold(&F::sample_ledger()).unwrap();
+            })
+        }
+    }
+
+    #[cfg(test)]
+    mod general {
+        use super::*;
+
+        #[test]
+        fn single_device_in_threshold_is_ok() {
+            let mut sut = SUT::new();
+
+            let f = F::sample_device();
+            sut.add_to_threshold(&f).unwrap();
+            let built = sut.build().unwrap();
+            assert_eq!(built.threshold_factors, vec![f])
+        }
+
+        #[test]
+        fn add_password_to_override_then_build() {
+            let mut sut = SUT::new();
+            let f = F::sample_password();
+            assert_eq!(
+                sut.add_to_override(&f),
+                forever_invalid(E::PasswordNotAllowedInOverrideListSinceItWouldBeAlone)
+            );
+        }
     }
 }

crates/rules/src/rules/role_builder_or_built.rs

@@ -32,8 +32,10 @@ where
     }
 
     pub type Built = RoleWithFactorsBuilt<F, R>;
-    pub type MutateResult = FactorBuilderResult<R::Violation>;
-    pub type BuildResult = Result<Self::Built, R::Violation>;
+    pub type MutateResult =
+        std::result::Result<(), FactorRulesFailure<FactorRulesViolation<R::Violation>>>;
+    pub type BuildResult =
+        std::result::Result<Self::Built, FactorRulesFailure<FactorRulesViolation<R::Violation>>>;
 
     pub fn build(self) -> Self::BuildResult {
         self.validate()?;
@@ -56,24 +58,36 @@ where
         self.add_to_list(factor, FactorListKind::Threshold)
     }
 
+    fn assert_supports_threshold(&self) -> Self::MutateResult {
+        if !R::supports_threshold_factors() {
+            return Self::MutateResult::Err(
+                FactorRulesFailure::<FactorRulesViolation<R::Violation>>::Basic(
+                    FactorRulesFailureBasic::ThresholdFactorsNotAllowed,
+                ),
+            );
+        }
+        Ok(())
+    }
+
     pub fn set_threshold(&mut self, threshold: u8) -> Self::MutateResult {
+        self.assert_supports_threshold()?;
+
         let mut simulation = self.clone();
         simulation.threshold = threshold;
         simulation.validate()
     }
 
     fn validate(&self) -> Self::MutateResult {
-        // R::validate(&self.snapshot())
-        todo!()
+        R::validate(&self.snapshot())
     }
 
     fn violation_if_add_factor_to_list_of_kind(
         &self,
         factor: &F,
         list_kind: FactorListKind,
     ) -> Self::MutateResult {
-        if let Some(violation) = R::violation_if_adding_factors_to_threshold() {
-            return Self::MutateResult::Err(FactorRulesViolation::ForeverInvalid(violation));
+        if list_kind == FactorListKind::Threshold {
+            self.assert_supports_threshold()?;
         }
 
         let factor = factor.clone();
@@ -94,8 +108,15 @@ where
         let factor = factor.borrow();
         let outcome = self.violation_if_add_factor_to_list_of_kind(factor, list_kind);
         match outcome.as_ref() {
-            Err(FactorRulesViolation::ForeverInvalid(_)) => return outcome,
-            Err(FactorRulesViolation::NotYetValid(_)) => {
+            Err(FactorRulesFailure::<FactorRulesViolation<R::Violation>>::Basic(_)) => {
+                return outcome
+            }
+            Err(FactorRulesFailure::<FactorRulesViolation<R::Violation>>::Specific(
+                FactorRulesViolation::ForeverInvalid(_),
+            )) => return outcome,
+            Err(FactorRulesFailure::<FactorRulesViolation<R::Violation>>::Specific(
+                FactorRulesViolation::NotYetValid(_),
+            )) => {
                 // lets try to make it valid.
             }
             Ok(_) => {}