make terminal transitions idempotent
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
c35eb649243aa9b8580c6b793482f6e470c79a9f- Parents
-
13125f4 - Tree
d1f544f
c35eb64
c35eb649243aa9b8580c6b793482f6e470c79a9f13125f4
d1f544f| Status | File | + | - |
|---|---|---|---|
| M |
garwarp/src/daemon.rs
|
47 | 0 |
| M |
garwarp/src/request.rs
|
27 | 0 |
garwarp/src/daemon.rsmodified@@ -542,6 +542,53 @@ mod tests { | |||
| 542 | assert_eq!(state.requests.state("req-1"), Some(RequestState::Pending)); | 542 | assert_eq!(state.requests.state("req-1"), Some(RequestState::Pending)); |
| 543 | } | 543 | } |
| 544 | 544 | ||
| 545 | + #[test] | ||
| 546 | + fn duplicate_cancel_returns_ack() { | ||
| 547 | + let (mut client, server) = UnixStream::pair().expect("pair should be created"); | ||
| 548 | + client | ||
| 549 | + .write_all(b"transition id=req-1 sender=:1.2 state=cancelled\n") | ||
| 550 | + .expect("transition request should be written"); | ||
| 551 | + | ||
| 552 | + let mut state = DaemonState { | ||
| 553 | + health: HealthStatus::Healthy, | ||
| 554 | + requests: RequestRegistry::new(Duration::from_secs(5)), | ||
| 555 | + running: true, | ||
| 556 | + }; | ||
| 557 | + state | ||
| 558 | + .requests | ||
| 559 | + .begin_at( | ||
| 560 | + "req-1", | ||
| 561 | + RequestOwner::new(":1.2", None), | ||
| 562 | + None, | ||
| 563 | + Instant::now(), | ||
| 564 | + ) | ||
| 565 | + .expect("request should be created"); | ||
| 566 | + state | ||
| 567 | + .requests | ||
| 568 | + .transition( | ||
| 569 | + "req-1", | ||
| 570 | + &RequestOwner::new(":1.2", None), | ||
| 571 | + RequestState::Cancelled, | ||
| 572 | + ) | ||
| 573 | + .expect("first cancel should transition"); | ||
| 574 | + handle_connection(server, &mut state).expect("transition should be handled"); | ||
| 575 | + | ||
| 576 | + let mut response_line = String::new(); | ||
| 577 | + let mut reader = BufReader::new(client); | ||
| 578 | + reader | ||
| 579 | + .read_line(&mut response_line) | ||
| 580 | + .expect("response should be readable"); | ||
| 581 | + let response = ControlResponse::parse_line(&response_line).expect("response should parse"); | ||
| 582 | + assert_eq!( | ||
| 583 | + response, | ||
| 584 | + ControlResponse::AckRequest { | ||
| 585 | + id: "req-1".to_string(), | ||
| 586 | + state: "cancelled".to_string(), | ||
| 587 | + } | ||
| 588 | + ); | ||
| 589 | + assert_eq!(state.requests.state("req-1"), Some(RequestState::Cancelled)); | ||
| 590 | + } | ||
| 591 | + | ||
| 545 | #[test] | 592 | #[test] |
| 546 | fn startup_recovery_expires_non_terminal_requests() { | 593 | fn startup_recovery_expires_non_terminal_requests() { |
| 547 | let path = unique_temp_file(); | 594 | let path = unique_temp_file(); |
garwarp/src/request.rsmodified@@ -191,6 +191,11 @@ impl RequestRegistry { | |||
| 191 | return Err(RequestError::OwnerMismatch(id.to_string())); | 191 | return Err(RequestError::OwnerMismatch(id.to_string())); |
| 192 | } | 192 | } |
| 193 | 193 | ||
| 194 | + if entry.state == target && target.is_terminal() { | ||
| 195 | + entry.last_updated_at = now; | ||
| 196 | + return Ok(()); | ||
| 197 | + } | ||
| 198 | + | ||
| 194 | if !is_valid_transition(entry.state, target) { | 199 | if !is_valid_transition(entry.state, target) { |
| 195 | return Err(RequestError::InvalidTransition { | 200 | return Err(RequestError::InvalidTransition { |
| 196 | id: id.to_string(), | 201 | id: id.to_string(), |
@@ -546,4 +551,26 @@ mod tests { | |||
| 546 | assert!(removed.is_empty()); | 551 | assert!(removed.is_empty()); |
| 547 | assert_eq!(registry.state("req-1"), Some(RequestState::Pending)); | 552 | assert_eq!(registry.state("req-1"), Some(RequestState::Pending)); |
| 548 | } | 553 | } |
| 554 | + | ||
| 555 | + #[test] | ||
| 556 | + fn duplicate_cancel_is_idempotent() { | ||
| 557 | + let now = Instant::now(); | ||
| 558 | + let request_owner = owner(":1.2"); | ||
| 559 | + let mut registry = RequestRegistry::new(Duration::from_secs(5)); | ||
| 560 | + registry | ||
| 561 | + .begin_at("req-1", request_owner.clone(), None, now) | ||
| 562 | + .expect("request should be created"); | ||
| 563 | + registry | ||
| 564 | + .transition_at("req-1", &request_owner, RequestState::Cancelled, now) | ||
| 565 | + .expect("first cancel should succeed"); | ||
| 566 | + registry | ||
| 567 | + .transition_at( | ||
| 568 | + "req-1", | ||
| 569 | + &request_owner, | ||
| 570 | + RequestState::Cancelled, | ||
| 571 | + now + Duration::from_millis(50), | ||
| 572 | + ) | ||
| 573 | + .expect("duplicate cancel should be idempotent"); | ||
| 574 | + assert_eq!(registry.state("req-1"), Some(RequestState::Cancelled)); | ||
| 575 | + } | ||
| 549 | } | 576 | } |