Adds replace_bridge test and fixes bugs in the function

crates/lox-library/src/lib.rs

@@ -334,7 +334,11 @@ impl BridgeAuth {
         // This will be the bad/annoying part
     }
 
-    pub fn allocate_bridges(&mut self, distributor_bridges: &mut Vec<BridgeLine>, bdb: &mut BridgeDb) {
+    pub fn allocate_bridges(
+        &mut self,
+        distributor_bridges: &mut Vec<BridgeLine>,
+        bdb: &mut BridgeDb,
+    ) {
         while let Some(bridge) = distributor_bridges.iter().next_back() {
             self.bridge_table.unallocated_bridges.push(*bridge)
         }
@@ -407,9 +411,9 @@ impl BridgeAuth {
         let mut res: bool = false;
         let reachable_bridges = &self.bridge_table.reachable.clone();
         if let Some(positions) = reachable_bridges.get(bridge) {
+            if available_bridge.is_some() {
                 for (bucketnum, offset) in positions.iter() {
                     assert!(self.bridge_table.buckets[*bucketnum as usize][*offset] == *bridge);
-                if available_bridge.is_some() {
                     self.bridge_table.buckets[*bucketnum as usize][*offset] =
                         *available_bridge.unwrap();
                     // Remove the bridge from the reachable bridges and add new bridge
@@ -419,42 +423,46 @@ impl BridgeAuth {
                     // Remove the bridge from the bucket
                     self.bridge_table.reachable.remove(bridge);
                     res = true
+                }
             } else if !self.bridge_table.unallocated_bridges.is_empty() {
-                    self.bridge_table.buckets[*bucketnum as usize][*offset] =
+                let replacement = &self.bridge_table.unallocated_bridges.pop().unwrap();
-                        self.bridge_table.unallocated_bridges.pop().unwrap();
+                for (bucketnum, offset) in positions.iter() {
-                    // Remove the bridge from the reachable bridges and add new bridge
-                    self.bridge_table.reachable.insert(
-                        self.bridge_table.unallocated_bridges.pop().unwrap(),
-                        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 mut replacement: &BridgeLine = &BridgeLine::default();
-                    // Make the first spare the replacement bridge, add the others to the set of
-                    // unallocated_bridges
-                    let spare_bucket = self.bridge_table.buckets[spare as usize].clone();
-                    for spare_bridge in spare_bucket.iter() {
-                        if replacement.port > 0 {
-                            self.bridge_table.unallocated_bridges.push(*spare_bridge);
-                        } else {
-                            replacement = spare_bridge;
-                        }
-                    }
                     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].clone();
+                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 = true
             }
             // If there are no available bridges that can be assigned here, the only thing
@@ -462,62 +470,6 @@ impl BridgeAuth {
             // 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
-
-
-                // Now we should check that the bucket hasn't become "blocked" due to having no available
-                // bridges
-                if !res {
-                    // Count how many bridges in this bucket are reachable
-                    let numreachable = self.bridge_table.buckets[*bucketnum as usize]
-                        .iter()
-                        .filter(|br| self.bridge_table.reachable.get(br).is_some())
-                        .count();
-
-                    // Is this bucket an open-invitation bucket?
-                    if bdb.openinv_buckets.contains(bucketnum) {
-                        bdb.openinv_buckets.remove(bucketnum);
-                        self.trustup_migration_table.table.remove(bucketnum);
-                        continue;
-                    }
-
-                    // Has the bucket fallen below the
-                    // threshold?
-                    if numreachable == MIN_BUCKET_REACHABILITY {
-                        // This bucket is now unreachable.  Get a spare bucket
-                        if self.bridge_table.spares.is_empty() {
-                            // Uh, oh.  No spares available.  Just delete any
-                            // migrations leading to this bucket.
-                            self.trustup_migration_table
-                                .table
-                                .retain(|_, &mut v| v != *bucketnum);
-                            self.blockage_migration_table
-                                .table
-                                .retain(|_, &mut v| v != *bucketnum);
-                        } else {
-                            // 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);
-                            // Add a blockage migration from this bucket to the spare
-                            self.blockage_migration_table
-                                .table
-                                .insert(*bucketnum, spare);
-                            // Remove any trust upgrade migrations to this
-                            // bucket
-                            self.trustup_migration_table
-                                .table
-                                .retain(|_, &mut v| v != *bucketnum);
-                            // Change any blockage migrations with this bucket
-                            // as the destination to the spare
-                            for (_, v) in self.blockage_migration_table.table.iter_mut() {
-                                if *v == *bucketnum {
-                                    *v = spare;
-                                }
-                            }
-                        }
-                    }
-                }
-            }
         }
         res
     }
@@ -538,6 +490,16 @@ impl BridgeAuth {
     /// there was none available.
     pub fn bridge_unreachable(&mut self, bridge: &BridgeLine, bdb: &mut BridgeDb) -> bool {
         let mut res: bool = true;
+        if self.bridge_table.unallocated_bridges.contains(bridge) {
+            let index = self
+                .bridge_table
+                .unallocated_bridges
+                .iter()
+                .position(|&b| b == *bridge)
+                .unwrap();
+            self.bridge_table.unallocated_bridges.remove(index);
+            res = true;
+        } else {
             let positions = self.bridge_table.reachable.get(bridge);
             if let Some(v) = positions {
                 for (bucketnum, offset) in v.iter() {
@@ -601,6 +563,7 @@ impl BridgeAuth {
                 }
             }
             self.bridge_table.reachable.remove(bridge);
+        }
 
         res
     }

crates/lox-library/src/tests.rs

@@ -598,6 +598,14 @@ fn test_redeem_invite() {
     println!("bob_cred = {:?}", bob_cred);
 }
 
+#[test]
+fn test_allocate_bridges() {
+    let mut th = TestHarness::new();
+    // Check that any bridges in distributor_bridges are first added to unallocated bridges
+    // and finally are added as openinv_buckets
+    // TODO
+}
+
 #[test]
 fn test_update_bridge() {
     let mut th = TestHarness::new();
@@ -612,8 +620,7 @@ fn test_update_bridge() {
     ];
     // Store first bridgeline to update later
     let bridgeline_to_update = bucket[0];
-    // Created changed info for bridgeline to be updated to
+    // Create changed info for bridgeline to be updated to
-    let updated_info = bucket[0];
     let infostr: String = format!(
         "type={} blocked_in={:?} protocol={} distribution={}",
         "obfs2 test bridge".to_string(),
@@ -684,23 +691,169 @@ fn test_update_bridge() {
 
 #[test]
 fn test_bridge_replace() {
-    // TODO
+    // Create 3 open invitation buckets and 3 spare buckets
-    let mut th = TestHarness::new();
+    let cases = vec!["available", "unallocated", "spare", "failed"];
-    // Add new bridge to table with known values,
+    for case in cases {
-    // check that I can find and update the values and that everything else stays the same
+        let mut th: TestHarness;
-
+        if case != "failed" {
-    // Create 3 bridges to test harness
+            th = TestHarness::new();
-    let bucket = [
+        } else {
-        BridgeLine::random(),
+            th = TestHarness::new_buckets(5, 0);
-        BridgeLine::random(),
-        BridgeLine::random(),
-    ];
-
         }
+        // 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);
+        let replaceable_bucket = th.ba.bridge_table.buckets.get(num).unwrap().clone();
+        let replacement_bridge = &replaceable_bucket[0];
+        assert!(
+            th.ba
+                .bridge_table
+                .reachable
+                .contains_key(replacement_bridge),
+            "Random bridge to replace not in reachable bridges"
+        );
+        match case {
+            "available" => {
+                // Case one: available_bridge != null
+                let random_bridgeline = &BridgeLine::random();
+                let unallocated_bridgeline = &BridgeLine::random();
+                th.ba
+                    .bridge_table
+                    .unallocated_bridges
+                    .push(*unallocated_bridgeline);
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(random_bridgeline)
+                        .is_none(),
+                    "Random bridge already in table"
+                );
+                assert!(
+                    th.ba
+                        .bridge_replace(replacement_bridge, Some(random_bridgeline), &mut th.bdb),
+                    "Bridge was not replaced with available bridge"
+                );
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(random_bridgeline)
+                        .is_some(),
+                    "Replacement bridge not added to reachable bridges"
+                );
+                assert!(
+                    table_size == th.ba.bridge_table.buckets.len(),
+                    "Number of buckets changed size"
+                );
+                assert!(
+                    th.ba.bridge_table.unallocated_bridges.len() == 1,
+                    "Extra bridge added to unallocated bridges"
+                );
+                println!("Successfully added passed bridgeline");
+            }
+            // Case two: available_bridge == null and unallocated_bridges !=null
+            "unallocated" => {
+                let unallocated_bridgeline = &BridgeLine::random();
+                th.ba
+                    .bridge_table
+                    .unallocated_bridges
+                    .push(*unallocated_bridgeline);
+                assert!(
+                    th.ba.bridge_table.unallocated_bridges.len() == 1,
+                    "Not enough bridges in unallocated bridges"
+                );
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(unallocated_bridgeline)
+                        .is_none(),
+                    "Unallocated bridge already marked as reachable"
+                );
+                assert!(
+                    th.ba.bridge_replace(replacement_bridge, None, &mut th.bdb),
+                    "Bridge was not replaced with available bridge"
+                );
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(unallocated_bridgeline)
+                        .is_some(),
+                    "Replacement bridge not added to reachable bridges"
+                );
+                assert!(
+                    table_size == th.ba.bridge_table.buckets.len(),
+                    "Number of buckets changed size"
+                );
+                assert!(
+                    th.ba.bridge_table.unallocated_bridges.len() == 0,
+                    "Allocated bridge still in unallocated bridges"
+                );
 
-#[test]
+                println!("Successfully added unallocated bridgeline");
-fn test_allocate_bridges() {
+            }
-    // TODO
+            "spare" => {
+                // 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, &mut th.bdb),
+                    "Bridge was not replaced with available spare bridge"
+                );
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(replacement_bridge)
+                        .is_none(),
+                    "Replacement bridge still marked as reachable"
+                );
+                assert!(
+                    table_size == th.ba.bridge_table.buckets.len(),
+                    "Number of buckets changed size"
+                );
+                assert!(
+                    th.ba.bridge_table.unallocated_bridges.len() == 2,
+                    "Extra spare bridges not added to unallocated bridges"
+                );
+
+                println!("Successfully added unallocated bridgeline");
+            }
+            "failed" => {
+                // Case four: available_bridge == None and unallocated_bridges == None and spare buckets == None
+                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, &mut th.bdb),
+                    "Bridge was somehow marked as replaced despite no replaceable bridges"
+                );
+                assert!(
+                    th.ba
+                        .bridge_table
+                        .reachable
+                        .get(replacement_bridge)
+                        .is_some(),
+                    "Replacement bridge marked as unreachable despite not being replaced"
+                );
+                assert!(
+                    table_size == th.ba.bridge_table.buckets.len(),
+                    "Number of buckets changed size"
+                );
+                assert!(
+                    th.ba.bridge_table.unallocated_bridges.len() == 0,
+                    "Unallocated bridges changed size"
+                );
+                println!("No bridges available to replace bridge so replacement gracefully failed");
+            }
+            _ => {}
+        }
+    }
 }
 
 #[test]