&str-based FromSql impls for firewall/route types

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<sql_types::Text, Pg> for L4PortRange {
-    fn to_sql<'a>(
-        &'a self,
-        out: &mut serialize::Output<'a, '_, Pg>,
-    ) -> serialize::Result {
-        <String as ToSql<sql_types::Text, Pg>>::to_sql(
-            &self.0.to_string(),
-            &mut out.reborrow(),
-        )
+impl DatabaseString for L4PortRange {
+    type Error = <external::L4PortRange as FromStr>::Err;
+
+    fn to_database_string(&self) -> Cow<str> {
+        self.0.to_string().into()
     }
-}
 
-// Deserialize the "L4PortRange" object from SQL INT4.
-impl<DB> FromSql<sql_types::Text, DB> for L4PortRange
-where
-    DB: Backend,
-    String: FromSql<sql_types::Text, DB>,
-{
-    fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result<Self> {
-        String::from_sql(bytes)?.parse().map(L4PortRange).map_err(|e| e.into())
+    fn from_database_string(s: &str) -> Result<Self, Self::Error> {
+        s.parse::<external::L4PortRange>().map(Self)
     }
 }
+
+impl_from_sql_text!(L4PortRange);

nexus/db-model/src/lib.rs

@@ -373,6 +373,39 @@ macro_rules! impl_enum_type {
 
 pub(crate) use impl_enum_type;
 
+/// Automatically implement `FromSql<Text, _>` and `ToSql<Text, _>` 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 {
+                <str as ::diesel::serialize::ToSql<::diesel::sql_types::Text, ::diesel::pg::Pg>>::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<Self>
+            {
+                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,24 +423,26 @@ 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<str>;
     fn from_database_string(s: &str) -> Result<Self, Self::Error>;
 }
 
 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<str> {
         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<str> {
         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<str> {
         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 \

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<str> {
             unimplemented!()
         }
 

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<sql_types::Text, Pg> for VpcFirewallRuleTarget {
-    fn to_sql<'a>(
-        &'a self,
-        out: &mut serialize::Output<'a, '_, Pg>,
-    ) -> serialize::Result {
-        <String as ToSql<sql_types::Text, Pg>>::to_sql(
-            &self.0.to_string(),
-            &mut out.reborrow(),
-        )
+impl DatabaseString for VpcFirewallRuleTarget {
+    type Error = <external::VpcFirewallRuleTarget as FromStr>::Err;
+
+    fn to_database_string(&self) -> Cow<str> {
+        self.0.to_string().into()
     }
-}
 
-// Deserialize the "VpcFirewallRuleTarget" object from SQL TEXT.
-impl<DB> FromSql<sql_types::Text, DB> for VpcFirewallRuleTarget
-where
-    DB: Backend,
-    String: FromSql<sql_types::Text, DB>,
-{
-    fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result<Self> {
-        Ok(VpcFirewallRuleTarget(
-            String::from_sql(bytes)?
-                .parse::<external::VpcFirewallRuleTarget>()?,
-        ))
+    fn from_database_string(s: &str) -> Result<Self, Self::Error> {
+        s.parse::<external::VpcFirewallRuleTarget>().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<sql_types::Text, Pg> for VpcFirewallRuleHostFilter {
-    fn to_sql<'a>(
-        &'a self,
-        out: &mut serialize::Output<'a, '_, Pg>,
-    ) -> serialize::Result {
-        <String as ToSql<sql_types::Text, Pg>>::to_sql(
-            &self.0.to_string(),
-            &mut out.reborrow(),
-        )
+impl DatabaseString for VpcFirewallRuleHostFilter {
+    type Error = <external::VpcFirewallRuleHostFilter as FromStr>::Err;
+
+    fn to_database_string(&self) -> Cow<str> {
+        self.0.to_string().into()
     }
-}
 
-// Deserialize the "VpcFirewallRuleHostFilter" object from SQL TEXT.
-impl<DB> FromSql<sql_types::Text, DB> for VpcFirewallRuleHostFilter
-where
-    DB: Backend,
-    String: FromSql<sql_types::Text, DB>,
-{
-    fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result<Self> {
-        Ok(VpcFirewallRuleHostFilter(
-            String::from_sql(bytes)?
-                .parse::<external::VpcFirewallRuleHostFilter>()?,
-        ))
+    fn from_database_string(s: &str) -> Result<Self, Self::Error> {
+        s.parse::<external::VpcFirewallRuleHostFilter>().map(Self)
     }
 }
 
+impl_from_sql_text!(VpcFirewallRuleHostFilter);
+
 /// Newtype wrapper around [`external::VpcFirewallRulePriority`] so we can derive
 /// diesel traits for it
 #[derive(

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<sql_types::Text, Pg> for RouteTarget {
-    fn to_sql<'a>(
-        &'a self,
-        out: &mut serialize::Output<'a, '_, Pg>,
-    ) -> serialize::Result {
-        <String as ToSql<sql_types::Text, Pg>>::to_sql(
-            &self.0.to_string(),
-            &mut out.reborrow(),
-        )
+impl DatabaseString for RouteTarget {
+    type Error = <external::RouteTarget as FromStr>::Err;
+
+    fn to_database_string(&self) -> Cow<str> {
+        self.0.to_string().into()
     }
-}
 
-impl<DB> FromSql<sql_types::Text, DB> for RouteTarget
-where
-    DB: Backend,
-    String: FromSql<sql_types::Text, DB>,
-{
-    fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result<Self> {
-        Ok(RouteTarget(
-            String::from_sql(bytes)?.parse::<external::RouteTarget>()?,
-        ))
+    fn from_database_string(s: &str) -> Result<Self, Self::Error> {
+        s.parse::<external::RouteTarget>().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<sql_types::Text, Pg> for RouteDestination {
-    fn to_sql<'a>(
-        &'a self,
-        out: &mut serialize::Output<'a, '_, Pg>,
-    ) -> serialize::Result {
-        <String as ToSql<sql_types::Text, Pg>>::to_sql(
-            &self.0.to_string(),
-            &mut out.reborrow(),
-        )
+impl DatabaseString for RouteDestination {
+    type Error = <external::RouteDestination as FromStr>::Err;
+
+    fn to_database_string(&self) -> Cow<str> {
+        self.0.to_string().into()
     }
-}
 
-impl<DB> FromSql<sql_types::Text, DB> for RouteDestination
-where
-    DB: Backend,
-    String: FromSql<sql_types::Text, DB>,
-{
-    fn from_sql(bytes: DB::RawValue<'_>) -> deserialize::Result<Self> {
-        Ok(RouteDestination::new(
-            String::from_sql(bytes)?.parse::<external::RouteDestination>()?,
-        ))
+    fn from_database_string(s: &str) -> Result<Self, Self::Error> {
+        s.parse::<external::RouteDestination>().map(Self)
     }
 }
 
+impl_from_sql_text!(RouteDestination);
+
 #[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)]
 #[diesel(table_name = router_route)]
 pub struct RouterRoute {

nexus/tests/integration_tests/role_assignments.rs

@@ -467,10 +467,10 @@ async fn run_test<T: RoleAssignmentTest>(
             .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);