From f783b4b1e2936f1cde43b87ef79119d911fb20a2 Mon Sep 17 00:00:00 2001 From: Gabi Date: Wed, 21 Aug 2024 18:13:13 -0300 Subject: [PATCH] chore(connlib): set internet resource at the top of the list (#6363) Part of #6047 --- rust/connlib/shared/src/callbacks.rs | 108 +++++++++++++++++++++ rust/connlib/shared/src/messages.rs | 70 ------------- rust/connlib/shared/src/messages/client.rs | 12 --- rust/connlib/tunnel/src/client.rs | 2 +- 4 files changed, 109 insertions(+), 83 deletions(-) diff --git a/rust/connlib/shared/src/callbacks.rs b/rust/connlib/shared/src/callbacks.rs index bf9ec9f3b..18175282d 100644 --- a/rust/connlib/shared/src/callbacks.rs +++ b/rust/connlib/shared/src/callbacks.rs @@ -78,6 +78,10 @@ impl ResourceDescription { ResourceDescription::Internet(r) => r.can_be_disabled, } } + + fn is_internet_resource(&self) -> bool { + matches!(self, ResourceDescription::Internet(_)) + } } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] @@ -132,3 +136,107 @@ pub struct ResourceDescriptionInternet { pub status: Status, pub can_be_disabled: bool, } + +impl PartialOrd for ResourceDescription { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for ResourceDescription { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + if self.is_internet_resource() { + return std::cmp::Ordering::Less; + } + + (self.name(), self.id()).cmp(&(other.name(), other.id())) + } +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use itertools::Itertools; + + use super::{ + ResourceDescription, ResourceDescriptionDns, ResourceDescriptionInternet, ResourceId, Site, + Status, + }; + + 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(), + address_description: Some("test description".to_string()), + sites: vec![Site { + name: "test".to_string(), + id: "99ba0c1e-5189-4cfc-a4db-fd6cb1c937fd".parse().unwrap(), + }], + status: Status::Online, + can_be_disabled: false, + }) + } + + fn internet_resource(uuid: &str) -> ResourceDescription { + ResourceDescription::Internet(ResourceDescriptionInternet { + name: "Internet Resource".to_string(), + address: "All internet addresses".to_string(), + id: ResourceId::from_str(uuid).unwrap(), + sites: vec![Site { + name: "test".to_string(), + id: "99ba0c1e-5189-4cfc-a4db-fd6cb1c937fd".parse().unwrap(), + }], + status: Status::Offline, + can_be_disabled: true, + }) + } + + #[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 internet = internet_resource("cb13bca0-490a-4aae-a039-31a8f93e2281"); + + let resource_descriptions = vec![ + nine.clone(), + ten.clone(), + fast.clone(), + ifconfig.clone(), + emoji.clone(), + example.clone(), + cloudflare.clone(), + metabase_2.clone(), + metabase_1.clone(), + internet.clone(), + ]; + + let expected = vec![ + internet, // Internet resources are always first + ten, // Numbers before letters + nine, // Numbers are sorted lexicographically, not numerically + cloudflare, // Then uppercase, in alphabetical order + example, // Then uppercase, in alphabetical order + fast, // Then uppercase, in alphabetical order + metabase_1, // UUIDs tie-break if the names are identical + metabase_2, // UUIDs tie-break if the names are identical + ifconfig, // Lowercase comes after all uppercase are done + // 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, + ]; + + assert_eq!( + resource_descriptions.into_iter().sorted().collect_vec(), + expected + ); + } +} diff --git a/rust/connlib/shared/src/messages.rs b/rust/connlib/shared/src/messages.rs index 0f74eddbf..b5bc26113 100644 --- a/rust/connlib/shared/src/messages.rs +++ b/rust/connlib/shared/src/messages.rs @@ -353,73 +353,3 @@ pub struct RelaysPresence { /// These relays are still online. We can/should use these. pub connected: Vec, } - -#[cfg(test)] -mod tests { - use std::str::FromStr; - - use itertools::Itertools; - - use super::{ - client::ResourceDescription, - client::{ResourceDescriptionDns, Site}, - ResourceId, - }; - - 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(), - address_description: Some("test description".to_string()), - sites: vec![Site { - name: "test".to_string(), - id: "99ba0c1e-5189-4cfc-a4db-fd6cb1c937fd".parse().unwrap(), - }], - }) - } - - #[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 resource_descriptions = vec![ - nine.clone(), - ten.clone(), - fast.clone(), - ifconfig.clone(), - emoji.clone(), - example.clone(), - cloudflare.clone(), - metabase_2.clone(), - metabase_1.clone(), - ]; - - let expected = vec![ - ten, // Numbers first - nine, // Numbers first - cloudflare, // Then uppercase, in alphabetical order - example, // Then uppercase, in alphabetical order - fast, // Then uppercase, in alphabetical order - metabase_1, // UUIDs tie-break if the names are identical - metabase_2, // UUIDs tie-break if the names are identical - ifconfig, // Lowercase comes after all uppercase are done - // 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, - ]; - - assert_eq!( - resource_descriptions.into_iter().sorted().collect_vec(), - expected - ); - } -} diff --git a/rust/connlib/shared/src/messages/client.rs b/rust/connlib/shared/src/messages/client.rs index e3304ba2d..dbb63b85b 100644 --- a/rust/connlib/shared/src/messages/client.rs +++ b/rust/connlib/shared/src/messages/client.rs @@ -217,18 +217,6 @@ impl ResourceDescription { } } -impl PartialOrd for ResourceDescription { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for ResourceDescription { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - (self.name(), self.id()).cmp(&(other.name(), other.id())) - } -} - #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, Hash)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ResourceDescription { diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index 6f06faf9e..664b66782 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -358,12 +358,12 @@ impl ClientState { pub(crate) fn resources(&self) -> Vec { self.resources_by_id .values() - .sorted() .cloned() .map(|r| { let status = self.resource_status(&r); r.with_status(status) }) + .sorted() .collect_vec() }