diff --git a/nexus/db-model/src/l4_port_range.rs b/nexus/db-model/src/l4_port_range.rs index b57a484b729..5cd88601515 100644 --- a/nexus/db-model/src/l4_port_range.rs +++ b/nexus/db-model/src/l4_port_range.rs @@ -2,14 +2,13 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use diesel::backend::Backend; -use diesel::deserialize::{self, FromSql}; -use diesel::pg::Pg; -use diesel::serialize::{self, ToSql}; +use super::{DatabaseString, impl_from_sql_text}; use diesel::sql_types; use omicron_common::api::external; use serde::Deserialize; use serde::Serialize; +use std::borrow::Cow; +use std::str::FromStr; /// Newtype wrapper around [`external::L4PortRange`] so we can derive /// diesel traits for it @@ -22,25 +21,16 @@ pub struct L4PortRange(pub external::L4PortRange); NewtypeFrom! { () pub struct L4PortRange(external::L4PortRange); } NewtypeDeref! { () pub struct L4PortRange(external::L4PortRange); } -impl ToSql for L4PortRange { - fn to_sql<'a>( - &'a self, - out: &mut serialize::Output<'a, '_, Pg>, - ) -> serialize::Result { - >::to_sql( - &self.0.to_string(), - &mut out.reborrow(), - ) +impl DatabaseString for L4PortRange { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() } -} -// Deserialize the "L4PortRange" object from SQL INT4. -impl FromSql for L4PortRange -where - DB: Backend, - String: FromSql, -{ - fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result { - String::from_sql(bytes)?.parse().map(L4PortRange).map_err(|e| e.into()) + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) } } + +impl_from_sql_text!(L4PortRange); diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index e0a0910f2aa..7a60c4d4aa6 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -373,6 +373,39 @@ macro_rules! impl_enum_type { pub(crate) use impl_enum_type; +/// Automatically implement `FromSql` and `ToSql` on a provided +/// type which implements the [`DatabaseString`] trait. +/// +/// This is necessary because we cannot blanket impl these (foreign) traits on +/// all implementors. +macro_rules! impl_from_sql_text { + ( + $model_type:ident + ) => { + impl ::diesel::serialize::ToSql<::diesel::sql_types::Text, ::diesel::pg::Pg> for $model_type { + fn to_sql<'a>( + &'a self, + out: &mut ::diesel::serialize::Output<'a, '_, ::diesel::pg::Pg>, + ) -> ::diesel::serialize::Result { + >::to_sql( + &self.to_database_string(), + &mut out.reborrow(), + ) + } + } + + impl ::diesel::deserialize::FromSql<::diesel::sql_types::Text, ::diesel::pg::Pg> for $model_type { + fn from_sql(bytes: <::diesel::pg::Pg as ::diesel::backend::Backend>::RawValue<'_>) + -> ::diesel::deserialize::Result + { + Ok($model_type::from_database_string(::std::str::from_utf8(bytes.as_bytes())?)?) + } + } + } +} + +pub(crate) use impl_from_sql_text; + /// Describes a type that's represented in the database using a String /// /// If you're reaching for this type, consider whether it'd be better to use an @@ -390,7 +423,7 @@ pub(crate) use impl_enum_type; pub trait DatabaseString: Sized { type Error: std::fmt::Display; - fn to_database_string(&self) -> &str; + fn to_database_string(&self) -> Cow; fn from_database_string(s: &str) -> Result; } @@ -398,16 +431,18 @@ use anyhow::anyhow; use nexus_types::external_api::shared::FleetRole; use nexus_types::external_api::shared::ProjectRole; use nexus_types::external_api::shared::SiloRole; +use std::borrow::Cow; impl DatabaseString for FleetRole { type Error = anyhow::Error; - fn to_database_string(&self) -> &str { + fn to_database_string(&self) -> Cow { match self { FleetRole::Admin => "admin", FleetRole::Collaborator => "collaborator", FleetRole::Viewer => "viewer", } + .into() } // WARNING: if you're considering changing this (including removing @@ -426,12 +461,13 @@ impl DatabaseString for FleetRole { impl DatabaseString for SiloRole { type Error = anyhow::Error; - fn to_database_string(&self) -> &str { + fn to_database_string(&self) -> Cow { match self { SiloRole::Admin => "admin", SiloRole::Collaborator => "collaborator", SiloRole::Viewer => "viewer", } + .into() } // WARNING: if you're considering changing this (including removing @@ -450,12 +486,13 @@ impl DatabaseString for SiloRole { impl DatabaseString for ProjectRole { type Error = anyhow::Error; - fn to_database_string(&self) -> &str { + fn to_database_string(&self) -> Cow { match self { ProjectRole::Admin => "admin", ProjectRole::Collaborator => "collaborator", ProjectRole::Viewer => "viewer", } + .into() } // WARNING: if you're considering changing this (including removing @@ -667,7 +704,7 @@ mod tests { // Serialize the variant. Verify that we can deserialize the thing // we just got back. let serialized = variant.to_database_string(); - let deserialized = T::from_database_string(serialized) + let deserialized = T::from_database_string(&serialized) .unwrap_or_else(|_| { panic!( "failed to deserialize the string {:?}, which we \ diff --git a/nexus/db-model/src/role_assignment.rs b/nexus/db-model/src/role_assignment.rs index 6ac10ffd7fa..6523e9e1f0d 100644 --- a/nexus/db-model/src/role_assignment.rs +++ b/nexus/db-model/src/role_assignment.rs @@ -118,6 +118,7 @@ where mod tests { use super::*; use omicron_common::api::external::ResourceType; + use std::borrow::Cow; #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq)] #[serde(rename_all = "kebab-case")] @@ -128,7 +129,7 @@ mod tests { impl crate::DatabaseString for DummyRoles { type Error = anyhow::Error; - fn to_database_string(&self) -> &str { + fn to_database_string(&self) -> Cow { unimplemented!() } diff --git a/nexus/db-model/src/vpc_firewall_rule.rs b/nexus/db-model/src/vpc_firewall_rule.rs index faeec7d93f4..b59c5d1481c 100644 --- a/nexus/db-model/src/vpc_firewall_rule.rs +++ b/nexus/db-model/src/vpc_firewall_rule.rs @@ -2,7 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use super::{L4PortRange, SqlU16, impl_enum_wrapper}; +use super::{ + DatabaseString, L4PortRange, SqlU16, impl_enum_wrapper, impl_from_sql_text, +}; use db_macros::Resource; use diesel::backend::Backend; use diesel::deserialize::{self, FromSql}; @@ -14,8 +16,10 @@ use nexus_types::identity::Resource; use omicron_common::api::external; use serde::Deserialize; use serde::Serialize; +use std::borrow::Cow; use std::collections::HashSet; use std::io::Write; +use std::str::FromStr; use uuid::Uuid; impl_enum_wrapper!( @@ -76,32 +80,20 @@ pub struct VpcFirewallRuleTarget(pub external::VpcFirewallRuleTarget); NewtypeFrom! { () pub struct VpcFirewallRuleTarget(external::VpcFirewallRuleTarget); } NewtypeDeref! { () pub struct VpcFirewallRuleTarget(external::VpcFirewallRuleTarget); } -impl ToSql for VpcFirewallRuleTarget { - fn to_sql<'a>( - &'a self, - out: &mut serialize::Output<'a, '_, Pg>, - ) -> serialize::Result { - >::to_sql( - &self.0.to_string(), - &mut out.reborrow(), - ) +impl DatabaseString for VpcFirewallRuleTarget { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() } -} -// Deserialize the "VpcFirewallRuleTarget" object from SQL TEXT. -impl FromSql for VpcFirewallRuleTarget -where - DB: Backend, - String: FromSql, -{ - fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result { - Ok(VpcFirewallRuleTarget( - String::from_sql(bytes)? - .parse::()?, - )) + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) } } +impl_from_sql_text!(VpcFirewallRuleTarget); + /// Newtype wrapper around [`external::VpcFirewallRuleHostFilter`] so we can derive /// diesel traits for it #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] @@ -111,32 +103,20 @@ pub struct VpcFirewallRuleHostFilter(pub external::VpcFirewallRuleHostFilter); NewtypeFrom! { () pub struct VpcFirewallRuleHostFilter(external::VpcFirewallRuleHostFilter); } NewtypeDeref! { () pub struct VpcFirewallRuleHostFilter(external::VpcFirewallRuleHostFilter); } -impl ToSql for VpcFirewallRuleHostFilter { - fn to_sql<'a>( - &'a self, - out: &mut serialize::Output<'a, '_, Pg>, - ) -> serialize::Result { - >::to_sql( - &self.0.to_string(), - &mut out.reborrow(), - ) +impl DatabaseString for VpcFirewallRuleHostFilter { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() } -} -// Deserialize the "VpcFirewallRuleHostFilter" object from SQL TEXT. -impl FromSql for VpcFirewallRuleHostFilter -where - DB: Backend, - String: FromSql, -{ - fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result { - Ok(VpcFirewallRuleHostFilter( - String::from_sql(bytes)? - .parse::()?, - )) + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) } } +impl_from_sql_text!(VpcFirewallRuleHostFilter); + /// Newtype wrapper around [`external::VpcFirewallRulePriority`] so we can derive /// diesel traits for it #[derive( diff --git a/nexus/db-model/src/vpc_route.rs b/nexus/db-model/src/vpc_route.rs index 5c6226de7ff..d664dae14b7 100644 --- a/nexus/db-model/src/vpc_route.rs +++ b/nexus/db-model/src/vpc_route.rs @@ -2,19 +2,17 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use super::{Name, impl_enum_wrapper}; +use super::{DatabaseString, Name, impl_enum_wrapper, impl_from_sql_text}; use chrono::{DateTime, Utc}; use db_macros::Resource; -use diesel::backend::Backend; -use diesel::deserialize::{self, FromSql}; -use diesel::pg::Pg; -use diesel::serialize::{self, ToSql}; use diesel::sql_types; use nexus_db_schema::schema::router_route; use nexus_types::external_api::params; use nexus_types::identity::Resource; use omicron_common::api::external; +use std::borrow::Cow; use std::io::Write; +use std::str::FromStr; use uuid::Uuid; impl_enum_wrapper!( @@ -34,30 +32,20 @@ impl_enum_wrapper!( #[diesel(sql_type = sql_types::Text)] pub struct RouteTarget(pub external::RouteTarget); -impl ToSql for RouteTarget { - fn to_sql<'a>( - &'a self, - out: &mut serialize::Output<'a, '_, Pg>, - ) -> serialize::Result { - >::to_sql( - &self.0.to_string(), - &mut out.reborrow(), - ) +impl DatabaseString for RouteTarget { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() } -} -impl FromSql for RouteTarget -where - DB: Backend, - String: FromSql, -{ - fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result { - Ok(RouteTarget( - String::from_sql(bytes)?.parse::()?, - )) + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) } } +impl_from_sql_text!(RouteTarget); + #[derive(Clone, Debug, AsExpression, FromSqlRow)] #[diesel(sql_type = sql_types::Text)] pub struct RouteDestination(pub external::RouteDestination); @@ -72,30 +60,20 @@ impl RouteDestination { } } -impl ToSql for RouteDestination { - fn to_sql<'a>( - &'a self, - out: &mut serialize::Output<'a, '_, Pg>, - ) -> serialize::Result { - >::to_sql( - &self.0.to_string(), - &mut out.reborrow(), - ) +impl DatabaseString for RouteDestination { + type Error = ::Err; + + fn to_database_string(&self) -> Cow { + self.0.to_string().into() } -} -impl FromSql for RouteDestination -where - DB: Backend, - String: FromSql, -{ - fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result { - Ok(RouteDestination::new( - String::from_sql(bytes)?.parse::()?, - )) + fn from_database_string(s: &str) -> Result { + s.parse::().map(Self) } } +impl_from_sql_text!(RouteDestination); + #[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)] #[diesel(table_name = router_route)] pub struct RouterRoute { diff --git a/nexus/tests/integration_tests/role_assignments.rs b/nexus/tests/integration_tests/role_assignments.rs index 7cdb952ad77..c11c0e1fb34 100644 --- a/nexus/tests/integration_tests/role_assignments.rs +++ b/nexus/tests/integration_tests/role_assignments.rs @@ -467,10 +467,10 @@ async fn run_test( .parsed_body() .unwrap(); new_policy.role_assignments.sort_by_key(|r| { - (r.identity_id, r.role_name.to_database_string().to_owned()) + (r.identity_id, r.role_name.to_database_string().into_owned()) }); updated_policy.role_assignments.sort_by_key(|r| { - (r.identity_id, r.role_name.to_database_string().to_owned()) + (r.identity_id, r.role_name.to_database_string().into_owned()) }); assert_eq!(updated_policy, new_policy);