make transition retries idempotent
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
8bc833741a83c7293e29a28950bc892d33137a8e- Parents
-
aae9a57 - Tree
8aedddf
8bc8337
8bc833741a83c7293e29a28950bc892d33137a8eaae9a57
8aedddf| Status | File | + | - |
|---|---|---|---|
| M |
garwarp/src/daemon.rs
|
50 | 0 |
| M |
garwarp/src/request.rs
|
23 | 1 |
garwarp/src/daemon.rsmodified@@ -732,6 +732,56 @@ mod tests { | |||
| 732 | assert_eq!(state.requests.state("req-1"), Some(RequestState::Cancelled)); | 732 | assert_eq!(state.requests.state("req-1"), Some(RequestState::Cancelled)); |
| 733 | } | 733 | } |
| 734 | 734 | ||
| 735 | + #[test] | ||
| 736 | + fn duplicate_awaiting_user_returns_ack() { | ||
| 737 | + let (mut client, server) = UnixStream::pair().expect("pair should be created"); | ||
| 738 | + client | ||
| 739 | + .write_all(b"transition id=req-1 sender=:1.2 state=awaiting_user\n") | ||
| 740 | + .expect("transition request should be written"); | ||
| 741 | + | ||
| 742 | + let mut state = DaemonState { | ||
| 743 | + health: HealthStatus::Healthy, | ||
| 744 | + requests: RequestRegistry::new(Duration::from_secs(5)), | ||
| 745 | + running: true, | ||
| 746 | + }; | ||
| 747 | + state | ||
| 748 | + .requests | ||
| 749 | + .begin_at( | ||
| 750 | + "req-1", | ||
| 751 | + RequestOwner::new(":1.2", None), | ||
| 752 | + None, | ||
| 753 | + Instant::now(), | ||
| 754 | + ) | ||
| 755 | + .expect("request should be created"); | ||
| 756 | + state | ||
| 757 | + .requests | ||
| 758 | + .transition( | ||
| 759 | + "req-1", | ||
| 760 | + &RequestOwner::new(":1.2", None), | ||
| 761 | + RequestState::AwaitingUser, | ||
| 762 | + ) | ||
| 763 | + .expect("first awaiting_user should transition"); | ||
| 764 | + handle_connection(server, &mut state).expect("transition should be handled"); | ||
| 765 | + | ||
| 766 | + let mut response_line = String::new(); | ||
| 767 | + let mut reader = BufReader::new(client); | ||
| 768 | + reader | ||
| 769 | + .read_line(&mut response_line) | ||
| 770 | + .expect("response should be readable"); | ||
| 771 | + let response = ControlResponse::parse_line(&response_line).expect("response should parse"); | ||
| 772 | + assert_eq!( | ||
| 773 | + response, | ||
| 774 | + ControlResponse::AckRequest { | ||
| 775 | + id: "req-1".to_string(), | ||
| 776 | + state: "awaiting_user".to_string(), | ||
| 777 | + } | ||
| 778 | + ); | ||
| 779 | + assert_eq!( | ||
| 780 | + state.requests.state("req-1"), | ||
| 781 | + Some(RequestState::AwaitingUser) | ||
| 782 | + ); | ||
| 783 | + } | ||
| 784 | + | ||
| 735 | #[test] | 785 | #[test] |
| 736 | fn inspect_returns_request_snapshot() { | 786 | fn inspect_returns_request_snapshot() { |
| 737 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); | 787 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); |
garwarp/src/request.rsmodified@@ -191,7 +191,7 @@ 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() { | 194 | + if entry.state == target { |
| 195 | entry.last_updated_at = now; | 195 | entry.last_updated_at = now; |
| 196 | return Ok(()); | 196 | return Ok(()); |
| 197 | } | 197 | } |
@@ -584,6 +584,28 @@ mod tests { | |||
| 584 | assert_eq!(registry.state("req-1"), Some(RequestState::Cancelled)); | 584 | assert_eq!(registry.state("req-1"), Some(RequestState::Cancelled)); |
| 585 | } | 585 | } |
| 586 | 586 | ||
| 587 | + #[test] | ||
| 588 | + fn duplicate_awaiting_user_is_idempotent() { | ||
| 589 | + let now = Instant::now(); | ||
| 590 | + let request_owner = owner(":1.2"); | ||
| 591 | + let mut registry = RequestRegistry::new(Duration::from_secs(5)); | ||
| 592 | + registry | ||
| 593 | + .begin_at("req-1", request_owner.clone(), None, now) | ||
| 594 | + .expect("request should be created"); | ||
| 595 | + registry | ||
| 596 | + .transition_at("req-1", &request_owner, RequestState::AwaitingUser, now) | ||
| 597 | + .expect("first awaiting_user should succeed"); | ||
| 598 | + registry | ||
| 599 | + .transition_at( | ||
| 600 | + "req-1", | ||
| 601 | + &request_owner, | ||
| 602 | + RequestState::AwaitingUser, | ||
| 603 | + now + Duration::from_millis(50), | ||
| 604 | + ) | ||
| 605 | + .expect("duplicate awaiting_user should be idempotent"); | ||
| 606 | + assert_eq!(registry.state("req-1"), Some(RequestState::AwaitingUser)); | ||
| 607 | + } | ||
| 608 | + | ||
| 587 | #[test] | 609 | #[test] |
| 588 | fn record_returns_current_snapshot() { | 610 | fn record_returns_current_snapshot() { |
| 589 | let now = Instant::now(); | 611 | let now = Instant::now(); |