Skip to content

Commit 3767f83

Browse files
david-crespodavid-crespo
authored
PoC for moving enums from common -> Nexus db::model (#776)
* alternate impl_enum_type that owns the enum itself (compiles!) * now move VpcRouter and VpcRouterKind into views (compiles, tests pass) * convert DatasetKind, very simple * Into -> From because it's better! * macro rename and comments so I can merge the intermediate state * make diff slightly nicer
1 parent 383ca78 commit 3767f83

File tree

10 files changed

+163
-84
lines changed

10 files changed

+163
-84
lines changed

‎common/src/api/external/mod.rs‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,27 +1154,6 @@ impl FromStr for IpNet {
11541154
}
11551155
}
11561156

1157-
#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)]
1158-
#[serde(rename_all = "snake_case")]
1159-
pub enum VpcRouterKind {
1160-
System,
1161-
Custom,
1162-
}
1163-
1164-
/// A VPC router defines a series of rules that indicate where traffic
1165-
/// should be sent depending on its destination.
1166-
#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)]
1167-
pub struct VpcRouter {
1168-
/// common identifying metadata
1169-
#[serde(flatten)]
1170-
pub identity: IdentityMetadata,
1171-
1172-
pub kind: VpcRouterKind,
1173-
1174-
/// The VPC to which the router belongs.
1175-
pub vpc_id: Uuid,
1176-
}
1177-
11781157
/// A `RouteTarget` describes the possible locations that traffic matching a
11791158
/// route destination can be sent.
11801159
#[derive(

‎nexus/src/db/datastore.rs‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,7 @@ impl DataStore {
299299
// We look for valid datasets (non-deleted crucible datasets).
300300
.filter(dsl::size_used.is_not_null())
301301
.filter(dsl::time_deleted.is_null())
302-
.filter(dsl::kind.eq(DatasetKind(
303-
crate::internal_api::params::DatasetKind::Crucible,
304-
)))
302+
.filter(dsl::kind.eq(DatasetKind::Crucible))
305303
.order(dsl::size_used.asc())
306304
// TODO: We admittedly don't actually *fail* any request for
307305
// running out of space - we try to send the request down to
@@ -3308,7 +3306,9 @@ mod test {
33083306
use crate::authz;
33093307
use crate::db::explain::ExplainableAsync;
33103308
use crate::db::identity::Resource;
3311-
use crate::db::model::{ConsoleSession, Organization, Project};
3309+
use crate::db::model::{
3310+
ConsoleSession, DatasetKind, Organization, Project,
3311+
};
33123312
use crate::external_api::params;
33133313
use chrono::{Duration, Utc};
33143314
use nexus_test_utils::db::test_setup_database;
@@ -3473,12 +3473,11 @@ mod test {
34733473
let dataset_count = REGION_REDUNDANCY_THRESHOLD * 2;
34743474
let bogus_addr =
34753475
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
3476-
let kind =
3477-
DatasetKind(crate::internal_api::params::DatasetKind::Crucible);
34783476
let dataset_ids: Vec<Uuid> =
34793477
(0..dataset_count).map(|_| Uuid::new_v4()).collect();
34803478
for id in &dataset_ids {
3481-
let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone());
3479+
let dataset =
3480+
Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible);
34823481
datastore.dataset_upsert(dataset).await.unwrap();
34833482
}
34843483

@@ -3549,12 +3548,11 @@ mod test {
35493548
let dataset_count = REGION_REDUNDANCY_THRESHOLD;
35503549
let bogus_addr =
35513550
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
3552-
let kind =
3553-
DatasetKind(crate::internal_api::params::DatasetKind::Crucible);
35543551
let dataset_ids: Vec<Uuid> =
35553552
(0..dataset_count).map(|_| Uuid::new_v4()).collect();
35563553
for id in &dataset_ids {
3557-
let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone());
3554+
let dataset =
3555+
Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible);
35583556
datastore.dataset_upsert(dataset).await.unwrap();
35593557
}
35603558

@@ -3610,12 +3608,11 @@ mod test {
36103608
let dataset_count = REGION_REDUNDANCY_THRESHOLD - 1;
36113609
let bogus_addr =
36123610
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
3613-
let kind =
3614-
DatasetKind(crate::internal_api::params::DatasetKind::Crucible);
36153611
let dataset_ids: Vec<Uuid> =
36163612
(0..dataset_count).map(|_| Uuid::new_v4()).collect();
36173613
for id in &dataset_ids {
3618-
let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone());
3614+
let dataset =
3615+
Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible);
36193616
datastore.dataset_upsert(dataset).await.unwrap();
36203617
}
36213618

@@ -3658,12 +3655,11 @@ mod test {
36583655
let dataset_count = REGION_REDUNDANCY_THRESHOLD;
36593656
let bogus_addr =
36603657
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
3661-
let kind =
3662-
DatasetKind(crate::internal_api::params::DatasetKind::Crucible);
36633658
let dataset_ids: Vec<Uuid> =
36643659
(0..dataset_count).map(|_| Uuid::new_v4()).collect();
36653660
for id in &dataset_ids {
3666-
let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone());
3661+
let dataset =
3662+
Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible);
36673663
datastore.dataset_upsert(dataset).await.unwrap();
36683664
}
36693665

‎nexus/src/db/model.rs‎

Lines changed: 101 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,19 @@ use uuid::Uuid;
4141

4242
// TODO: Break up types into multiple files
4343

44-
/// This macro implements serialization and deserialization of an enum type
45-
/// from our database into our model types.
46-
/// See [`VpcRouterKindEnum`] and [`VpcRouterKind`] for a sample usage
47-
macro_rules! impl_enum_type {
44+
// TODO: The existence of both impl_enum_type and impl_enum_wrapper is a
45+
// temporary state of affairs while we do the work of converting uses of
46+
// impl_enum_wrapper to impl_enum_type. This is part of a broader initiative to
47+
// move types out of the common crate into Nexus where possible. See
48+
// https://github.com/oxidecomputer/omicron/issues/388
49+
50+
/// This macro implements serialization and deserialization of an enum type from
51+
/// our database into our model types. This version wraps an enum imported from
52+
/// the common crate in a struct so we can implement DB traits on it. We are
53+
/// moving those enum definitions into this file and using impl_enum_type
54+
/// instead, so eventually this macro will go away. See [`InstanceState`] for a
55+
/// sample usage.
56+
macro_rules! impl_enum_wrapper {
4857
(
4958
$(#[$enum_meta:meta])*
5059
pub struct $diesel_type:ident;
@@ -101,6 +110,70 @@ macro_rules! impl_enum_type {
101110
}
102111
}
103112

113+
/// This macro implements serialization and deserialization of an enum type from
114+
/// our database into our model types. See [`VpcRouterKindEnum`] and
115+
/// [`VpcRouterKind`] for a sample usage
116+
macro_rules! impl_enum_type {
117+
(
118+
$(#[$enum_meta:meta])*
119+
pub struct $diesel_type:ident;
120+
121+
$(#[$model_meta:meta])*
122+
pub enum $model_type:ident;
123+
$($enum_item:ident => $sql_value:literal)+
124+
) => {
125+
126+
$(#[$enum_meta])*
127+
pub struct $diesel_type;
128+
129+
$(#[$model_meta])*
130+
pub enum $model_type {
131+
$(
132+
$enum_item,
133+
)*
134+
}
135+
136+
impl<DB> ToSql<$diesel_type, DB> for $model_type
137+
where
138+
DB: Backend,
139+
{
140+
fn to_sql<W: std::io::Write>(
141+
&self,
142+
out: &mut serialize::Output<W, DB>,
143+
) -> serialize::Result {
144+
match self {
145+
$(
146+
$model_type::$enum_item => {
147+
out.write_all($sql_value)?
148+
}
149+
)*
150+
}
151+
Ok(IsNull::No)
152+
}
153+
}
154+
155+
impl<DB> FromSql<$diesel_type, DB> for $model_type
156+
where
157+
DB: Backend + for<'a> BinaryRawValue<'a>,
158+
{
159+
fn from_sql(bytes: RawValue<DB>) -> deserialize::Result<Self> {
160+
match DB::as_bytes(bytes) {
161+
$(
162+
$sql_value => {
163+
Ok($model_type::$enum_item)
164+
}
165+
)*
166+
_ => {
167+
Err(concat!("Unrecognized enum variant for ",
168+
stringify!{$model_type})
169+
.into())
170+
}
171+
}
172+
}
173+
}
174+
}
175+
}
176+
104177
/// Newtype wrapper around [external::Name].
105178
#[derive(
106179
Clone,
@@ -634,7 +707,7 @@ impl_enum_type!(
634707

635708
#[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq)]
636709
#[sql_type = "DatasetKindEnum"]
637-
pub struct DatasetKind(pub internal_api::params::DatasetKind);
710+
pub enum DatasetKind;
638711

639712
// Enum values
640713
Crucible => b"crucible"
@@ -644,7 +717,17 @@ impl_enum_type!(
644717

645718
impl From<internal_api::params::DatasetKind> for DatasetKind {
646719
fn from(k: internal_api::params::DatasetKind) -> Self {
647-
Self(k)
720+
match k {
721+
internal_api::params::DatasetKind::Crucible => {
722+
DatasetKind::Crucible
723+
}
724+
internal_api::params::DatasetKind::Cockroach => {
725+
DatasetKind::Cockroach
726+
}
727+
internal_api::params::DatasetKind::Clickhouse => {
728+
DatasetKind::Clickhouse
729+
}
730+
}
648731
}
649732
}
650733

@@ -687,7 +770,7 @@ impl Dataset {
687770
kind: DatasetKind,
688771
) -> Self {
689772
let size_used = match kind {
690-
DatasetKind(internal_api::params::DatasetKind::Crucible) => Some(0),
773+
DatasetKind::Crucible => Some(0),
691774
_ => None,
692775
};
693776
Self {
@@ -1067,7 +1150,7 @@ impl Into<internal::nexus::InstanceRuntimeState> for InstanceRuntimeState {
10671150
}
10681151
}
10691152

1070-
impl_enum_type!(
1153+
impl_enum_wrapper!(
10711154
#[derive(SqlType, Debug)]
10721155
#[postgres(type_name = "instance_state", type_schema = "public")]
10731156
pub struct InstanceStateEnum;
@@ -1587,9 +1670,9 @@ impl_enum_type!(
15871670
#[postgres(type_name = "vpc_router_kind", type_schema = "public")]
15881671
pub struct VpcRouterKindEnum;
15891672

1590-
#[derive(Clone, Debug, AsExpression, FromSqlRow)]
1673+
#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)]
15911674
#[sql_type = "VpcRouterKindEnum"]
1592-
pub struct VpcRouterKind(pub external::VpcRouterKind);
1675+
pub enum VpcRouterKind;
15931676

15941677
// Enum values
15951678
System => b"system"
@@ -1611,16 +1694,11 @@ impl VpcRouter {
16111694
pub fn new(
16121695
router_id: Uuid,
16131696
vpc_id: Uuid,
1614-
kind: external::VpcRouterKind,
1697+
kind: VpcRouterKind,
16151698
params: params::VpcRouterCreate,
16161699
) -> Self {
16171700
let identity = VpcRouterIdentity::new(router_id, params.identity);
1618-
Self {
1619-
identity,
1620-
vpc_id,
1621-
kind: VpcRouterKind(kind),
1622-
rcgen: Generation::new(),
1623-
}
1701+
Self { identity, vpc_id, kind, rcgen: Generation::new() }
16241702
}
16251703
}
16261704

@@ -1631,16 +1709,6 @@ impl DatastoreCollection<RouterRoute> for VpcRouter {
16311709
type CollectionIdColumn = router_route::dsl::router_id;
16321710
}
16331711

1634-
impl Into<external::VpcRouter> for VpcRouter {
1635-
fn into(self) -> external::VpcRouter {
1636-
external::VpcRouter {
1637-
identity: self.identity(),
1638-
vpc_id: self.vpc_id,
1639-
kind: self.kind.0,
1640-
}
1641-
}
1642-
}
1643-
16441712
#[derive(AsChangeset)]
16451713
#[table_name = "vpc_router"]
16461714
pub struct VpcRouterUpdate {
@@ -1659,7 +1727,7 @@ impl From<params::VpcRouterUpdate> for VpcRouterUpdate {
16591727
}
16601728
}
16611729

1662-
impl_enum_type!(
1730+
impl_enum_wrapper!(
16631731
#[derive(SqlType, Debug)]
16641732
#[postgres(type_name = "router_route_kind", type_schema = "public")]
16651733
pub struct RouterRouteKindEnum;
@@ -1807,7 +1875,7 @@ impl From<external::RouterRouteUpdateParams> for RouterRouteUpdate {
18071875
}
18081876
}
18091877

1810-
impl_enum_type!(
1878+
impl_enum_wrapper!(
18111879
#[derive(SqlType, Debug)]
18121880
#[postgres(type_name = "vpc_firewall_rule_status", type_schema = "public")]
18131881
pub struct VpcFirewallRuleStatusEnum;
@@ -1822,7 +1890,7 @@ impl_enum_type!(
18221890
NewtypeFrom! { () pub struct VpcFirewallRuleStatus(external::VpcFirewallRuleStatus); }
18231891
NewtypeDeref! { () pub struct VpcFirewallRuleStatus(external::VpcFirewallRuleStatus); }
18241892

1825-
impl_enum_type!(
1893+
impl_enum_wrapper!(
18261894
#[derive(SqlType, Debug)]
18271895
#[postgres(type_name = "vpc_firewall_rule_direction", type_schema = "public")]
18281896
pub struct VpcFirewallRuleDirectionEnum;
@@ -1837,7 +1905,7 @@ impl_enum_type!(
18371905
NewtypeFrom! { () pub struct VpcFirewallRuleDirection(external::VpcFirewallRuleDirection); }
18381906
NewtypeDeref! { () pub struct VpcFirewallRuleDirection(external::VpcFirewallRuleDirection); }
18391907

1840-
impl_enum_type!(
1908+
impl_enum_wrapper!(
18411909
#[derive(SqlType, Debug)]
18421910
#[postgres(type_name = "vpc_firewall_rule_action", type_schema = "public")]
18431911
pub struct VpcFirewallRuleActionEnum;
@@ -1852,7 +1920,7 @@ impl_enum_type!(
18521920
NewtypeFrom! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); }
18531921
NewtypeDeref! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); }
18541922

1855-
impl_enum_type!(
1923+
impl_enum_wrapper!(
18561924
#[derive(SqlType, Debug)]
18571925
#[postgres(type_name = "vpc_firewall_rule_protocol", type_schema = "public")]
18581926
pub struct VpcFirewallRuleProtocolEnum;
@@ -2252,7 +2320,7 @@ impl RoleAssignmentBuiltin {
22522320
}
22532321
}
22542322

2255-
impl_enum_type!(
2323+
impl_enum_wrapper!(
22562324
#[derive(SqlType, Debug, QueryId)]
22572325
#[postgres(type_name = "update_artifact_kind", type_schema = "public")]
22582326
pub struct UpdateArtifactKindEnum;

‎nexus/src/external_api/http_entrypoints.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ use crate::ServerContext;
1111
use super::{
1212
console_api, params,
1313
views::{
14-
Organization, Project, Rack, Role, Sled, Snapshot, User, Vpc, VpcSubnet,
14+
Organization, Project, Rack, Role, Sled, Snapshot, User, Vpc,
15+
VpcRouter, VpcSubnet,
1516
},
1617
};
1718
use crate::context::OpContext;
@@ -57,8 +58,6 @@ use omicron_common::api::external::RouterRouteUpdateParams;
5758
use omicron_common::api::external::Saga;
5859
use omicron_common::api::external::VpcFirewallRuleUpdateParams;
5960
use omicron_common::api::external::VpcFirewallRules;
60-
use omicron_common::api::external::VpcRouter;
61-
use omicron_common::api::external::VpcRouterKind;
6261
use ref_cast::RefCast;
6362
use schemars::JsonSchema;
6463
use serde::Deserialize;
@@ -1863,7 +1862,7 @@ async fn vpc_routers_post(
18631862
&path.organization_name,
18641863
&path.project_name,
18651864
&path.vpc_name,
1866-
&VpcRouterKind::Custom,
1865+
&db::model::VpcRouterKind::Custom,
18671866
&create_params.into_inner(),
18681867
)
18691868
.await?;

‎nexus/src/external_api/params.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,14 +186,14 @@ pub struct VpcSubnetUpdate {
186186

187187
// VPC ROUTERS
188188

189-
/// Create-time parameters for a [`VpcRouter`](omicron_common::api::external::VpcRouter)
189+
/// Create-time parameters for a [`VpcRouter`](crate::external_api::views::VpcRouter)
190190
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
191191
pub struct VpcRouterCreate {
192192
#[serde(flatten)]
193193
pub identity: IdentityMetadataCreateParams,
194194
}
195195

196-
/// Updateable properties of a [`VpcRouter`](omicron_common::api::external::VpcRouter)
196+
/// Updateable properties of a [`VpcRouter`](crate::external_api::views::VpcRouter)
197197
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
198198
pub struct VpcRouterUpdate {
199199
#[serde(flatten)]

0 commit comments

Comments
 (0)