gardesk/garwarp / aae9a57

Browse files

make begin retries idempotent

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
aae9a5763a94fa6ff1ed549b5462d34b58f40e0c
Parents
2dab4d0
Tree
9cd691e

1 changed file

StatusFile+-
M garwarp/src/daemon.rs 115 2
garwarp/src/daemon.rsmodified
@@ -13,7 +13,7 @@ use crate::dbus::{self, SessionNameGuard};
1313
 use crate::error::{PortalError, map_portal_error, map_request_error};
1414
 use crate::lock::SingleInstanceGuard;
1515
 use crate::logging;
16
-use crate::request::{RequestOwner, RequestRegistry, RequestState};
16
+use crate::request::{RequestError, RequestOwner, RequestRegistry, RequestState};
1717
 use crate::request_store;
1818
 use crate::runtime::RuntimePaths;
1919
 use crate::validate::{validate_request_id, validate_request_identity};
@@ -189,12 +189,40 @@ fn handle_connection(stream: UnixStream, state: &mut DaemonState) -> io::Result<
189189
190190
             match state
191191
                 .requests
192
-                .begin(id.clone(), owner, parsed_parent_window)
192
+                .begin(id.clone(), owner.clone(), parsed_parent_window)
193193
             {
194194
                 Ok(()) => ControlResponse::AckRequest {
195195
                     id,
196196
                     state: "pending".to_string(),
197197
                 },
198
+                Err(RequestError::AlreadyExists(_)) => {
199
+                    let existing = state.requests.record(&id);
200
+                    match existing {
201
+                        Some(record)
202
+                            if record.owner == owner
203
+                                && record.parent_window == parsed_parent_window =>
204
+                        {
205
+                            ControlResponse::AckRequest {
206
+                                id,
207
+                                state: record.state.as_str().to_string(),
208
+                            }
209
+                        }
210
+                        Some(record) if record.owner != owner => {
211
+                            let mapping = map_portal_error(&PortalError::OwnershipMismatch);
212
+                            ControlResponse::Error {
213
+                                code: mapping.code as u32,
214
+                                reason: mapping.reason.to_string(),
215
+                            }
216
+                        }
217
+                        _ => {
218
+                            let mapping = map_portal_error(&PortalError::RequestAlreadyExists);
219
+                            ControlResponse::Error {
220
+                                code: mapping.code as u32,
221
+                                reason: mapping.reason.to_string(),
222
+                            }
223
+                        }
224
+                    }
225
+                }
198226
                 Err(error) => {
199227
                     let mapping = map_request_error(&error);
200228
                     ControlResponse::Error {
@@ -446,6 +474,91 @@ mod tests {
446474
         assert_eq!(state.requests.in_flight_count(), 1);
447475
     }
448476
477
+    #[test]
478
+    fn duplicate_begin_with_same_owner_is_idempotent() {
479
+        let (mut client, server) = UnixStream::pair().expect("pair should be created");
480
+        client
481
+            .write_all(b"begin id=req-1 sender=:1.2 parent=x11:0x2a\n")
482
+            .expect("begin request should be written");
483
+
484
+        let mut state = DaemonState {
485
+            health: HealthStatus::Healthy,
486
+            requests: RequestRegistry::new(Duration::from_secs(5)),
487
+            running: true,
488
+        };
489
+        state
490
+            .requests
491
+            .begin_at(
492
+                "req-1",
493
+                RequestOwner::new(":1.2", None),
494
+                Some(ParentWindowContext::X11 { window_id: 42 }),
495
+                Instant::now(),
496
+            )
497
+            .expect("request should be created");
498
+        state
499
+            .requests
500
+            .transition(
501
+                "req-1",
502
+                &RequestOwner::new(":1.2", None),
503
+                RequestState::AwaitingUser,
504
+            )
505
+            .expect("request should transition");
506
+
507
+        handle_connection(server, &mut state).expect("begin should be handled");
508
+
509
+        let mut response_line = String::new();
510
+        let mut reader = BufReader::new(client);
511
+        reader
512
+            .read_line(&mut response_line)
513
+            .expect("response should be readable");
514
+        let response = ControlResponse::parse_line(&response_line).expect("response should parse");
515
+        assert_eq!(
516
+            response,
517
+            ControlResponse::AckRequest {
518
+                id: "req-1".to_string(),
519
+                state: "awaiting_user".to_string(),
520
+            }
521
+        );
522
+    }
523
+
524
+    #[test]
525
+    fn duplicate_begin_with_different_owner_maps_to_ownership_mismatch() {
526
+        let (mut client, server) = UnixStream::pair().expect("pair should be created");
527
+        client
528
+            .write_all(b"begin id=req-1 sender=:1.7 parent=x11:0x2a\n")
529
+            .expect("begin request should be written");
530
+
531
+        let mut state = DaemonState {
532
+            health: HealthStatus::Healthy,
533
+            requests: RequestRegistry::new(Duration::from_secs(5)),
534
+            running: true,
535
+        };
536
+        state
537
+            .requests
538
+            .begin_at(
539
+                "req-1",
540
+                RequestOwner::new(":1.2", None),
541
+                Some(ParentWindowContext::X11 { window_id: 42 }),
542
+                Instant::now(),
543
+            )
544
+            .expect("request should be created");
545
+        handle_connection(server, &mut state).expect("begin should be handled");
546
+
547
+        let mut response_line = String::new();
548
+        let mut reader = BufReader::new(client);
549
+        reader
550
+            .read_line(&mut response_line)
551
+            .expect("response should be readable");
552
+        let response = ControlResponse::parse_line(&response_line).expect("response should parse");
553
+        assert_eq!(
554
+            response,
555
+            ControlResponse::Error {
556
+                code: 2,
557
+                reason: "ownership_mismatch".to_string(),
558
+            }
559
+        );
560
+    }
561
+
449562
     #[test]
450563
     fn invalid_parent_window_maps_to_stable_reason() {
451564
         let (mut client, server) = UnixStream::pair().expect("pair should be created");