From e2efd725e3af1563c6d08305728cd385cdac70d1 Mon Sep 17 00:00:00 2001
From: Reactor Scram <ReactorScram@users.noreply.github.com>
Date: Wed, 31 Jan 2024 19:14:30 -0600
Subject: [PATCH] feat(firezone-tunnel): sort resources alphabetically (#3465)

Closes #3217. I just now noticed that one was assigned to me


![image](https://github.com/firezone/firezone/assets/13400041/106ba400-fda8-49b9-ad81-b6ced8414ea4)

The sorting is naive, just sorts the UTF-8 encoded bytes, so lowercase
resources come after all uppercase resources, and it's probably very
wrong for anything outside Latin-1 and English locale. If the names are
identical, resource ID tie-breaks.
---
 rust/connlib/shared/src/messages.rs |  2 +-
 rust/connlib/tunnel/src/client.rs   | 95 +++++++++++++++++++++++++++--
 2 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/rust/connlib/shared/src/messages.rs b/rust/connlib/shared/src/messages.rs
index cc9e8959c..f945b184a 100644
--- a/rust/connlib/shared/src/messages.rs
+++ b/rust/connlib/shared/src/messages.rs
@@ -19,7 +19,7 @@ use crate::Dname;
 
 #[derive(Hash, Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq)]
 pub struct GatewayId(Uuid);
-#[derive(Hash, Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq)]
+#[derive(Hash, Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ResourceId(Uuid);
 #[derive(Hash, Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Eq)]
 pub struct ClientId(Uuid);
diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs
index df04a2fb6..52ed5cf58 100644
--- a/rust/connlib/tunnel/src/client.rs
+++ b/rust/connlib/tunnel/src/client.rs
@@ -83,12 +83,20 @@ where
             }
         }
 
-        let mut role_state = self.role_state.lock();
-        role_state
-            .resource_ids
-            .insert(resource_description.id(), resource_description);
-        self.callbacks
-            .on_update_resources(role_state.resource_ids.values().cloned().collect())?;
+        let mut resource_descriptions = {
+            let mut role_state = self.role_state.lock();
+            role_state
+                .resource_ids
+                .insert(resource_description.id(), resource_description);
+            role_state
+                .resource_ids
+                .values()
+                .cloned()
+                .collect::<Vec<_>>()
+        };
+        sort_resources(&mut resource_descriptions);
+
+        self.callbacks.on_update_resources(resource_descriptions)?;
         Ok(())
     }
 
@@ -800,3 +808,78 @@ impl RoleState for ClientState {
         peers_to_stop
     }
 }
+
+/// Sorts `resource_descriptions` in-place alphabetically
+fn sort_resources(resource_descriptions: &mut [ResourceDescription]) {
+    // Unstable sort is slightly faster, and we use the ID as a tie-break,
+    // so stability should not matter much
+    resource_descriptions.sort_unstable_by(|a, b| (a.name(), a.id()).cmp(&(b.name(), b.id())));
+}
+
+#[cfg(test)]
+mod tests {
+    use connlib_shared::messages::{ResourceDescription, ResourceDescriptionDns, ResourceId};
+    use std::str::FromStr;
+
+    fn fake_resource(name: &str, uuid: &str) -> ResourceDescription {
+        ResourceDescription::Dns(ResourceDescriptionDns {
+            id: ResourceId::from_str(uuid).unwrap(),
+            name: name.to_string(),
+            address: "unused.example.com".to_string(),
+        })
+    }
+
+    #[test]
+    fn sort_resources_normal() {
+        let cloudflare = fake_resource("Cloudflare DNS", "2efe9c25-bd92-49a0-99d7-8b92da014dd5");
+        let example = fake_resource("Example", "613eaf56-6efa-45e5-88aa-ea4ad64d8c18");
+        let fast = fake_resource("Fast.com", "624b7154-08f6-4c9e-bac0-c3a587fc9322");
+        let metabase_1 = fake_resource("Metabase", "98ee1682-8192-4f15-b4a6-03178dfa7f95");
+        let metabase_2 = fake_resource("Metabase", "e431d1b8-afc2-4f93-95c2-0d15413f5422");
+        let ifconfig = fake_resource("ifconfig.net", "6b7188f5-00ac-41dc-9ddd-57e2384f31ef");
+        let ten = fake_resource("10", "9d1907cc-0693-4063-b388-4d29524e2514");
+        let nine = fake_resource("9", "a7b66f28-9cd1-40fc-bdc4-4763ed92ea41");
+        let emoji = fake_resource("🫠", "7d08cfca-8737-4c5e-a88e-e92574657217");
+
+        let mut resource_descriptions = vec![
+            nine.clone(),
+            ten.clone(),
+            fast.clone(),
+            ifconfig.clone(),
+            emoji.clone(),
+            example.clone(),
+            cloudflare.clone(),
+            metabase_2.clone(),
+            metabase_1.clone(),
+        ];
+
+        super::sort_resources(&mut resource_descriptions);
+
+        let expected = vec![
+            // Numbers first
+            // Numbers are sorted byte-wise, if they don't use leading zeroes
+            // they won't be in numeric order
+            ten.clone(),
+            nine.clone(),
+            // Then uppercase, in alphabetical order
+            cloudflare.clone(),
+            example.clone(),
+            fast.clone(),
+            // UUIDs tie-break if the names are identical
+            metabase_1.clone(),
+            metabase_2.clone(),
+            // Lowercase comes after all uppercase are done
+            ifconfig.clone(),
+            // Emojis start with a leading '1' bit, so they come after all
+            // [Basic Latin](https://en.wikipedia.org/wiki/Basic_Latin_\(Unicode_block\)) chars
+            emoji.clone(),
+        ];
+
+        assert!(
+            resource_descriptions == expected,
+            "Actual: {:#?}\nExpected: {:#?}",
+            resource_descriptions,
+            expected
+        );
+    }
+}