Harden state transition errors

src/state_machine.rs

@@ -28,20 +28,42 @@ pub enum CommitmentAction {
     RecoverExplicitly,
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum TransitionError {
     PolicyRejected,
-    IllegalRuntimeTransition,
+    IllegalRuntimeTransition {
-    IllegalCommitmentTransition,
+        current: RuntimeState,
+        action: RuntimeAction,
+    },
+    IllegalCommitmentTransition {
+        current: CommitmentState,
+        action: CommitmentAction,
+    },
+    InvalidAdminOverrideExitTarget {
+        target: RuntimeState,
+    },
 }
 
 impl std::fmt::Display for TransitionError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             TransitionError::PolicyRejected => write!(f, "policy was rejected"),
-            TransitionError::IllegalRuntimeTransition => write!(f, "illegal runtime transition"),
+            TransitionError::IllegalRuntimeTransition { current, action } => {
-            TransitionError::IllegalCommitmentTransition => {
+                write!(
-                write!(f, "illegal commitment transition")
+                    f,
+                    "illegal runtime transition from {:?} via {:?}",
+                    current, action
+                )
+            }
+            TransitionError::IllegalCommitmentTransition { current, action } => {
+                write!(
+                    f,
+                    "illegal commitment transition from {:?} via {:?}",
+                    current, action
+                )
+            }
+            TransitionError::InvalidAdminOverrideExitTarget { target } => {
+                write!(f, "invalid admin override exit target {:?}", target)
             }
         }
     }
@@ -49,6 +71,12 @@ impl std::fmt::Display for TransitionError {
 
 impl std::error::Error for TransitionError {}
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct CompositeTransition {
+    pub runtime_state: RuntimeState,
+    pub commitment_state: CommitmentState,
+}
+
 pub fn transition_runtime(
     current: RuntimeState,
     action: RuntimeAction,
@@ -58,8 +86,18 @@ pub fn transition_runtime(
 
     match (current, action) {
         (Locked, EnterPlanning) => Ok(Planning),
-        (Planning, Activate { policy_accepted: true }) => Ok(Active),
+        (
-        (Planning, Activate { policy_accepted: false }) => Err(TransitionError::PolicyRejected),
+            Planning,
+            Activate {
+                policy_accepted: true,
+            },
+        ) => Ok(Active),
+        (
+            Planning,
+            Activate {
+                policy_accepted: false,
+            },
+        ) => Err(TransitionError::PolicyRejected),
         (Planning, CancelOrTimeout) => Ok(Locked),
         (Active, StartTransition) => Ok(Transition),
         (Active, CompleteForReview) => Ok(Review),
@@ -71,8 +109,13 @@ pub fn transition_runtime(
         (Review, ContinuePlanning) => Ok(Planning),
         (Review, EndWorkPeriod) => Ok(Locked),
         (_, EnterAdminOverride) => Ok(AdminOverride),
+        (AdminOverride, ExitAdminOverride(AdminOverride)) => {
+            Err(TransitionError::InvalidAdminOverrideExitTarget {
+                target: AdminOverride,
+            })
+        }
         (AdminOverride, ExitAdminOverride(next)) => Ok(next),
-        _ => Err(TransitionError::IllegalRuntimeTransition),
+        _ => Err(TransitionError::IllegalRuntimeTransition { current, action }),
     }
 }
 
@@ -83,7 +126,7 @@ pub fn transition_commitment(
     use CommitmentAction::*;
     use CommitmentState::*;
 
-    match (current, action) {
+    match (current.clone(), action) {
         (Draft, Activate) => Ok(Active),
         (Active, PauseForTransition) => Ok(Paused),
         (Paused, ReturnFromPause) => Ok(Active),
@@ -93,10 +136,24 @@ pub fn transition_commitment(
         (Paused, Abandon) => Ok(Abandoned),
         (Violated, RecoverExplicitly) => Ok(Active),
         (Violated, Abandon) => Ok(Abandoned),
-        _ => Err(TransitionError::IllegalCommitmentTransition),
+        _ => Err(TransitionError::IllegalCommitmentTransition { current, action }),
     }
 }
 
+pub fn start_transition(
+    runtime_state: RuntimeState,
+    commitment_state: CommitmentState,
+) -> Result<CompositeTransition, TransitionError> {
+    let next_runtime = transition_runtime(runtime_state, RuntimeAction::StartTransition)?;
+    let next_commitment =
+        transition_commitment(commitment_state, CommitmentAction::PauseForTransition)?;
+
+    Ok(CompositeTransition {
+        runtime_state: next_runtime,
+        commitment_state: next_commitment,
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -141,14 +198,42 @@ mod tests {
 
     #[test]
     fn locked_cannot_directly_become_active() {
+        let err = transition_runtime(
+            RuntimeState::Locked,
+            RuntimeAction::Activate {
+                policy_accepted: true,
+            },
+        )
+        .unwrap_err();
+
         assert_eq!(
-            transition_runtime(
+            err,
-                RuntimeState::Locked,
+            TransitionError::IllegalRuntimeTransition {
-                RuntimeAction::Activate {
+                current: RuntimeState::Locked,
+                action: RuntimeAction::Activate {
                     policy_accepted: true,
-                }
+                },
-            ),
+            }
-            Err(TransitionError::IllegalRuntimeTransition)
+        );
+        assert_eq!(
+            err.to_string(),
+            "illegal runtime transition from Locked via Activate { policy_accepted: true }"
+        );
+    }
+
+    #[test]
+    fn admin_override_cannot_exit_to_admin_override() {
+        let err = transition_runtime(
+            RuntimeState::AdminOverride,
+            RuntimeAction::ExitAdminOverride(RuntimeState::AdminOverride),
+        )
+        .unwrap_err();
+
+        assert_eq!(
+            err,
+            TransitionError::InvalidAdminOverrideExitTarget {
+                target: RuntimeState::AdminOverride,
+            }
         );
     }
 
@@ -161,9 +246,49 @@ mod tests {
             ),
             Ok(CommitmentState::Active)
         );
+        let err =
+            transition_commitment(CommitmentState::Violated, CommitmentAction::ReturnFromPause)
+                .unwrap_err();
+
         assert_eq!(
-            transition_commitment(CommitmentState::Violated, CommitmentAction::ReturnFromPause),
+            err,
-            Err(TransitionError::IllegalCommitmentTransition)
+            TransitionError::IllegalCommitmentTransition {
+                current: CommitmentState::Violated,
+                action: CommitmentAction::ReturnFromPause,
+            }
+        );
+        assert_eq!(
+            err.to_string(),
+            "illegal commitment transition from Violated via ReturnFromPause"
+        );
+    }
+
+    #[test]
+    fn start_transition_moves_runtime_and_commitment_together() {
+        assert_eq!(
+            start_transition(RuntimeState::Active, CommitmentState::Active),
+            Ok(CompositeTransition {
+                runtime_state: RuntimeState::Transition,
+                commitment_state: CommitmentState::Paused,
+            })
+        );
+    }
+
+    #[test]
+    fn start_transition_fails_before_returning_partial_state() {
+        assert_eq!(
+            start_transition(RuntimeState::Active, CommitmentState::Draft),
+            Err(TransitionError::IllegalCommitmentTransition {
+                current: CommitmentState::Draft,
+                action: CommitmentAction::PauseForTransition,
+            })
+        );
+        assert_eq!(
+            start_transition(RuntimeState::Locked, CommitmentState::Active),
+            Err(TransitionError::IllegalRuntimeTransition {
+                current: RuntimeState::Locked,
+                action: RuntimeAction::StartTransition,
+            })
         );
     }
 }