From 82604139ce94173d2134f5b700c88a03d6fdb1f2 Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Fri, 22 Dec 2023 16:18:53 -0600 Subject: [PATCH] refactor(windows): remove ResourceDisplay (#3002) ... and move its methods into ResourceDescription. This was a TODO from some pull request in the last few days. I assume the goal is to share this function between all clients if needed. It doesn't reduce the number of lines of code, since I could have removed ResourceDisplay and done this on-the-fly when building the systray menu, as an alternative. --- rust/connlib/shared/src/messages.rs | 21 ++++++++++- .../src-tauri/src/client/gui.rs | 7 ++-- .../src/client/gui/system_tray_menu.rs | 37 +++---------------- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/rust/connlib/shared/src/messages.rs b/rust/connlib/shared/src/messages.rs index becc1cb5f..6af134185 100644 --- a/rust/connlib/shared/src/messages.rs +++ b/rust/connlib/shared/src/messages.rs @@ -1,5 +1,8 @@ //! Message types that are used by both the gateway and client. -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::{ + borrow::Cow, + net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, +}; use chrono::{serde::ts_seconds, DateTime, Utc}; use ip_network::IpNetwork; @@ -183,6 +186,22 @@ impl ResourceDescription { ResourceDescription::Cidr(r) => r.id, } } + + /// What the GUI clients should show as the user-friendly display name, e.g. `Firezone GitHub` + pub fn name(&self) -> &str { + match self { + ResourceDescription::Dns(r) => &r.name, + ResourceDescription::Cidr(r) => &r.name, + } + } + + /// What the GUI clients should paste to the clipboard, e.g. `https://github.com/firezone` + pub fn pastable(&self) -> Cow<'_, str> { + match self { + ResourceDescription::Dns(r) => Cow::from(&r.address), + ResourceDescription::Cidr(r) => Cow::from(r.address.to_string()), + } + } } /// Description of a resource that maps to a CIDR. diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index aa98149c1..3e353d300 100755 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -14,7 +14,7 @@ use connlib_client_shared::{file_logger, ResourceDescription}; use connlib_shared::messages::ResourceId; use secrecy::{ExposeSecret, SecretString}; use std::{net::IpAddr, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; -use system_tray_menu::{Event as TrayMenuEvent, Resource as ResourceDisplay}; +use system_tray_menu::Event as TrayMenuEvent; use tauri::{Manager, SystemTray, SystemTrayEvent}; use tokio::{ sync::{mpsc, oneshot, Notify}, @@ -428,6 +428,7 @@ impl Controller { async fn handle_deep_link(&mut self, url: &url::Url) -> Result<()> { let Some(auth) = client::deep_link::parse_auth_callback(url) else { + // TODO: `bail` is redundant here, just do `.context("")?;` since it's `anyhow` bail!("couldn't parse scheme request"); }; @@ -448,6 +449,7 @@ impl Controller { token: auth.token, }; if let Err(e) = self.start_session(auth_info) { + // TODO: Replace `bail` with `context` here too bail!("couldn't start session: {e:#?}"); } Ok(()) @@ -458,8 +460,7 @@ impl Controller { tracing::warn!("got notified to update resources but there is no session"); return Ok(()); }; - let resources = session.callback_handler.resources.load().as_ref().clone(); - let resources: Vec<_> = resources.into_iter().map(ResourceDisplay::from).collect(); + let resources = session.callback_handler.resources.load(); // TODO: Save the user name between runs of the app let actor_name = self .session diff --git a/rust/windows-client/src-tauri/src/client/gui/system_tray_menu.rs b/rust/windows-client/src-tauri/src/client/gui/system_tray_menu.rs index ad21f7eb8..be0b5dfa8 100755 --- a/rust/windows-client/src-tauri/src/client/gui/system_tray_menu.rs +++ b/rust/windows-client/src-tauri/src/client/gui/system_tray_menu.rs @@ -2,34 +2,6 @@ use connlib_client_shared::ResourceDescription; use std::str::FromStr; use tauri::{CustomMenuItem, SystemTrayMenu, SystemTrayMenuItem, SystemTraySubmenu}; -/// The information needed for the GUI to display a resource inside the Firezone VPN -// TODO: Just make these methods on `ResourceDescription` -pub(crate) struct Resource { - pub id: connlib_shared::messages::ResourceId, - /// User-friendly name, e.g. "GitLab" - pub name: String, - /// What will be copied to the clipboard to paste into a web browser - pub pastable: String, -} - -impl From for Resource { - fn from(x: ResourceDescription) -> Self { - match x { - ResourceDescription::Dns(x) => Self { - id: x.id, - name: x.name, - pastable: x.address, - }, - ResourceDescription::Cidr(x) => Self { - id: x.id, - name: x.name, - // TODO: CIDRs aren't URLs right? - pastable: x.address.to_string(), - }, - } - } -} - #[derive(Debug, PartialEq)] pub(crate) enum Event { About, @@ -64,7 +36,7 @@ impl FromStr for Event { } } -pub(crate) fn signed_in(user_name: &str, resources: &[Resource]) -> SystemTrayMenu { +pub(crate) fn signed_in(user_name: &str, resources: &[ResourceDescription]) -> SystemTrayMenu { let mut menu = SystemTrayMenu::new() .add_item( CustomMenuItem::new("".to_string(), format!("Signed in as {user_name}")).disabled(), @@ -73,12 +45,13 @@ pub(crate) fn signed_in(user_name: &str, resources: &[Resource]) -> SystemTrayMe .add_native_item(SystemTrayMenuItem::Separator) .add_item(CustomMenuItem::new("".to_string(), "Resources").disabled()); - for Resource { id, name, pastable } in resources { + for res in resources { + let id = res.id(); let submenu = SystemTrayMenu::new().add_item(CustomMenuItem::new( format!("/resource/{id}"), - pastable.to_string(), + res.pastable(), )); - menu = menu.add_submenu(SystemTraySubmenu::new(name, submenu)); + menu = menu.add_submenu(SystemTraySubmenu::new(res.name(), submenu)); } menu = menu