Skip to content

Commit

Permalink
Centralize IDs to "si-id"
Browse files Browse the repository at this point in the history
This commit centralizes (mostly) all IDs in the codebase to a new crate
called "si-id". This work was continued from #4746 and original research
in a now-deleted branch, "nick/3b20c5e".

Methodology:

The new "id!" macro in "si-id" contains a superset of all methods and
types contained in the old macros. Yes, this means that the unsightly
"NONE" type is available for all ID types now. However, we've developed
good habits and will be able to splice out this usages in follow-up
work.

Potential future work:

- Clean-up ID call sites (e.g. reduce "pub use" statements)
- Migrate all IDs in "si-layer-cache" (were not reliant on a macro)
- Migrate all IDs in "module-index-server" (were not reliant on a macro)
- Remove all usages of the third party ulid crate and go through "si-id"
- Add another macro in "si-id" that contains the "NONE" type (the main
  macro should not have it)
- Add another macro in "si-id" that contains the postgres types (the
  main macro should not have it)

Signed-off-by: Nick Gerace <[email protected]>
  • Loading branch information
nickgerace committed Dec 9, 2024
1 parent ab4d461 commit a392382
Show file tree
Hide file tree
Showing 115 changed files with 544 additions and 1,083 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ members = [
"lib/si-firecracker",
"lib/si-frontend-types-rs",
"lib/si-hash",
"lib/si-id",
"lib/si-layer-cache",
"lib/si-pkg",
"lib/si-pool-noodle",
Expand Down
1 change: 1 addition & 0 deletions lib/dal/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ rust_library(
"//lib/si-events-rs:si-events",
"//lib/si-frontend-types-rs:si-frontend-types",
"//lib/si-hash:si-hash",
"//lib/si-id:si-id",
"//lib/si-layer-cache:si-layer-cache",
"//lib/si-pkg:si-pkg",
"//lib/si-runtime-rs:si-runtime",
Expand Down
1 change: 1 addition & 0 deletions lib/dal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ si-data-pg = { path = "../../lib/si-data-pg" }
si-events = { path = "../../lib/si-events-rs" }
si-frontend-types = { path = "../../lib/si-frontend-types-rs" }
si-hash = { path = "../../lib/si-hash" }
si-id = { path = "../../lib/si-id" }
si-layer-cache = { path = "../../lib/si-layer-cache" }
si-pkg = { path = "../../lib/si-pkg" }
si-runtime = { path = "../../lib/si-runtime-rs" }
Expand Down
19 changes: 3 additions & 16 deletions lib/dal/src/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
attribute::value::{AttributeValueError, DependentValueGraph},
component::inferred_connection_graph::InferredConnectionGraphError,
func::FuncExecutionPk,
id, implement_add_edge_to,
implement_add_edge_to,
job::definition::ActionJob,
workspace_snapshot::node_weight::{
category_node_weight::CategoryNodeKind, ActionNodeWeight, NodeWeight, NodeWeightError,
Expand Down Expand Up @@ -65,21 +65,8 @@ pub enum ActionError {

pub type ActionResult<T> = Result<T, ActionError>;

id!(ActionId);

impl From<ActionId> for si_events::ActionId {
fn from(value: ActionId) -> Self {
value.into_inner().into()
}
}

id!(ActionPrototypeId);

impl From<ActionPrototypeId> for si_events::ActionPrototypeId {
fn from(value: ActionPrototypeId) -> Self {
value.into_inner().into()
}
}
pub use si_id::ActionId;
pub use si_id::ActionPrototypeId;

#[derive(Debug, Copy, Clone, Deserialize, Serialize, EnumDiscriminants, PartialEq, Eq, Display)]
#[strum_discriminants(derive(strum::Display, Serialize, Deserialize))]
Expand Down
10 changes: 2 additions & 8 deletions lib/dal/src/attribute/prototype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::workspace_snapshot::node_weight::{
};
use crate::workspace_snapshot::WorkspaceSnapshotError;
use crate::{
attribute::prototype::argument::AttributePrototypeArgumentId, id, implement_add_edge_to,
attribute::prototype::argument::AttributePrototypeArgumentId, implement_add_edge_to,
AttributeValue, AttributeValueId, ComponentId, DalContext, FuncId, HelperError, InputSocketId,
OutputSocketId, PropId, SchemaVariant, SchemaVariantError, SchemaVariantId, Timestamp,
TransactionsError,
Expand Down Expand Up @@ -102,13 +102,7 @@ pub enum AttributePrototypeEventualParent {
SchemaVariantFromProp(SchemaVariantId, PropId),
}

id!(AttributePrototypeId);

impl From<AttributePrototypeId> for si_events::AttributePrototypeId {
fn from(value: AttributePrototypeId) -> Self {
si_events::AttributePrototypeId::from_raw_id(value.into())
}
}
pub use si_id::AttributePrototypeId;

#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
pub struct AttributePrototype {
Expand Down
16 changes: 2 additions & 14 deletions lib/dal/src/attribute/prototype/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::workspace_snapshot::node_weight::traits::SiNodeWeight;
use crate::{
change_set::ChangeSetError,
func::argument::{FuncArgument, FuncArgumentError, FuncArgumentId},
id, implement_add_edge_to,
implement_add_edge_to,
socket::input::InputSocketId,
workspace_snapshot::{
content_address::ContentAddressDiscriminants,
Expand All @@ -40,19 +40,7 @@ pub use crate::workspace_snapshot::node_weight::attribute_prototype_argument_nod
pub mod static_value;
pub mod value_source;

id!(AttributePrototypeArgumentId);

impl From<si_events::AttributePrototypeArgumentId> for AttributePrototypeArgumentId {
fn from(value: si_events::AttributePrototypeArgumentId) -> Self {
Self(value.into_raw_id())
}
}

impl From<AttributePrototypeArgumentId> for si_events::AttributePrototypeArgumentId {
fn from(value: AttributePrototypeArgumentId) -> Self {
Self::from_raw_id(value.0)
}
}
pub use si_id::AttributePrototypeArgumentId;

#[remain::sorted]
#[derive(Error, Debug)]
Expand Down
3 changes: 1 addition & 2 deletions lib/dal/src/attribute/prototype/argument/static_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::sync::Arc;
use serde::{Deserialize, Serialize};

use crate::{
id,
layer_db_types::{StaticArgumentValueContent, StaticArgumentValueContentV1},
workspace_snapshot::{
content_address::ContentAddress, node_weight::NodeWeight, WorkspaceSnapshotError,
Expand All @@ -13,7 +12,7 @@ use crate::{

use super::AttributePrototypeArgumentResult;

id!(StaticArgumentValueId);
pub use si_id::StaticArgumentValueId;

#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
pub struct StaticArgumentValue {
Expand Down
10 changes: 2 additions & 8 deletions lib/dal/src/attribute/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use crate::workspace_snapshot::node_weight::{
};
use crate::workspace_snapshot::{serde_value_to_string_type, WorkspaceSnapshotError};
use crate::{
id, implement_add_edge_to, AttributePrototype, AttributePrototypeId, Component, ComponentError,
implement_add_edge_to, AttributePrototype, AttributePrototypeId, Component, ComponentError,
ComponentId, DalContext, Func, FuncError, FuncId, HelperError, InputSocket, InputSocketId,
OutputSocket, OutputSocketId, Prop, PropId, PropKind, Secret, SecretError, TransactionsError,
};
Expand Down Expand Up @@ -232,13 +232,7 @@ impl From<ComponentError> for AttributeValueError {

pub type AttributeValueResult<T> = Result<T, AttributeValueError>;

id!(AttributeValueId);

impl From<AttributeValueId> for si_events::AttributeValueId {
fn from(value: AttributeValueId) -> Self {
value.into_inner().into()
}
}
pub use si_id::AttributeValueId;

#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
pub struct AttributeValue {
Expand Down
22 changes: 11 additions & 11 deletions lib/dal/src/audit_logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ pub(crate) async fn publish_pending(
tracker.to_owned(),
source_stream.stream().await?,
source_stream.subject_for_audit_log(
workspace_id.into(),
ctx.change_set_id().into(),
workspace_id,
ctx.change_set_id(),
match override_event_session_id {
Some(override_id) => override_id,
None => ctx.event_session_id(),
},
),
destination_stream.publishing_subject_for_workspace(workspace_id.into()),
destination_stream.publishing_subject_for_workspace(workspace_id),
)
.await?;

Expand Down Expand Up @@ -208,13 +208,13 @@ pub(crate) async fn write(
};

let destination_change_set_id =
override_destination_change_set_id.unwrap_or(ctx.change_set_id().into());
override_destination_change_set_id.unwrap_or(ctx.change_set_id());

let pending_events_stream = PendingEventsStream::get_or_create(ctx.jetstream_context()).await?;
pending_events_stream
.publish_audit_log(
workspace_id.into(),
ctx.change_set_id().into(),
workspace_id,
ctx.change_set_id(),
ctx.event_session_id(),
&AuditLog::new(
ctx.events_actor(),
Expand All @@ -240,8 +240,8 @@ pub(crate) async fn write_final_message(ctx: &DalContext) -> Result<()> {
let pending_events_stream = PendingEventsStream::get_or_create(ctx.jetstream_context()).await?;
pending_events_stream
.publish_audit_log_final_message(
workspace_id.into(),
ctx.change_set_id().into(),
workspace_id,
ctx.change_set_id(),
ctx.event_session_id(),
)
.await?;
Expand All @@ -258,7 +258,7 @@ pub async fn list(
let change_set_id = ctx.change_set_id();

let change_set_ids = {
let mut change_set_ids = vec![change_set_id.into()];
let mut change_set_ids = vec![change_set_id];
if ctx
.get_workspace_default_change_set_id()
.await
Expand All @@ -273,15 +273,15 @@ pub async fn list(
.await
.map_err(Box::new)?
{
change_set_ids.push(applied_change_set.id.into());
change_set_ids.push(applied_change_set.id);
}
}
change_set_ids
};

Ok(AuditLogRow::list(
audit_database_context,
workspace_id.into(),
workspace_id,
change_set_ids,
size,
)
Expand Down
4 changes: 2 additions & 2 deletions lib/dal/src/authentication_prototype.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use serde::{Deserialize, Serialize};

use crate::{id, FuncId, SchemaVariantId};
use crate::{FuncId, SchemaVariantId};

id!(AuthenticationPrototypeId);
pub use si_id::AuthenticationPrototypeId;

// TODO(nick): remove this once import can just create the edge.
#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
Expand Down
28 changes: 14 additions & 14 deletions lib/dal/src/billing_publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ pub(crate) async fn for_head_change_set_pointer_update(

let workspace_id = change_set.workspace_id()?;
let event = BillingEvent {
workspace_id: workspace_id.into(),
workspace_id,
workspace_snapshot_address: change_set.workspace_snapshot_address,
event_timestamp: Utc::now(),
change_set_status: change_set.status.into(),
change_set_id: change_set.id.into(),
change_set_id: change_set.id,
merge_requested_by_user_id: change_set.merge_requested_by_user_id.map(Into::into),

resource_count: Some(resource_count),
Expand Down Expand Up @@ -133,11 +133,11 @@ pub(crate) async fn for_change_set_status_update(

let workspace_id = change_set.workspace_id()?;
let event = BillingEvent {
workspace_id: workspace_id.into(),
workspace_id,
workspace_snapshot_address: change_set.workspace_snapshot_address,
event_timestamp: Utc::now(),
change_set_status: change_set.status.into(),
change_set_id: change_set.id.into(),
change_set_id: change_set.id,
merge_requested_by_user_id: change_set.merge_requested_by_user_id.map(Into::into),

resource_count: Some(resource_count),
Expand Down Expand Up @@ -189,19 +189,19 @@ pub(crate) async fn for_resource_create(

let workspace_id = change_set.workspace_id()?;
let event = BillingEvent {
workspace_id: workspace_id.into(),
workspace_id,
workspace_snapshot_address: change_set.workspace_snapshot_address,
event_timestamp: Utc::now(),
change_set_status: change_set.status.into(),
change_set_id: change_set.id.into(),
change_set_id: change_set.id,
merge_requested_by_user_id: change_set.merge_requested_by_user_id.map(Into::into),

resource_count: None,

component_id: Some(component_id.into()),
component_id: Some(component_id),
component_name: Some(component_name),
schema_variant_id: Some(schema_variant_id.into()),
schema_id: Some(schema_id.into()),
schema_variant_id: Some(schema_variant_id),
schema_id: Some(schema_id),
schema_name: Some(schema_name),
func_run_id: Some(func_run_id),

Expand Down Expand Up @@ -245,19 +245,19 @@ pub(crate) async fn for_resource_delete(

let workspace_id = change_set.workspace_id()?;
let event = BillingEvent {
workspace_id: workspace_id.into(),
workspace_id,
workspace_snapshot_address: change_set.workspace_snapshot_address,
event_timestamp: Utc::now(),
change_set_status: change_set.status.into(),
change_set_id: change_set.id.into(),
change_set_id: change_set.id,
merge_requested_by_user_id: change_set.merge_requested_by_user_id.map(Into::into),

resource_count: None,

component_id: Some(component_id.into()),
component_id: Some(component_id),
component_name: Some(component_name),
schema_variant_id: Some(schema_variant_id.into()),
schema_id: Some(schema_id.into()),
schema_variant_id: Some(schema_variant_id),
schema_id: Some(schema_id),
schema_name: Some(schema_name),
func_run_id: Some(func_run_id),

Expand Down
5 changes: 2 additions & 3 deletions lib/dal/src/cached_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ use tokio::task::JoinSet;
use ulid::Ulid;

use crate::{
pk,
slow_rt::{self, SlowRuntimeError},
ComponentType, DalContext, SchemaId, TransactionsError,
};
use module_index_client::{ModuleDetailsResponse, ModuleIndexClient, ModuleIndexClientError};
use si_data_pg::{PgError, PgRow};
use si_pkg::{SiPkg, SiPkgError};

pk!(CachedModuleId);
pub use si_id::CachedModuleId;

#[remain::sorted]
#[derive(Error, Debug)]
Expand Down Expand Up @@ -67,7 +66,7 @@ pub struct CachedModule {
impl From<CachedModule> for si_frontend_types::UninstalledVariant {
fn from(value: CachedModule) -> Self {
Self {
schema_id: value.schema_id.into(),
schema_id: value.schema_id,
schema_name: value.schema_name,
display_name: value.display_name,
category: value.category,
Expand Down
Loading

0 comments on commit a392382

Please sign in to comment.