Update lox-distributor to handle gone resources more correctly

crates/lox-distributor/src/lox_context.rs

@@ -66,15 +66,17 @@ impl LoxServerContext {
         ba_obj.add_spare_bucket(bucket);
     }
 
-    pub fn replace_with_new(&self, bridgeline: BridgeLine) -> bool {
+    pub fn replace_with_new(&self, bridgeline: BridgeLine) -> lox::ReplaceSuccess {
         let mut ba_obj = self.ba.lock().unwrap();
         let eb_obj = self.extra_bridges.lock().unwrap();
         let available_bridge = eb_obj.last();
-        // .last() doesn't actually remove the object so we still have to do that
+        let result = ba_obj.bridge_replace(&bridgeline, available_bridge);
-        if eb_obj.len() > 0 {
+        // .last() doesn't actually remove the object so we still have to do that if the bridge was
+        // replaced with an available bridge
+        if result == lox::ReplaceSuccess::Replaced && eb_obj.len() > 0 {
             self.remove_single_bridge();
         }
-        ba_obj.bridge_replace(&bridgeline, available_bridge)
+        result
     }
 
     pub fn add_unreachable(&self, bridgeline: BridgeLine) -> bool {

crates/lox-distributor/src/main.rs

@@ -135,9 +135,15 @@ async fn context_manager(mut context_rx: mpsc::Receiver<Command>) {
                             if context.unreplaced_bridges.lock().unwrap().len() > 0 {
                                 println!("BridgeLine to be replaced: {:?}", bridgeline);
                                 let res = context.replace_with_new(bridgeline);
-                                if res {
+                                if res == lox::ReplaceSuccess::NotFound {
+                                    println!(
+                                        "BridgeLine not found in bridge_table, already updated {:?}",
+                                        bridgeline
+                                    );
+                                } else if res == lox::ReplaceSuccess::Replaced {
                                     println!("BridgeLine successfully replaced: {:?}", bridgeline);
                                 } else {
+                                    assert!(res == lox::ReplaceSuccess::NotReplaced, "ReplaceSuccess incorrectly set somehow");
                                     // Add the bridge to the list of unreplaced bridges in the Lox context and try
                                     // again to replace at the next update (nothing changes in the Lox Authority)
                                     println!("'Gone' BridgeLine NOT replaced, saved for next update! : {:?}", bridgeline);
@@ -210,9 +216,9 @@ async fn context_manager(mut context_rx: mpsc::Receiver<Command>) {
                                 let bridgeline = parse_resource(resource);
                                 println!("BridgeLine to be replaced: {:?}", bridgeline);
                                 let res = context.replace_with_new(bridgeline);
-                                if res {
+                                if res == lox::ReplaceSuccess::Replaced {
                                     println!("BridgeLine successfully replaced: {:?}", bridgeline);
-                                } else {
+                                } else if res == lox::ReplaceSuccess::NotReplaced {
                                     // Add the bridge to the list of unreplaced bridges in the Lox context and try
                                     // again to replace at the next update (nothing changes in the Lox Authority)
                                     println!(

crates/lox-distributor/src/request_handler.rs

@@ -322,21 +322,21 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_handle_not_found(){
+    async fn test_handle_not_found() {
         let th = TestHarness::new();
         // Test Random page
         let four_oh_four_req = Request::builder()
-                .header("Content-Type", "application/json")
+            .header("Content-Type", "application/json")
-                .method("POST")
+            .method("POST")
-                .uri("http://localhost/givemecreds")
+            .uri("http://localhost/givemecreds")
-                .body(Body::empty())
+            .body(Body::empty())
-                .unwrap();
+            .unwrap();
         let not_found_response = handle(th.context.clone(), four_oh_four_req).await.unwrap();
         assert_eq!(not_found_response.status(), StatusCode::NOT_FOUND);
     }
 
     #[tokio::test]
-    async fn test_handle_bad_request(){
+    async fn test_handle_bad_request() {
         let th = TestHarness::new();
         // Test that empty request to a credential issuing endpoint fails
         let req = Request::builder()
@@ -349,7 +349,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_handle_invite(){
+    async fn test_handle_invite() {
         let th = TestHarness::new();
         let lc = LoxClientMock {};
 
@@ -360,7 +360,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_handle_reachability(){
+    async fn test_handle_reachability() {
         let th = TestHarness::new();
         let lc = LoxClientMock {};
         // Test Reachability
@@ -372,7 +372,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_handle_pubkeys(){
+    async fn test_handle_pubkeys() {
         let th = TestHarness::new();
         let lc = LoxClientMock {};
         // Test Pubkeys
@@ -382,7 +382,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_handle_lox_protocols(){
+    async fn test_handle_lox_protocols() {
         let mut th = TestHarness::new();
         let lc = LoxClientMock {};
         // Request Invite and pubkeys required for protocol tests
@@ -411,12 +411,13 @@ mod tests {
         let pubkeys = body_to_string(pubkey_response).await;
         let pubkeys_obj: lox_utils::PubKeys = serde_json::from_str(&pubkeys).unwrap();
 
-         // Test Trust Promotion and get response
+        // Test Trust Promotion and get response
         let lox_cred = lox::proto::open_invite::handle_response(
             state,
             open_response_obj,
             &pubkeys_obj.lox_pub,
-        ).unwrap();
+        )
+        .unwrap();
         let lox_cred: lox_utils::LoxCredential = lox_utils::LoxCredential {
             lox_credential: lox_cred.0,
             bridgeline: Some(lox_cred.1),

crates/lox-library/src/lib.rs

@@ -58,6 +58,13 @@ lazy_static! {
         dalek_constants::RISTRETTO_BASEPOINT_TABLE;
 }
 
+#[derive(PartialEq, Eq)]
+pub enum ReplaceSuccess {
+    NotFound = 0,
+    NotReplaced = 1,
+    Replaced = 2,
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct IssuerPrivKey {
     x0tilde: Scalar,
@@ -406,70 +413,76 @@ impl BridgeAuth {
         &mut self,
         bridge: &BridgeLine,
         available_bridge: Option<&BridgeLine>,
-    ) -> bool {
+    ) -> ReplaceSuccess {
-        let mut res: bool = false;
+        let mut res = ReplaceSuccess::NotFound;
         let reachable_bridges = &self.bridge_table.reachable.clone();
-        if let Some(positions) = reachable_bridges.get(bridge) {
+        match reachable_bridges.get(bridge) {
-            if let Some(replacement) = available_bridge {
+            Some(positions) => {
-                for (bucketnum, offset) in positions.iter() {
+                println!("Should not get here");
-                    assert!(self.bridge_table.buckets[*bucketnum as usize][*offset] == *bridge);
+                if let Some(replacement) = available_bridge {
-                    self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
+                    for (bucketnum, offset) in positions.iter() {
-                    // Remove the bridge from the reachable bridges and add new bridge
+                        assert!(self.bridge_table.buckets[*bucketnum as usize][*offset] == *bridge);
-                    self.bridge_table
+                        self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
-                        .reachable
+                        // Remove the bridge from the reachable bridges and add new bridge
-                        .insert(*replacement, positions.clone());
+                        self.bridge_table
-                    // Remove the bridge from the bucket
+                            .reachable
-                    self.bridge_table.reachable.remove(bridge);
+                            .insert(*replacement, positions.clone());
-                    res = true
+                        // Remove the bridge from the bucket
-                }
+                        self.bridge_table.reachable.remove(bridge);
-            } else if !self.bridge_table.unallocated_bridges.is_empty() {
+                        res = ReplaceSuccess::Replaced
-                let replacement = &self.bridge_table.unallocated_bridges.pop().unwrap();
-                for (bucketnum, offset) in positions.iter() {
-                    self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
-                    self.bridge_table
-                        .reachable
-                        .insert(*replacement, positions.clone());
-                    // Remove the bridge from the bucket
-                    self.bridge_table.reachable.remove(bridge);
-                }
-                res = true
-            } else if !self.bridge_table.spares.is_empty() {
-                // Get the first spare and remove it from the spares set.
-                let spare = *self.bridge_table.spares.iter().next().unwrap();
-                self.bridge_table.spares.remove(&spare);
-                // Indicate the removed bucket as a recyclable bucket
-                self.bridge_table.recyclable.insert(spare);
-                // Get the actual bridges from the spare bucket
-                let spare_bucket = self.bridge_table.buckets[spare as usize];
-                let mut replacement: &BridgeLine = &BridgeLine::default();
-                // Make the first spare the replacement bridge, add the others to the set of
-                // unallocated_bridges
-                for spare_bridge in spare_bucket.iter() {
-                    if replacement.port > 0 {
-                        self.bridge_table.unallocated_bridges.push(*spare_bridge);
-                        // Mark bucket as unreachable while it is unallocated
-                        self.bridge_table.reachable.remove(spare_bridge);
-                    } else {
-                        replacement = spare_bridge;
                     }
+                } else if !self.bridge_table.unallocated_bridges.is_empty() {
+                    let replacement = &self.bridge_table.unallocated_bridges.pop().unwrap();
+                    for (bucketnum, offset) in positions.iter() {
+                        self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
+                        self.bridge_table
+                            .reachable
+                            .insert(*replacement, positions.clone());
+                        // Remove the bridge from the bucket
+                        self.bridge_table.reachable.remove(bridge);
+                    }
+                    res = ReplaceSuccess::Replaced
+                } else if !self.bridge_table.spares.is_empty() {
+                    // Get the first spare and remove it from the spares set.
+                    let spare = *self.bridge_table.spares.iter().next().unwrap();
+                    self.bridge_table.spares.remove(&spare);
+                    // Indicate the removed bucket as a recyclable bucket
+                    self.bridge_table.recyclable.insert(spare);
+                    // Get the actual bridges from the spare bucket
+                    let spare_bucket = self.bridge_table.buckets[spare as usize];
+                    let mut replacement: &BridgeLine = &BridgeLine::default();
+                    // Make the first spare the replacement bridge, add the others to the set of
+                    // unallocated_bridges
+                    for spare_bridge in spare_bucket.iter() {
+                        if replacement.port > 0 {
+                            self.bridge_table.unallocated_bridges.push(*spare_bridge);
+                            // Mark bucket as unreachable while it is unallocated
+                            self.bridge_table.reachable.remove(spare_bridge);
+                        } else {
+                            replacement = spare_bridge;
+                        }
+                    }
+                    for (bucketnum, offset) in positions.iter() {
+                        self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
+                        self.bridge_table
+                            .reachable
+                            .insert(*replacement, positions.clone());
+                        // Remove the bridge from the bucket
+                        self.bridge_table.reachable.remove(bridge);
+                    }
+                    res = ReplaceSuccess::Replaced
+                } else {
+                    // If there are no available bridges that can be assigned here, the only thing
+                    // that can be done is return an indication that updating the gone bridge
+                    // didn't work.
+                    // In this case, we do not mark the bridge as unreachable or remove it from the
+                    // reachable bridges so that we can still find it when a new bridge does become available
+                    res = ReplaceSuccess::NotReplaced
                 }
-                for (bucketnum, offset) in positions.iter() {
-                    self.bridge_table.buckets[*bucketnum as usize][*offset] = *replacement;
-                    self.bridge_table
-                        .reachable
-                        .insert(*replacement, positions.clone());
-                    // Remove the bridge from the bucket
-                    self.bridge_table.reachable.remove(bridge);
-                }
-                res = true
             }
-            // If there are no available bridges that can be assigned here, the only thing
+            None => return res,
-            // that can be done is return an indication that updating the gone bridge
+        };
-            // didn't work.
+        return res;
-            // In this case, we do not mark the bridge as unreachable or remove it from the
-            // reachable bridges so that we can still find it when a new bridge does become available
-        }
-        res
     }
 
     /// Mark a bridge as unreachable

crates/lox-library/src/tests.rs

@@ -727,7 +727,7 @@ fn test_update_bridge() {
 #[test]
 fn test_bridge_replace() {
     // Create 3 open invitation buckets and 3 spare buckets
-    let cases = vec!["available", "unallocated", "spare", "failed"];
+    let cases = vec!["not found", "available", "unallocated", "spare", "failed"];
     for case in cases {
         let mut th: TestHarness;
         if case != "failed" {
@@ -735,6 +735,7 @@ fn test_bridge_replace() {
         } else {
             th = TestHarness::new_buckets(5, 0);
         }
+
         // Randomly select a bridge to replace
         let table_size = th.ba.bridge_table.buckets.len();
         let num = rand::thread_rng().gen_range(0, table_size - 1);
@@ -747,7 +748,22 @@ fn test_bridge_replace() {
                 .contains_key(replacement_bridge),
             "Random bridge to replace not in reachable bridges"
         );
+
         match case {
+            "not found" => {
+                // Case zero: bridge to be replaced is not in the bridgetable
+                let random_bridgeline = &BridgeLine::random();
+                assert!(
+                    !th.ba.bridge_table.reachable.contains_key(random_bridgeline),
+                    "Random bridgeline happens to be in the bridge_table (and should not be)"
+                );
+                assert!(
+                    th.ba
+                        .bridge_replace(random_bridgeline, Some(random_bridgeline))
+                        == ReplaceSuccess::NotFound,
+                    "Bridge should be marked as NotFound"
+                );
+            }
             "available" => {
                 // Case one: available_bridge != null
                 let random_bridgeline = &BridgeLine::random();
@@ -766,7 +782,8 @@ fn test_bridge_replace() {
                 );
                 assert!(
                     th.ba
-                        .bridge_replace(replacement_bridge, Some(random_bridgeline)),
+                        .bridge_replace(replacement_bridge, Some(random_bridgeline))
+                        == ReplaceSuccess::Replaced,
                     "Bridge was not replaced with available bridge"
                 );
                 assert!(
@@ -807,7 +824,7 @@ fn test_bridge_replace() {
                     "Unallocated bridge already marked as reachable"
                 );
                 assert!(
-                    th.ba.bridge_replace(replacement_bridge, None),
+                    th.ba.bridge_replace(replacement_bridge, None) == ReplaceSuccess::Replaced,
                     "Bridge was not replaced with available bridge"
                 );
                 assert!(
@@ -830,13 +847,13 @@ fn test_bridge_replace() {
                 println!("Successfully added unallocated bridgeline");
             }
             "spare" => {
-                // Case three: available_bridge == null and unallocated_bridges ==null
+                // Case three: available_bridge == null and unallocated_bridges == null
                 assert!(
                     th.ba.bridge_table.unallocated_bridges.len() == 0,
                     "Unallocated bridges should have a length of 0"
                 );
                 assert!(
-                    th.ba.bridge_replace(replacement_bridge, None),
+                    th.ba.bridge_replace(replacement_bridge, None) == ReplaceSuccess::Replaced,
                     "Bridge was not replaced with available spare bridge"
                 );
                 assert!(
@@ -865,7 +882,7 @@ fn test_bridge_replace() {
                     "Unallocated bridges should have a length of 0"
                 );
                 assert!(
-                    !th.ba.bridge_replace(replacement_bridge, None),
+                    th.ba.bridge_replace(replacement_bridge, None) == ReplaceSuccess::NotReplaced,
                     "Bridge was somehow marked as replaced despite no replaceable bridges"
                 );
                 assert!(