diff --git a/lib/dal-test/src/helpers/change_set.rs b/lib/dal-test/src/helpers/change_set.rs index c8249d57d9..52410f93f8 100644 --- a/lib/dal-test/src/helpers/change_set.rs +++ b/lib/dal-test/src/helpers/change_set.rs @@ -140,9 +140,7 @@ impl ChangeSetTestHelpers { async fn blocking_commit(ctx: &DalContext) -> Result<()> { // TODO(nick,brit): we need to expand Brit's 409 conflict work to work with blocking commits // too rather than evaluating an optional set of conflicts. - match ctx.blocking_commit().await? { - Some(conflicts) => Err(eyre!("found conflicts after commit: {conflicts:?}")), - None => Ok(()), - } + ctx.blocking_commit().await?; + Ok(()) } } diff --git a/lib/dal/examples/rebase/main.rs b/lib/dal/examples/rebase/main.rs index 966f1960b1..c3ce2bbe06 100644 --- a/lib/dal/examples/rebase/main.rs +++ b/lib/dal/examples/rebase/main.rs @@ -21,23 +21,12 @@ fn main() -> Result<()> { let to_rebase_path = args.get(1).expect(USAGE); let onto_path = args.get(2).expect(USAGE); - let mut to_rebase_graph = load_snapshot_graph(&to_rebase_path)?; + let to_rebase_graph = load_snapshot_graph(&to_rebase_path)?; let onto_graph = load_snapshot_graph(&onto_path)?; - let to_rebase_vector_clock_id = dbg!(to_rebase_graph - .max_recently_seen_clock_id(None) - .expect("Unable to find a vector clock id in to_rebase")); - let onto_vector_clock_id = dbg!(onto_graph - .max_recently_seen_clock_id(None) - .expect("Unable to find a vector clock id in onto")); + let updates = to_rebase_graph.detect_updates(&onto_graph); - let conflicts_and_updates = to_rebase_graph.detect_conflicts_and_updates( - dbg!(to_rebase_vector_clock_id), - &onto_graph, - onto_vector_clock_id, - )?; - - for update in &conflicts_and_updates.updates { + for update in &updates { dbg!(update); } diff --git a/lib/dal/src/attribute/prototype.rs b/lib/dal/src/attribute/prototype.rs index e6f944d567..64ebb915af 100644 --- a/lib/dal/src/attribute/prototype.rs +++ b/lib/dal/src/attribute/prototype.rs @@ -350,7 +350,6 @@ impl AttributePrototype { workspace_snapshot .remove_edge( - ctx.vector_clock_id()?, attribute_prototype_idx, current_func_node_idx, EdgeWeightKindDiscriminants::Use, diff --git a/lib/dal/src/attribute/prototype/argument.rs b/lib/dal/src/attribute/prototype/argument.rs index 8462151503..7c4987a4a1 100644 --- a/lib/dal/src/attribute/prototype/argument.rs +++ b/lib/dal/src/attribute/prototype/argument.rs @@ -440,7 +440,6 @@ impl AttributePrototypeArgument { let self_node_index = workspace_snapshot.get_node_index_by_id(self.id).await?; workspace_snapshot .remove_edge( - ctx.vector_clock_id()?, self_node_index, existing_value_source, EdgeWeightKindDiscriminants::PrototypeArgumentValue, diff --git a/lib/dal/src/change_set.rs b/lib/dal/src/change_set.rs index cd1f59d31d..283bc0290d 100644 --- a/lib/dal/src/change_set.rs +++ b/lib/dal/src/change_set.rs @@ -6,7 +6,6 @@ use std::sync::Arc; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use si_events::VectorClockChangeSetId; use si_layer_cache::LayerDbError; use thiserror::Error; @@ -14,9 +13,8 @@ use si_data_pg::{PgError, PgRow}; use si_events::{ulid::Ulid, WorkspaceSnapshotAddress}; use telemetry::prelude::*; -use crate::context::{Conflicts, RebaseRequest}; +use crate::context::RebaseRequest; use crate::slow_rt::SlowRuntimeError; -use crate::workspace_snapshot::vector_clock::VectorClockId; use crate::{ action::{ActionError, ActionId}, id, ChangeSetStatus, ComponentError, DalContext, HistoryActor, HistoryEvent, HistoryEventError, @@ -109,8 +107,6 @@ pub enum ChangeSetApplyError { ChangeSetNotFound(ChangeSetId), #[error("component error: {0}")] Component(#[from] ComponentError), - #[error("could not apply to head because of merge conflicts")] - ConflictsOnApply(Conflicts), #[error("invalid user: {0}")] InvalidUser(UserPk), #[error("invalid user system init")] @@ -177,10 +173,6 @@ impl ChangeSet { workspace_snapshot_address: WorkspaceSnapshotAddress, ) -> ChangeSetResult { let id: Ulid = Ulid::new(); - let vector_clock_id = VectorClockId::new( - VectorClockChangeSetId::new(id), - ctx.vector_clock_id()?.actor_id(), - ); let change_set_id: ChangeSetId = id.into(); let workspace_snapshot = WorkspaceSnapshot::find(ctx, workspace_snapshot_address) @@ -191,10 +183,7 @@ impl ChangeSet { // the edit session vs what the changeset already contained. The "onto" // changeset needs to have seen the "to_rebase" or we will treat them as // completely disjoint changesets. - let workspace_snapshot_address = workspace_snapshot - .write(ctx, vector_clock_id) - .await - .map_err(Box::new)?; + let workspace_snapshot_address = workspace_snapshot.write(ctx).await.map_err(Box::new)?; let workspace_id = ctx.tenancy().workspace_pk(); let name = name.as_ref(); @@ -437,16 +426,10 @@ impl ChangeSet { .apply_to_base_change_set_inner(ctx) .await?; - // do we need this commit? - if let Some(conflicts) = ctx.blocking_commit().await? { - error!("Conflicts when commiting again:{:?}", conflicts); - - return Err(ChangeSetApplyError::ConflictsOnApply(conflicts)); - } - - let change_set_that_was_applied = change_set_to_be_applied; + // This is just to send the ws events + ctx.blocking_commit_no_rebase().await?; - Ok(change_set_that_was_applied) + Ok(change_set_to_be_applied) } /// Applies the current [`ChangeSet`] in the provided [`DalContext`] to its base @@ -456,19 +439,18 @@ impl ChangeSet { /// This function neither changes the visibility nor the snapshot after performing the /// aforementioned actions. async fn apply_to_base_change_set_inner(&mut self, ctx: &DalContext) -> ChangeSetResult<()> { - let to_rebase_change_set_id = self + let base_change_set_id = self .base_change_set_id .ok_or(ChangeSetError::NoBaseChangeSet(self.id))?; - let to_rebase_workspace_snapshot = Arc::new( - WorkspaceSnapshot::find_for_change_set(ctx, to_rebase_change_set_id) + let base_snapshot = Arc::new( + WorkspaceSnapshot::find_for_change_set(ctx, base_change_set_id) .await .map_err(Box::new)?, ); if let Some(rebase_batch) = WorkspaceSnapshot::calculate_rebase_batch( - to_rebase_change_set_id, - to_rebase_workspace_snapshot, + base_snapshot, ctx.workspace_snapshot().map_err(Box::new)?, ) .await @@ -476,13 +458,13 @@ impl ChangeSet { { let rebase_batch_address = ctx.write_rebase_batch(rebase_batch).await?; - let rebase_request = RebaseRequest::new(to_rebase_change_set_id, rebase_batch_address); + let rebase_request = RebaseRequest::new(base_change_set_id, rebase_batch_address); ctx.do_rebase_request(rebase_request).await?; } self.update_status(ctx, ChangeSetStatus::Applied).await?; let user = Self::extract_userid_from_context(ctx).await; - WsEvent::change_set_applied(ctx, self.id, to_rebase_change_set_id, user) + WsEvent::change_set_applied(ctx, self.id, base_change_set_id, user) .await? .publish_on_commit(ctx) .await?; diff --git a/lib/dal/src/context.rs b/lib/dal/src/context.rs index 5cbdfccb1c..689d6b20dd 100644 --- a/lib/dal/src/context.rs +++ b/lib/dal/src/context.rs @@ -29,10 +29,9 @@ use crate::feature_flags::FeatureFlagService; use crate::job::definition::AttributeValueBasedJobIdentifier; use crate::layer_db_types::ContentTypes; use crate::slow_rt::SlowRuntimeError; +use crate::workspace_snapshot::graph::detect_updates::Update; use crate::workspace_snapshot::{ - conflict::Conflict, graph::{RebaseBatch, WorkspaceSnapshotGraph}, - update::Update, vector_clock::VectorClockId, }; use crate::{ @@ -217,30 +216,28 @@ impl ConnectionState { tenancy: &Tenancy, layer_db: &DalLayerDb, rebase_request: Option, - ) -> Result<(Self, Option), TransactionsError> { - let (conns, conflicts) = match self { + ) -> Result { + let conns = match self { Self::Connections(conns) => { // We need to rebase and wait for the rebaser to update the change set // pointer, even if we are not in a "transactions" state - let conflicts = if let Some(rebase_request) = rebase_request { - rebase(tenancy, layer_db, rebase_request).await? - } else { - None - }; + if let Some(rebase_request) = rebase_request { + rebase(tenancy, layer_db, rebase_request).await?; + } trace!("no active transactions present when commit was called"); - Ok((Self::Connections(conns), conflicts)) + Ok(Self::Connections(conns)) } Self::Transactions(txns) => { - let (conns, conflicts) = txns + let conns = txns .commit_into_conns(tenancy, layer_db, rebase_request) .await?; - Ok((Self::Connections(conns), conflicts)) + Ok(Self::Connections(conns)) } Self::Invalid => Err(TransactionsError::TxnCommit), }?; - Ok((conns, conflicts)) + Ok(conns) } async fn blocking_commit( @@ -248,26 +245,24 @@ impl ConnectionState { tenancy: &Tenancy, layer_db: &DalLayerDb, rebase_request: Option, - ) -> Result<(Self, Option), TransactionsError> { + ) -> Result { match self { Self::Connections(conns) => { trace!("no active transactions present when commit was called, but we will still attempt rebase"); // Even if there are no open dal transactions, we may have written to the layer db // and we need to perform a rebase if one is requested - let conflicts = if let Some(rebase_request) = rebase_request { - rebase(tenancy, layer_db, rebase_request).await? - } else { - None - }; + if let Some(rebase_request) = rebase_request { + rebase(tenancy, layer_db, rebase_request).await?; + } - Ok((Self::Connections(conns), conflicts)) + Ok(Self::Connections(conns)) } Self::Transactions(txns) => { - let (conns, conflicts) = txns + let conns = txns .blocking_commit_into_conns(tenancy, layer_db, rebase_request) .await?; - Ok((Self::Connections(conns), conflicts)) + Ok(Self::Connections(conns)) } Self::Invalid => Err(TransactionsError::TxnCommit), } @@ -369,11 +364,9 @@ impl DalContext { &self, ) -> Result, TransactionsError> { if let Some(snapshot) = &self.workspace_snapshot { - let vector_clock_id = self.vector_clock_id()?; - - Ok(Some(snapshot.write(self, vector_clock_id).await.map_err( - |err| TransactionsError::WorkspaceSnapshot(Box::new(err)), - )?)) + Ok(Some(snapshot.write(self).await.map_err(|err| { + TransactionsError::WorkspaceSnapshot(Box::new(err)) + })?)) } else { Ok(None) } @@ -383,54 +376,38 @@ impl DalContext { &self, rebase_request: RebaseRequest, ) -> Result<(), TransactionsError> { - if let Some(conflicts) = rebase(&self.tenancy, &self.layer_db(), rebase_request).await? { - let conflict_count = &conflicts.conflicts_found.len(); - let updates_found_and_skipped = &conflicts.updates_found_and_skipped.clone(); - let err = TransactionsError::ConflictsOccurred(conflicts); - error!( - si.error.message = ?err, - si.conflicts.count = { conflict_count }, - si.updates_found_and_skipped = format!("{:?}", updates_found_and_skipped), - "conflicts found on commit" - ); - return Err(err); - } + rebase(&self.tenancy, &self.layer_db(), rebase_request).await?; Ok(()) } async fn commit_internal( &self, rebase_request: Option, - ) -> Result, TransactionsError> { - let conflicts = if self.blocking { - self.blocking_commit_internal(rebase_request).await? + ) -> Result<(), TransactionsError> { + if self.blocking { + self.blocking_commit_internal(rebase_request).await?; } else { let mut guard = self.conns_state.lock().await; - let (new_guard, conflicts) = guard + *guard = guard .take() .commit(&self.tenancy, &self.layer_db(), rebase_request) .await?; - *guard = new_guard; - - conflicts }; - Ok(conflicts) + Ok(()) } async fn blocking_commit_internal( &self, rebase_request: Option, - ) -> Result, TransactionsError> { + ) -> Result<(), TransactionsError> { let mut guard = self.conns_state.lock().await; - - let (new_guard, conflicts) = guard + *guard = guard .take() .blocking_commit(&self.tenancy, &self.layer_db(), rebase_request) .await?; - *guard = new_guard; - Ok(conflicts) + Ok(()) } pub fn to_builder(&self) -> DalContextBuilder { @@ -466,11 +443,7 @@ impl DalContext { &self, ) -> Result, TransactionsError> { Ok(if let Some(snapshot) = &self.workspace_snapshot { - if let Some(rebase_batch) = snapshot - .current_rebase_batch(self.change_set_id(), self.vector_clock_id()?) - .await - .map_err(Box::new)? - { + if let Some(rebase_batch) = snapshot.current_rebase_batch().await.map_err(Box::new)? { Some(self.write_rebase_batch(rebase_batch).await?) } else { None @@ -489,22 +462,11 @@ impl DalContext { RebaseRequest::new(self.change_set_id(), rebase_batch_address) }); - if let Some(conflicts) = if self.blocking { - self.blocking_commit_internal(rebase_request).await? + if self.blocking { + self.blocking_commit_internal(rebase_request).await } else { - self.commit_internal(rebase_request).await? - } { - let conflict_count = &conflicts.conflicts_found.len(); - let err = TransactionsError::ConflictsOccurred(conflicts.clone()); - error!( - si.error.message = ?err, - si.conflicts.count = { conflict_count }, - si.conflicts = ?conflicts.conflicts_found.clone(), - "conflicts found on commit" - ); - return Err(err); + self.commit_internal(rebase_request).await } - Ok(()) } pub async fn commit_no_rebase(&self) -> Result<(), TransactionsError> { @@ -593,7 +555,7 @@ impl DalContext { /// Consumes all inner transactions, committing all changes made within them, and /// blocks until all queued jobs have reported as finishing. - pub async fn blocking_commit(&self) -> Result, TransactionsError> { + pub async fn blocking_commit(&self) -> Result<(), TransactionsError> { let rebase_request = self .write_current_rebase_batch() .await? @@ -1136,8 +1098,6 @@ pub enum TransactionsError { ChangeSetNotFound(ChangeSetId), #[error("change set not set on DalContext")] ChangeSetNotSet, - #[error("one or more conflicts have occurred: {0:?}")] - ConflictsOccurred(Conflicts), #[error(transparent)] JobQueueProcessor(#[from] JobQueueProcessorError), #[error("tokio join error: {0}")] @@ -1265,11 +1225,6 @@ impl RebaseRequest { } } -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct Conflicts { - pub conflicts_found: Vec, - pub updates_found_and_skipped: Vec, -} #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Updates { pub updates_found: Vec, @@ -1294,7 +1249,7 @@ async fn rebase( tenancy: &Tenancy, layer_db: &DalLayerDb, rebase_request: RebaseRequest, -) -> Result, TransactionsError> { +) -> Result<(), TransactionsError> { let start = Instant::now(); let span = Span::current(); @@ -1338,28 +1293,13 @@ async fn rebase( } => { //span.record("si.updates", updates_performed); //span.record("si.updates.count", updates.updates_found.len().to_string()); - Ok(None) + Ok(()) } RebaseStatus::Error { message } => Err(TransactionsError::RebaseFailed( rebase_request.rebase_batch_address, rebase_request.to_rebase_change_set_id, message.to_string(), )), - RebaseStatus::ConflictsFound { - conflicts_found, - updates_found_and_skipped, - } => { - let conflicts = Conflicts { - conflicts_found: serde_json::from_str(conflicts_found)?, - updates_found_and_skipped: serde_json::from_str(updates_found_and_skipped)?, - }; - span.record("si.conflicts", conflicts_found); - span.record( - "si.conflicts.count", - conflicts.conflicts_found.len().to_string(), - ); - Ok(Some(conflicts)) - } }, p => Err(TransactionsError::BadActivity( ActivityPayloadDiscriminants::RebaseFinished, @@ -1409,7 +1349,7 @@ impl Transactions { tenancy: &Tenancy, layer_db: &DalLayerDb, rebase_request: Option, - ) -> Result<(Connections, Option), TransactionsError> { + ) -> Result { let span = Span::current(); span.record( "si.workspace.id", @@ -1420,7 +1360,7 @@ impl Transactions { ); let pg_conn = self.pg_txn.commit_into_conn().await?; - let conflicts = if let Some(rebase_request) = rebase_request { + if let Some(rebase_request) = rebase_request { span.record( "si.change_set.id", rebase_request.to_rebase_change_set_id.to_string(), @@ -1431,18 +1371,13 @@ impl Transactions { .job_queue .take_dependent_values_for_change_set(rebase_request.to_rebase_change_set_id) .await; - rebase(tenancy, layer_db, rebase_request).await? - } else { - None + rebase(tenancy, layer_db, rebase_request).await?; }; - let nats_conn = self.nats_txn.commit_into_conn().await?; + let nats_conn = self.nats_txn.commit_into_conn().await?; self.job_processor.process_queue(self.job_queue).await?; - Ok(( - Connections::new(pg_conn, nats_conn, self.job_processor), - conflicts, - )) + Ok(Connections::new(pg_conn, nats_conn, self.job_processor)) } /// Consumes all inner transactions, committing all changes made within them, and returns @@ -1458,23 +1393,11 @@ impl Transactions { tenancy: &Tenancy, layer_db: &DalLayerDb, rebase_request: Option, - ) -> Result<(Connections, Option), TransactionsError> { + ) -> Result { let pg_conn = self.pg_txn.commit_into_conn().await?; - let conflicts = if let Some(rebase_request) = rebase_request { - rebase(tenancy, layer_db, rebase_request).await? - } else { - None - }; - if let Some(ref conflicts) = conflicts { - let conflict_count = conflicts.conflicts_found.len(); - let err = TransactionsError::ConflictsOccurred(conflicts.clone()); - error!( - si.error.message = ?err, - si.conflicts.count = { conflict_count }, - si.conflicts = ?conflicts.conflicts_found.clone(), - "conflicts found on blocking commit" - ); + if let Some(rebase_request) = rebase_request { + rebase(tenancy, layer_db, rebase_request).await?; } let nats_conn = self.nats_txn.commit_into_conn().await?; @@ -1484,7 +1407,7 @@ impl Transactions { .await?; let conns = Connections::new(pg_conn, nats_conn, self.job_processor); - Ok((conns, conflicts)) + Ok(conns) } /// Rolls all inner transactions back, discarding all changes made within them, and returns diff --git a/lib/dal/src/job/definition/action.rs b/lib/dal/src/job/definition/action.rs index 667295afdf..4c29d1e275 100644 --- a/lib/dal/src/job/definition/action.rs +++ b/lib/dal/src/job/definition/action.rs @@ -1,11 +1,9 @@ -use std::{convert::TryFrom, time::Duration}; +use std::convert::TryFrom; use async_trait::async_trait; -use futures::Future; use serde::{Deserialize, Serialize}; use si_events::ActionResultState; use telemetry::prelude::*; -use tryhard::RetryPolicy; use ulid::Ulid; use veritech_client::{ActionRunResultSuccess, ResourceStatus}; @@ -24,8 +22,8 @@ use crate::{ producer::{JobProducer, JobProducerResult}, }, workspace_snapshot::graph::WorkspaceSnapshotGraphError, - AccessBuilder, ActionPrototypeId, Component, ComponentId, DalContext, TransactionsError, - Visibility, WorkspaceSnapshotError, WsEvent, + AccessBuilder, ActionPrototypeId, Component, ComponentId, DalContext, Visibility, + WorkspaceSnapshotError, WsEvent, }; #[derive(Debug, Deserialize, Serialize)] @@ -142,25 +140,9 @@ async fn inner_run( // Execute the action function let maybe_resource = ActionPrototype::run(ctx, prototype_id, component_id).await?; - // Retry process_and_record_execution on a conflict error up to a max - let nodes_to_remove: Vec = retry_on_conflicts( - || process_and_record_execution(ctx.clone(), maybe_resource.as_ref(), action_id), - 10, - Duration::from_millis(10), - "si.action_job.process.retry.process_and_record_execution", - ) - .await?; - - // check if the things we expected to be removed actually were. The snapshot has already been updated to latest by this point - if !nodes_to_remove.is_empty() { - retry_on_conflicts( - || ensure_deletes_happened(ctx.clone(), nodes_to_remove.clone()), - 10, - Duration::from_millis(10), - "si.action_job.process.retry.ensure_deletes_happened", - ) - .await?; - } + let nodes_to_remove = + process_and_record_execution(ctx.clone(), maybe_resource.as_ref(), action_id).await?; + ensure_deletes_happened(ctx.clone(), nodes_to_remove).await?; // if the action kind was a delete, let's see if any components are ready to be removed that weren't already let prototype = ActionPrototype::get_by_id(ctx, prototype_id).await?; @@ -179,18 +161,7 @@ async fn inner_run( } } if did_remove { - if let Err(err) = ctx.commit().await { - match err { - // if this fails due to conflicts, it's recoverable. - // the next time a destroy action runs we'll try again - TransactionsError::ConflictsOccurred(ref conflicts) => warn!( - ?err, - ?conflicts, - "Conflicts occurred while deleting components" - ), - _ => return Err(JobConsumerError::Transactions(err)), - } - } + ctx.commit().await.map_err(JobConsumerError::Transactions)?; } } Ok(maybe_resource) @@ -362,38 +333,3 @@ async fn process_failed_action(ctx: &DalContext, action_id: ActionId) -> JobCons ctx.commit().await?; Ok(()) } - -async fn retry_on_conflicts( - f: F, - max_attempts: u32, - fixed_delay: Duration, - span_field: &'static str, -) -> Result -where - F: FnMut() -> Fut, - Fut: Future>, -{ - let span = Span::current(); - - // Jitter implementation thanks to the `fure` crate, released under the MIT license. - // - // See: https://github.com/Leonqn/fure/blob/8945c35655f7e0f6966d8314ab21a297181cc080/src/backoff.rs#L44-L51 - fn jitter(duration: Duration) -> Duration { - let jitter = rand::random::(); - let secs = ((duration.as_secs() as f64) * jitter).ceil() as u64; - let nanos = ((f64::from(duration.subsec_nanos())) * jitter).ceil() as u32; - Duration::new(secs, nanos) - } - - tryhard::retry_fn(f) - .retries(max_attempts.saturating_sub(1)) - .custom_backoff(|attempt, err: &JobConsumerError| { - if let JobConsumerError::Transactions(TransactionsError::ConflictsOccurred(_)) = err { - span.record(span_field, attempt); - RetryPolicy::Delay(jitter(fixed_delay)) - } else { - RetryPolicy::Break - } - }) - .await -} diff --git a/lib/dal/src/job/definition/dependent_values_update.rs b/lib/dal/src/job/definition/dependent_values_update.rs index c7df55a0e4..4561634fde 100644 --- a/lib/dal/src/job/definition/dependent_values_update.rs +++ b/lib/dal/src/job/definition/dependent_values_update.rs @@ -20,7 +20,7 @@ use crate::{ job::{ consumer::{ JobCompletionState, JobConsumer, JobConsumerError, JobConsumerMetadata, - JobConsumerResult, JobInfo, RetryBackoff, + JobConsumerResult, JobInfo, }, producer::{JobProducer, JobProducerResult}, }, @@ -30,8 +30,6 @@ use crate::{ WorkspacePk, WorkspaceSnapshotError, WsEvent, WsEventError, }; -const MAX_RETRIES: u32 = 8; - #[remain::sorted] #[derive(Debug, Error)] pub enum DependentValueUpdateError { @@ -271,20 +269,9 @@ impl DependentValuesUpdate { debug!("DependentValuesUpdate took: {:?}", start.elapsed()); - Ok(match ctx.commit().await { - Ok(_) => JobCompletionState::Done, - Err(err) => match err { - TransactionsError::ConflictsOccurred(_) => { - // retry still on conflicts in DVU - warn!("Retrying DependentValueUpdate due to conflicts"); - JobCompletionState::Retry { - limit: MAX_RETRIES, - backoff: RetryBackoff::Exponential, - } - } - err => return Err(DependentValueUpdateError::Transactions(err)), - }, - }) + ctx.commit().await?; + + Ok(JobCompletionState::Done) } } diff --git a/lib/dal/src/schema.rs b/lib/dal/src/schema.rs index 74e906d770..2ec41af647 100644 --- a/lib/dal/src/schema.rs +++ b/lib/dal/src/schema.rs @@ -275,12 +275,7 @@ impl Schema { // We have found the existing Default edge between schema and schema variant // we now need to update that edge to be a Use workspace_snapshot - .remove_edge( - ctx.vector_clock_id()?, - source_index, - target_index, - edge_weight.kind().into(), - ) + .remove_edge(source_index, target_index, edge_weight.kind().into()) .await?; Self::add_edge_to_variant( @@ -302,12 +297,7 @@ impl Schema { .await?; workspace_snapshot - .remove_edge( - ctx.vector_clock_id()?, - source_index, - target_index, - EdgeWeightKind::new_use().into(), - ) + .remove_edge(source_index, target_index, EdgeWeightKind::new_use().into()) .await?; Self::add_edge_to_variant( diff --git a/lib/dal/src/schema/variant.rs b/lib/dal/src/schema/variant.rs index 6ddf563d8e..f38d7a2931 100644 --- a/lib/dal/src/schema/variant.rs +++ b/lib/dal/src/schema/variant.rs @@ -2001,7 +2001,6 @@ impl SchemaVariant { | EdgeWeightKindDiscriminants::ActionPrototype => { workspace_snapshot .remove_all_edges( - ctx.vector_clock_id()?, workspace_snapshot.get_node_weight(target_index).await?.id(), ) .await?; @@ -2009,7 +2008,7 @@ impl SchemaVariant { EdgeWeightKindDiscriminants::AuthenticationPrototype => { workspace_snapshot - .remove_edge(ctx.vector_clock_id()?, source_index, target_index, kind) + .remove_edge(source_index, target_index, kind) .await?; } diff --git a/lib/dal/src/workspace.rs b/lib/dal/src/workspace.rs index 1ef282882e..daf183c422 100644 --- a/lib/dal/src/workspace.rs +++ b/lib/dal/src/workspace.rs @@ -183,7 +183,7 @@ impl Workspace { // 'reset' the workspace snapshot' so that we remigrate all builtins on each startup let vector_clock = ctx.vector_clock_id()?; let new_snapshot = WorkspaceSnapshot::initial(ctx, vector_clock).await?; - let new_snap_address = new_snapshot.write(ctx, vector_clock).await?; + let new_snap_address = new_snapshot.write(ctx).await?; let new_change_set = ChangeSet::new(ctx, DEFAULT_CHANGE_SET_NAME, None, new_snap_address).await?; found_builtin @@ -549,9 +549,7 @@ impl Workspace { )?) }; - // XXX: fake vector clock here. Figure out the right one - let vector_clock_id = VectorClockId::new(Ulid::new(), Ulid::new()); - let new_snap_address = imported_snapshot.write(ctx, vector_clock_id).await?; + let new_snap_address = imported_snapshot.write(ctx).await?; let new_change_set = ChangeSet::new( ctx, diff --git a/lib/dal/src/workspace_snapshot.rs b/lib/dal/src/workspace_snapshot.rs index a4ca266cac..b9007f7eb2 100644 --- a/lib/dal/src/workspace_snapshot.rs +++ b/lib/dal/src/workspace_snapshot.rs @@ -21,9 +21,7 @@ // clippy::missing_panics_doc // )] -pub mod conflict; pub mod content_address; -pub mod edge_info; pub mod edge_weight; pub mod graph; pub mod lamport_clock; @@ -32,6 +30,7 @@ pub mod node_weight; pub mod update; pub mod vector_clock; +use graph::detect_updates::Update; use graph::{RebaseBatch, WorkspaceSnapshotGraph}; use std::collections::HashSet; use std::sync::atomic::AtomicBool; @@ -49,7 +48,6 @@ use thiserror::Error; use tokio::sync::{Mutex, RwLock, RwLockReadGuard, RwLockWriteGuard}; use tokio::task::JoinError; -use self::graph::ConflictsAndUpdates; use self::node_weight::{NodeWeightDiscriminants, OrderingNodeWeight}; use crate::action::{Action, ActionError}; use crate::attribute::prototype::argument::{ @@ -64,7 +62,6 @@ use crate::workspace_snapshot::edge_weight::{ use crate::workspace_snapshot::graph::LineageId; use crate::workspace_snapshot::node_weight::category_node_weight::CategoryNodeKind; use crate::workspace_snapshot::node_weight::NodeWeight; -use crate::workspace_snapshot::update::Update; use crate::workspace_snapshot::vector_clock::VectorClockId; use crate::{ pk, AttributeValueId, ChangeSet, Component, ComponentError, ComponentId, Workspace, @@ -311,7 +308,7 @@ impl WorkspaceSnapshot { dvu_roots: Arc::new(Mutex::new(HashSet::new())), }; - initial.write(ctx, vector_clock_id).await?; + initial.write(ctx).await?; Ok(initial) } @@ -354,86 +351,35 @@ impl WorkspaceSnapshot { self.cycle_check.load(std::sync::atomic::Ordering::Relaxed) } - pub async fn current_rebase_batch( - &self, - to_rebase_change_set_id: ChangeSetId, - vector_clock_id: VectorClockId, - ) -> WorkspaceSnapshotResult> { + /// Calculates the set of updates for the current snapshot against its working copy + pub async fn current_rebase_batch(&self) -> WorkspaceSnapshotResult> { let self_clone = self.clone(); - let conflicts_and_updates = slow_rt::spawn(async move { + let updates = slow_rt::spawn(async move { let mut working_copy = self_clone.working_copy_mut().await; working_copy.cleanup(); - working_copy.mark_graph_seen(vector_clock_id)?; - - let to_rebase_vector_clock_id = self_clone - .read_only_graph - .max_recently_seen_clock_id(Some(to_rebase_change_set_id)) - .ok_or(WorkspaceSnapshotError::MissingVectorClockForChangeSet( - to_rebase_change_set_id, - ))?; - - let onto_vector_clock_id = working_copy.max_recently_seen_clock_id(None).ok_or( - WorkspaceSnapshotError::MissingVectorClockForChangeSet(to_rebase_change_set_id), - )?; - let conflicts_and_updates = self_clone.read_only_graph.detect_conflicts_and_updates( - to_rebase_vector_clock_id, - &working_copy, - onto_vector_clock_id, - )?; - Ok::(conflicts_and_updates) + self_clone.read_only_graph.detect_updates(&working_copy) })? - .await??; + .await?; - Ok(if !conflicts_and_updates.conflicts.is_empty() { - Err(WorkspaceSnapshotError::ConflictsInRebaseBatch)? - } else if conflicts_and_updates.updates.is_empty() { - None - } else { - Some(RebaseBatch::new(conflicts_and_updates.updates)) - }) + Ok((!updates.is_empty()).then_some(RebaseBatch::new(updates))) } + /// Calculates the set of updates made to `updated_snapshot` against + /// `base_snapshot`. For these updates to be correct, `updated_snapshot` must + /// have already seen all the changes made to `base_snapshot` pub async fn calculate_rebase_batch( - to_rebase_change_set_id: ChangeSetId, - to_rebase_workspace_snapshot: Arc, - onto_workspace_snapshot: Arc, + base_snapshot: Arc, + updated_snapshot: Arc, ) -> WorkspaceSnapshotResult> { - let conflicts_and_updates = slow_rt::spawn(async move { - let to_rebase_vector_clock_id = to_rebase_workspace_snapshot - .max_recently_seen_clock_id(Some(to_rebase_change_set_id)) - .await? - .ok_or(WorkspaceSnapshotError::MissingVectorClockForChangeSet( - to_rebase_change_set_id, - ))?; + let updates = slow_rt::spawn(async move { + let updates = base_snapshot.detect_updates(&updated_snapshot).await?; - let onto_vector_clock_id = onto_workspace_snapshot - .max_recently_seen_clock_id(None) - .await? - .ok_or(WorkspaceSnapshotError::MissingVectorClockForChangeSet( - to_rebase_change_set_id, - ))?; - - let conflicts_and_updates = to_rebase_workspace_snapshot - .detect_conflicts_and_updates( - to_rebase_vector_clock_id, - &onto_workspace_snapshot, - onto_vector_clock_id, - ) - .await?; - - Ok::(conflicts_and_updates) + Ok::, WorkspaceSnapshotError>(updates) })? .await??; - Ok(if !conflicts_and_updates.conflicts.is_empty() { - // error - Err(WorkspaceSnapshotError::ConflictsInRebaseBatch)? - } else if conflicts_and_updates.updates.is_empty() { - None - } else { - Some(RebaseBatch::new(conflicts_and_updates.updates)) - }) + Ok((!updates.is_empty()).then_some(RebaseBatch::new(updates))) } #[instrument( @@ -441,14 +387,12 @@ impl WorkspaceSnapshot { level = "debug", skip_all, fields( - si.vector_clock.id = %vector_clock_id, si.workspace_snapshot.address = Empty, ) )] pub async fn write( &self, ctx: &DalContext, - vector_clock_id: VectorClockId, ) -> WorkspaceSnapshotResult { // Pull out the working copy and clean it up. let new_address = { @@ -466,9 +410,6 @@ impl WorkspaceSnapshot { let mut working_copy = self_clone.working_copy_mut().await; working_copy.cleanup(); - // Mark everything left as seen. - working_copy.mark_graph_seen(vector_clock_id)?; - let (new_address, _) = layer_db .workspace_snapshot() .write( @@ -683,17 +624,15 @@ impl WorkspaceSnapshot { } #[instrument( - name = "workspace_snapshot.detect_conflicts_and_updates", + name = "workspace_snapshot.detect_updates", level = "debug", skip_all, fields() )] - pub async fn detect_conflicts_and_updates( + pub async fn detect_updates( &self, - to_rebase_vector_clock_id: VectorClockId, onto_workspace_snapshot: &WorkspaceSnapshot, - onto_vector_clock_id: VectorClockId, - ) -> WorkspaceSnapshotResult { + ) -> WorkspaceSnapshotResult> { let self_clone = self.clone(); let onto_clone = onto_workspace_snapshot.clone(); @@ -701,13 +640,9 @@ impl WorkspaceSnapshot { self_clone .working_copy() .await - .detect_conflicts_and_updates( - to_rebase_vector_clock_id, - &*onto_clone.working_copy().await, - onto_vector_clock_id, - ) + .detect_updates(&*onto_clone.working_copy().await) })? - .await??) + .await?) } /// Gives the exact node index endpoints of an edge. Use with care, since @@ -1105,18 +1040,14 @@ impl WorkspaceSnapshot { skip_all, fields() )] - pub async fn remove_all_edges( - &self, - vector_clock_id: VectorClockId, - id: impl Into, - ) -> WorkspaceSnapshotResult<()> { + pub async fn remove_all_edges(&self, id: impl Into) -> WorkspaceSnapshotResult<()> { let id = id.into(); for (edge_weight, source, target) in self.edges_directed(id, Direction::Outgoing).await? { - self.remove_edge(vector_clock_id, source, target, edge_weight.kind().into()) + self.remove_edge(source, target, edge_weight.kind().into()) .await?; } for (edge_weight, source, target) in self.edges_directed(id, Direction::Incoming).await? { - self.remove_edge(vector_clock_id, source, target, edge_weight.kind().into()) + self.remove_edge(source, target, edge_weight.kind().into()) .await?; } Ok(()) @@ -1258,7 +1189,7 @@ impl WorkspaceSnapshot { )] pub async fn remove_incoming_edges_of_kind( &self, - vector_clock_id: VectorClockId, + _vector_clock_id: VectorClockId, target_id: impl Into, kind: EdgeWeightKindDiscriminants, ) -> WorkspaceSnapshotResult<()> { @@ -1269,7 +1200,7 @@ impl WorkspaceSnapshot { .await?; for source_node_idx in sources { let target_node_idx = self.get_node_index_by_id(target_id).await?; - self.remove_edge(vector_clock_id, source_node_idx, target_node_idx, kind) + self.remove_edge(source_node_idx, target_node_idx, kind) .await?; } @@ -1307,12 +1238,12 @@ impl WorkspaceSnapshot { )] pub async fn remove_node_by_id( &self, - vector_clock_id: VectorClockId, + _vector_clock_id: VectorClockId, id: impl Into, ) -> WorkspaceSnapshotResult<()> { let id: Ulid = id.into(); let node_idx = self.get_node_index_by_id(id).await?; - self.remove_all_edges(vector_clock_id, id).await?; + self.remove_all_edges(id).await?; self.working_copy_mut().await.remove_node(node_idx); self.working_copy_mut().await.remove_node_id(id); @@ -1327,13 +1258,11 @@ impl WorkspaceSnapshot { )] pub async fn remove_edge( &self, - vector_clock_id: VectorClockId, source_node_index: NodeIndex, target_node_index: NodeIndex, edge_kind: EdgeWeightKindDiscriminants, ) -> WorkspaceSnapshotResult<()> { Ok(self.working_copy_mut().await.remove_edge( - vector_clock_id, source_node_index, target_node_index, edge_kind, @@ -1363,7 +1292,7 @@ impl WorkspaceSnapshot { )] pub async fn remove_edge_for_ulids( &self, - vector_clock_id: VectorClockId, + _vector_clock_id: VectorClockId, source_node_id: impl Into, target_node_id: impl Into, edge_kind: EdgeWeightKindDiscriminants, @@ -1376,13 +1305,8 @@ impl WorkspaceSnapshot { .working_copy() .await .get_node_index_by_id(target_node_id)?; - self.remove_edge( - vector_clock_id, - source_node_index, - target_node_index, - edge_kind, - ) - .await + self.remove_edge(source_node_index, target_node_index, edge_kind) + .await } /// Perform [`Updates`](Update) using [`self`](WorkspaceSnapshot) as the "to rebase" graph and @@ -1393,15 +1317,8 @@ impl WorkspaceSnapshot { skip_all, fields() )] - pub async fn perform_updates( - &self, - to_rebase_vector_clock_id: VectorClockId, - updates: &[Update], - ) -> WorkspaceSnapshotResult<()> { - Ok(self - .working_copy_mut() - .await - .perform_updates(to_rebase_vector_clock_id, updates)?) + pub async fn perform_updates(&self, updates: &[Update]) -> WorkspaceSnapshotResult<()> { + Ok(self.working_copy_mut().await.perform_updates(updates)?) } /// Mark whether a prop can be used as an input to a function. Props below @@ -1524,41 +1441,26 @@ impl WorkspaceSnapshot { } let base_snapshot = WorkspaceSnapshot::find_for_change_set(ctx, base_change_set_id).await?; - let base_vector_clock_id = base_snapshot - .read_only_graph - .max_recently_seen_clock_id(Some(base_change_set_id)) - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set_id, - ))?; // If there is no vector clock in this snapshot for the current change // set, that's because the snapshot has *just* been forked from the base // change set, and has not been written to yet - if let Some(change_set_vector_clock_id) = self + let updates = base_snapshot .read_only_graph - .max_recently_seen_clock_id(Some(ctx.change_set_id())) - { - let conflicts_and_updates = - base_snapshot.read_only_graph.detect_conflicts_and_updates( - base_vector_clock_id, - &self.read_only_graph, - change_set_vector_clock_id, - )?; + .detect_updates(&self.read_only_graph); - for update in &conflicts_and_updates.updates { - match update { - Update::RemoveEdge { .. } - | Update::ReplaceNode { .. } - | Update::NewEdge { .. } => {} - Update::NewNode { node_weight } => { - if let NodeWeight::Component(inner) = node_weight { - if base_snapshot - .read_only_graph - .try_get_node_index_by_id(inner.id) - .is_none() - { - new_component_ids.push(inner.id.into()) - } + for update in &updates { + match update { + Update::RemoveEdge { .. } | Update::ReplaceNode { .. } | Update::NewEdge { .. } => { + } + Update::NewNode { node_weight } => { + if let NodeWeight::Component(inner) = node_weight { + if base_snapshot + .read_only_graph + .try_get_node_index_by_id(inner.id) + .is_none() + { + new_component_ids.push(inner.id.into()) } } } @@ -1600,13 +1502,6 @@ impl WorkspaceSnapshot { } let base_snapshot = WorkspaceSnapshot::find_for_change_set(ctx, base_change_set_id).await?; - let base_vector_clock_id = base_snapshot - .read_only_graph - .max_recently_seen_clock_id(Some(base_change_set_id)) - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set_id, - ))?; - let component_category_id = if let Some((category_id, _)) = base_snapshot .read_only_graph .get_category_node(None, CategoryNodeKind::Component)? @@ -1618,63 +1513,53 @@ impl WorkspaceSnapshot { return Ok(removed_component_ids); }; - if let Some(change_set_vector_clock_id) = self + let updates = base_snapshot .read_only_graph - .max_recently_seen_clock_id(Some(ctx.change_set_id())) - { - let conflicts_and_updates = - base_snapshot.read_only_graph.detect_conflicts_and_updates( - base_vector_clock_id, - &self.read_only_graph, - change_set_vector_clock_id, - )?; + .detect_updates(&self.read_only_graph); - let mut added_component_ids = HashSet::new(); + let mut added_component_ids = HashSet::new(); - for update in &conflicts_and_updates.updates { - match update { - Update::ReplaceNode { .. } | Update::NewNode { .. } => { - /* Updates unused for determining if a Component is removed in regard to the updates */ + for update in &updates { + match update { + Update::ReplaceNode { .. } | Update::NewNode { .. } => { + /* Updates unused for determining if a Component is removed in regard to the updates */ + } + Update::NewEdge { + source, + destination, + edge_weight: _, + } => { + // get updates that add an edge from the Components category to a component, which implies component creation + if source.id != component_category_id.into() + || destination.node_weight_kind != NodeWeightDiscriminants::Component + { + continue; } - Update::NewEdge { - source, - destination, - edge_weight: _, - } => { - // get updates that add an edge from the Components category to a component, which implies component creation - if source.id != component_category_id.into() - || destination.node_weight_kind != NodeWeightDiscriminants::Component - { - continue; - } - added_component_ids.insert(ComponentId::from(Ulid::from(destination.id))); + added_component_ids.insert(ComponentId::from(Ulid::from(destination.id))); + } + Update::RemoveEdge { + source, + destination, + edge_kind: _, + } => { + // get updates that remove an edge from the Components category to a component, which implies component deletion + if source.id != component_category_id.into() + || destination.node_weight_kind != NodeWeightDiscriminants::Component + { + continue; } - Update::RemoveEdge { - source, - destination, - edge_kind: _, - } => { - // get updates that remove an edge from the Components category to a component, which implies component deletion - if source.id != component_category_id.into() - || destination.node_weight_kind != NodeWeightDiscriminants::Component - { - continue; - } - removed_component_ids.push(ComponentId::from(Ulid::from(destination.id))); - } + removed_component_ids.push(ComponentId::from(Ulid::from(destination.id))); } } - - // Filter out ComponentIds that have both been deleted and created, since that implies an upgrade and not a real deletion - Ok(removed_component_ids - .into_iter() - .filter(|id| !added_component_ids.contains(id)) - .collect()) - } else { - Ok(removed_component_ids) } + + // Filter out ComponentIds that have both been deleted and created, since that implies an upgrade and not a real deletion + Ok(removed_component_ids + .into_iter() + .filter(|id| !added_component_ids.contains(id)) + .collect()) } #[instrument( @@ -1709,62 +1594,43 @@ impl WorkspaceSnapshot { } let base_snapshot = WorkspaceSnapshot::find_for_change_set(ctx, base_change_set_id).await?; - let base_vector_clock_id = base_snapshot - .read_only_graph - .max_recently_seen_clock_id(Some(base_change_set_id)) - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set_id, - ))?; - // If there is no vector clock in this snapshot for the current change - // set, that's because the snapshot has *just* been forked from the base - // change set, and has not been written to yet - if let Some(change_set_vector_clock_id) = self + let updates = base_snapshot .read_only_graph - .max_recently_seen_clock_id(Some(ctx.change_set_id())) - { - let conflicts_and_updates = - base_snapshot.read_only_graph.detect_conflicts_and_updates( - base_vector_clock_id, - &self.read_only_graph, - change_set_vector_clock_id, - )?; + .detect_updates(&self.read_only_graph); - for update in &conflicts_and_updates.updates { - match update { - Update::NewNode { .. } - | Update::RemoveEdge { .. } - | Update::ReplaceNode { .. } => { - // Updates unused for determining if a socket to socket connection (in frontend - // terms) is new. + for update in &updates { + match update { + Update::NewNode { .. } | Update::RemoveEdge { .. } | Update::ReplaceNode { .. } => { + // Updates unused for determining if a socket to socket connection (in frontend + // terms) is new. + } + Update::NewEdge { + source: _source, + destination, + edge_weight: _, + } => { + if destination.node_weight_kind + != NodeWeightDiscriminants::AttributePrototypeArgument + { + // We're interested in new AttributePrototypeArguments as they represent + // the connection between sockets. (The input socket has the output + // socket as one of its function arguments.) + continue; } - Update::NewEdge { - source: _source, - destination, - edge_weight: _, - } => { - if destination.node_weight_kind - != NodeWeightDiscriminants::AttributePrototypeArgument - { - // We're interested in new AttributePrototypeArguments as they represent - // the connection between sockets. (The input socket has the output - // socket as one of its function arguments.) - continue; - } - let prototype_argument = AttributePrototypeArgument::get_by_id( - ctx, - AttributePrototypeArgumentId::from(Ulid::from(destination.id)), - ) - .await - .map_err(Box::new)?; - if prototype_argument.targets().is_some() { - new_attribute_prototype_argument_ids.push(prototype_argument.id()); - } else { - // If the AttributePrototypeArgument doesn't have targets, then it's - // not for a socket to socket connection. - continue; - } + let prototype_argument = AttributePrototypeArgument::get_by_id( + ctx, + AttributePrototypeArgumentId::from(Ulid::from(destination.id)), + ) + .await + .map_err(Box::new)?; + if prototype_argument.targets().is_some() { + new_attribute_prototype_argument_ids.push(prototype_argument.id()); + } else { + // If the AttributePrototypeArgument doesn't have targets, then it's + // not for a socket to socket connection. + continue; } } } @@ -1783,11 +1649,6 @@ impl WorkspaceSnapshot { ctx: &DalContext, ) -> WorkspaceSnapshotResult> { let mut removed_attribute_prototype_argument_ids = Vec::new(); - let base_change_set_id = if let Some(change_set_id) = ctx.change_set()?.base_change_set_id { - change_set_id - } else { - return Ok(removed_attribute_prototype_argument_ids); - }; // Even though the default change set for a workspace can have a base change set, we don't // want to consider anything as new/modified/removed when looking at the default change @@ -1865,59 +1726,41 @@ impl WorkspaceSnapshot { // * For each removed AttributePrototypeArgument (removed edge connects two Components // that have not been removed): let base_snapshot = base_change_set_ctx.workspace_snapshot()?; - let base_vector_clock_id = base_snapshot + let updates = base_snapshot .read_only_graph - .max_recently_seen_clock_id(Some(base_change_set_id)) - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set_id, - ))?; - if let Some(change_set_vector_clock_id) = self - .read_only_graph - .max_recently_seen_clock_id(Some(ctx.change_set_id())) - { - let conflicts_and_updates = - base_snapshot.read_only_graph.detect_conflicts_and_updates( - base_vector_clock_id, - &self.read_only_graph, - change_set_vector_clock_id, - )?; + .detect_updates(&self.read_only_graph); - for update in conflicts_and_updates.updates { - match update { - Update::ReplaceNode { .. } - | Update::NewEdge { .. } - | Update::NewNode { .. } => { - /* Updates unused for determining if a connection between sockets has been removed */ + for update in updates { + match update { + Update::ReplaceNode { .. } | Update::NewEdge { .. } | Update::NewNode { .. } => { + /* Updates unused for determining if a connection between sockets has been removed */ + } + Update::RemoveEdge { + source: _, + destination, + edge_kind, + } => { + if edge_kind != EdgeWeightKindDiscriminants::PrototypeArgument { + continue; } - Update::RemoveEdge { - source: _, - destination, - edge_kind, - } => { - if edge_kind != EdgeWeightKindDiscriminants::PrototypeArgument { - continue; - } - let attribute_prototype_argument = AttributePrototypeArgument::get_by_id( - base_change_set_ctx, - AttributePrototypeArgumentId::from(Ulid::from(destination.id)), - ) - .await - .map_err(Box::new)?; - - // * Interested in all of them that have targets (connecting two Components - // via sockets). - if attribute_prototype_argument.targets().is_some() { - removed_attribute_prototype_argument_ids - .push(attribute_prototype_argument.id()); - } + let attribute_prototype_argument = AttributePrototypeArgument::get_by_id( + base_change_set_ctx, + AttributePrototypeArgumentId::from(Ulid::from(destination.id)), + ) + .await + .map_err(Box::new)?; + + // * Interested in all of them that have targets (connecting two Components + // via sockets). + if attribute_prototype_argument.targets().is_some() { + removed_attribute_prototype_argument_ids + .push(attribute_prototype_argument.id()); } } } - - Ok(removed_attribute_prototype_argument_ids) - } else { - Ok(vec![]) } + + Ok(removed_attribute_prototype_argument_ids) } /// Returns whether or not any Actions were dispatched. diff --git a/lib/dal/src/workspace_snapshot/conflict.rs b/lib/dal/src/workspace_snapshot/conflict.rs deleted file mode 100644 index fd263c81c3..0000000000 --- a/lib/dal/src/workspace_snapshot/conflict.rs +++ /dev/null @@ -1,31 +0,0 @@ -use serde::{Deserialize, Serialize}; - -use crate::{workspace_snapshot::NodeInformation, EdgeWeightKindDiscriminants}; - -/// Describe the type of conflict between the given locations in a -/// workspace graph. -#[remain::sorted] -#[derive(Debug, Copy, Clone, PartialEq, Eq, Deserialize, Serialize, Hash)] -pub enum Conflict { - ChildOrder { - onto: NodeInformation, - to_rebase: NodeInformation, - }, - ExclusiveEdgeMismatch { - source: NodeInformation, - destination: NodeInformation, - edge_kind: EdgeWeightKindDiscriminants, - }, - ModifyRemovedItem { - container: NodeInformation, - modified_item: NodeInformation, - }, - NodeContent { - onto: NodeInformation, - to_rebase: NodeInformation, - }, - RemoveModifiedItem { - container: NodeInformation, - removed_item: NodeInformation, - }, -} diff --git a/lib/dal/src/workspace_snapshot/edge_info.rs b/lib/dal/src/workspace_snapshot/edge_info.rs deleted file mode 100644 index 9184903517..0000000000 --- a/lib/dal/src/workspace_snapshot/edge_info.rs +++ /dev/null @@ -1,26 +0,0 @@ -use petgraph::prelude::{EdgeIndex, NodeIndex}; - -use crate::{EdgeWeightKindDiscriminants, WorkspaceSnapshotGraphV1}; - -#[derive(Debug, Copy, Clone)] -pub struct EdgeInfo { - pub source_node_index: NodeIndex, - pub target_node_index: NodeIndex, - pub edge_kind: EdgeWeightKindDiscriminants, - pub edge_index: EdgeIndex, -} - -impl EdgeInfo { - pub fn simple_debug_string(&self, graph: &WorkspaceSnapshotGraphV1) -> String { - let source = graph.graph().node_weight(self.source_node_index); - let target = graph.graph().node_weight(self.target_node_index); - let edge_weight = graph.graph().edge_weight(self.edge_index); - - format!( - "{:?} -- {:?} --> {:?}", - source.map(|s| s.id()), - edge_weight.map(|e| e.kind()), - target.map(|t| t.id()) - ) - } -} diff --git a/lib/dal/src/workspace_snapshot/graph.rs b/lib/dal/src/workspace_snapshot/graph.rs index f01ccf6de1..998f6f3a4c 100644 --- a/lib/dal/src/workspace_snapshot/graph.rs +++ b/lib/dal/src/workspace_snapshot/graph.rs @@ -4,7 +4,7 @@ use std::io::Write; use std::sync::{Arc, Mutex}; use chrono::Utc; -use detect_conflicts_and_updates::DetectConflictsAndUpdates; +use detect_updates::{Detector, Update}; /// Ensure [`NodeIndex`] is usable by external crates. pub use petgraph::graph::NodeIndex; use petgraph::stable_graph::Edges; @@ -27,16 +27,14 @@ use crate::workspace_snapshot::node_weight::category_node_weight::CategoryNodeKi use crate::workspace_snapshot::node_weight::{CategoryNodeWeight, NodeWeightDiscriminants}; use crate::workspace_snapshot::vector_clock::{HasVectorClocks, VectorClockId}; use crate::workspace_snapshot::{ - conflict::Conflict, content_address::ContentAddress, edge_weight::{EdgeWeight, EdgeWeightError, EdgeWeightKind, EdgeWeightKindDiscriminants}, node_weight::{NodeWeight, NodeWeightError, OrderingNodeWeight}, - update::Update, }; use crate::ChangeSetId; pub mod deprecated; -pub mod detect_conflicts_and_updates; +pub mod detect_updates; mod tests; pub type LineageId = Ulid; @@ -131,12 +129,6 @@ pub struct WorkspaceSnapshotGraphV1 { ulid_generator: Arc>, } -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct ConflictsAndUpdates { - pub conflicts: Vec, - pub updates: Vec, -} - #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct RebaseBatch { updates: Vec, @@ -476,8 +468,6 @@ impl WorkspaceSnapshotGraphV1 { pub fn nodes(&self) -> impl Iterator { self.graph.node_indices().filter_map(|node_idx| { self.get_node_weight_opt(node_idx) - .ok() - .flatten() .map(|weight| (weight, node_idx)) }) } @@ -679,15 +669,8 @@ impl WorkspaceSnapshotGraphV1 { self.add_node(self.get_node_weight(node_index_to_copy)?.clone()) } - #[instrument(level = "info", skip_all)] - pub fn detect_conflicts_and_updates( - &self, - to_rebase_vector_clock_id: VectorClockId, - onto: &WorkspaceSnapshotGraphV1, - onto_vector_clock_id: VectorClockId, - ) -> WorkspaceSnapshotGraphResult { - DetectConflictsAndUpdates::new(self, to_rebase_vector_clock_id, onto, onto_vector_clock_id) - .detect_conflicts_and_updates() + pub fn detect_updates(&self, updated_graph: &Self) -> Vec { + Detector::new(self, updated_graph).calculate_updates() } #[allow(dead_code)] @@ -1031,18 +1014,15 @@ impl WorkspaceSnapshotGraphV1 { .map(|node_weight| node_weight.id()) } - pub fn get_node_weight_opt( - &self, - node_index: NodeIndex, - ) -> WorkspaceSnapshotGraphResult> { - Ok(self.graph.node_weight(node_index)) + pub fn get_node_weight_opt(&self, node_index: NodeIndex) -> Option<&NodeWeight> { + self.graph.node_weight(node_index) } pub fn get_node_weight( &self, node_index: NodeIndex, ) -> WorkspaceSnapshotGraphResult<&NodeWeight> { - self.get_node_weight_opt(node_index)? + self.get_node_weight_opt(node_index) .ok_or(WorkspaceSnapshotGraphError::NodeWeightNotFound) } @@ -1293,7 +1273,7 @@ impl WorkspaceSnapshotGraphV1 { ) -> WorkspaceSnapshotGraphResult> { Ok( match self.ordering_node_index_for_container(container_node_index)? { - Some(ordering_node_idx) => match self.get_node_weight_opt(ordering_node_idx)? { + Some(ordering_node_idx) => match self.get_node_weight_opt(ordering_node_idx) { Some(node_weight) => Some(node_weight.get_ordering_node_weight()?.clone()), None => None, }, @@ -1342,18 +1322,11 @@ impl WorkspaceSnapshotGraphV1 { /// [`Self::cleanup()`] has run should be considered invalid. pub fn remove_edge( &mut self, - vector_clock_id: VectorClockId, source_node_index: NodeIndex, target_node_index: NodeIndex, edge_kind: EdgeWeightKindDiscriminants, ) -> WorkspaceSnapshotGraphResult<()> { - self.remove_edge_inner( - vector_clock_id, - source_node_index, - target_node_index, - edge_kind, - true, - ) + self.remove_edge_inner(source_node_index, target_node_index, edge_kind) } /// Removes an edge from `source_node_index` to `target_node_index`, and @@ -1361,11 +1334,9 @@ impl WorkspaceSnapshotGraphV1 { /// the node at `source_node_index`. fn remove_edge_inner( &mut self, - vector_clock_id: VectorClockId, source_node_index: NodeIndex, target_node_index: NodeIndex, edge_kind: EdgeWeightKindDiscriminants, - increment_vector_clocks: bool, ) -> WorkspaceSnapshotGraphResult<()> { let source_node_index = self.get_latest_node_idx(source_node_index)?; let target_node_index = self.get_latest_node_idx(target_node_index)?; @@ -1392,11 +1363,7 @@ impl WorkspaceSnapshotGraphV1 { // We only want to update the ordering of the container if we removed an edge to // one of the ordered relationships. - if new_container_ordering_node_weight.remove_from_order( - vector_clock_id, - element_id, - increment_vector_clocks, - ) { + if new_container_ordering_node_weight.remove_from_order(element_id) { self.remove_edge_of_kind( previous_container_ordering_node_index, target_node_index, @@ -1677,11 +1644,7 @@ impl WorkspaceSnapshotGraphV1 { /// Perform [`Updates`](Update) using [`self`](WorkspaceSnapshotGraph) as the "to rebase" graph /// and a provided graph as the "onto" graph. - pub fn perform_updates( - &mut self, - to_rebase_vector_clock_id: VectorClockId, - updates: &[Update], - ) -> WorkspaceSnapshotGraphResult<()> { + pub fn perform_updates(&mut self, updates: &[Update]) -> WorkspaceSnapshotGraphResult<()> { for update in updates { match update { Update::NewEdge { @@ -1707,16 +1670,7 @@ impl WorkspaceSnapshotGraphV1 { if let (Some(updated_source), Some(destination)) = (updated_source, destination) { - self.remove_edge_inner( - to_rebase_vector_clock_id, - updated_source, - destination, - *edge_kind, - // Updating the vector clocks here may cause us to pick - // the to_rebase node incorrectly in ReplaceNode, if - // ReplaceNode comes after this RemoveEdge - false, - )?; + self.remove_edge_inner(updated_source, destination, *edge_kind)?; } } Update::NewNode { node_weight } => { diff --git a/lib/dal/src/workspace_snapshot/graph/detect_conflicts_and_updates.rs b/lib/dal/src/workspace_snapshot/graph/detect_conflicts_and_updates.rs deleted file mode 100644 index 659282bd7e..0000000000 --- a/lib/dal/src/workspace_snapshot/graph/detect_conflicts_and_updates.rs +++ /dev/null @@ -1,668 +0,0 @@ -use std::collections::{HashMap, HashSet}; - -use petgraph::{prelude::*, visit::DfsEvent}; -use telemetry::prelude::*; - -use si_events::{ulid::Ulid, VectorClockId}; - -use crate::{ - workspace_snapshot::{ - conflict::Conflict, edge_info::EdgeInfo, graph::WorkspaceSnapshotGraphError, - node_weight::NodeWeight, update::Update, vector_clock::HasVectorClocks, NodeInformation, - }, - EdgeWeightKind, WorkspaceSnapshotGraphV1, -}; - -use super::{ConflictsAndUpdates, WorkspaceSnapshotGraphResult}; - -pub struct DetectConflictsAndUpdates<'a, 'b> { - to_rebase_graph: &'a WorkspaceSnapshotGraphV1, - to_rebase_vector_clock_id: VectorClockId, - - onto_graph: &'b WorkspaceSnapshotGraphV1, - onto_vector_clock_id: VectorClockId, -} - -enum ConflictsAndUpdatesControl { - Continue(Vec, Vec), -} - -#[derive(PartialEq, Eq, Clone, Debug)] -enum OntoNodeDifference { - MerkleTreeHash, - NewNode, -} - -impl ConflictsAndUpdatesControl { - fn into_inners(self) -> (petgraph::visit::Control<()>, Vec, Vec) { - match self { - ConflictsAndUpdatesControl::Continue(conflicts, updates) => { - (petgraph::visit::Control::Continue, conflicts, updates) - } - } - } -} - -impl<'a, 'b> DetectConflictsAndUpdates<'a, 'b> { - pub fn new( - to_rebase_graph: &'a WorkspaceSnapshotGraphV1, - to_rebase_vector_clock_id: VectorClockId, - onto_graph: &'b WorkspaceSnapshotGraphV1, - onto_vector_clock_id: VectorClockId, - ) -> Self { - Self { - to_rebase_graph, - to_rebase_vector_clock_id, - onto_graph, - onto_vector_clock_id, - } - } - - pub fn detect_conflicts_and_updates( - &self, - ) -> WorkspaceSnapshotGraphResult { - let mut conflicts: Vec = Vec::new(); - let mut updates: Vec = Vec::new(); - if let Err(traversal_error) = petgraph::visit::depth_first_search( - self.onto_graph.graph(), - Some(self.onto_graph.root()), - |event| { - self.detect_conflicts_and_updates_process_dfs_event( - event, - &mut conflicts, - &mut updates, - ) - }, - ) { - return Err(WorkspaceSnapshotGraphError::GraphTraversal(traversal_error)); - } - - // updates.extend(self.maybe_merge_category_nodes()?); - - // Now that we have the full set of updates to be performed, we can check to see if we'd be - // breaking any "exclusive edge" constraints. We need to wait until after we've detected - // all of the updates to be performed as adding a second "exclusive" edge is only a - // violation of the constraint if we are not also removing the first one. We need to ensure - // that the net result is that there is only one of that edge kind. - - // conflicts.extend(self.detect_exclusive_edge_conflicts_in_updates(&updates)?); - - Ok(ConflictsAndUpdates { conflicts, updates }) - } - - fn detect_conflicts_and_updates_process_dfs_event( - &self, - event: DfsEvent, - conflicts: &mut Vec, - updates: &mut Vec, - ) -> Result, petgraph::visit::DfsEvent> { - match event { - DfsEvent::Discover(onto_node_index, _) => { - let node_diff = self - .onto_node_difference_from_to_rebase(onto_node_index) - .map_err(|err| { - error!( - err=?err, - "Error detecting conflicts and updates for onto {:?}", - onto_node_index, - ); - event - })?; - - Ok(match node_diff { - None => petgraph::visit::Control::Prune, - Some(OntoNodeDifference::NewNode) => { - let node_weight= self.onto_graph.get_node_weight(onto_node_index) - .map_err(|err| { - error!(err=?err, "Error detecting conflicts and updates for onto: {:?}", onto_node_index); - event - })?.to_owned(); - - updates.push(Update::NewNode { node_weight }); - petgraph::visit::Control::Continue - } - Some(OntoNodeDifference::MerkleTreeHash) => petgraph::visit::Control::Continue, - }) - } - DfsEvent::Finish(onto_node_index, _time) => { - // Even though we're pruning in `DfsEvent::Discover`, we'll still get a `Finish` - // for the node where we returned a `petgraph::visit::Control::Prune`. Since we - // already know that there won't be any conflicts/updates with a nodes that have - // identical merkle tree hashes, we can `Continue` - let node_diff = self - .onto_node_difference_from_to_rebase(onto_node_index) - .map_err(|err| { - error!( - err=?err, - "Error detecting conflicts and updates for onto {:?}", - onto_node_index, - ); - event - })?; - if node_diff.is_none() { - return Ok(petgraph::visit::Control::Continue); - } - - let (petgraph_control, node_conflicts, node_updates) = self - .detect_conflicts_and_updates_for_node_index(onto_node_index) - .map_err(|err| { - error!( - err=?err, - "Error detecting conflicts and updates for onto {:?}", - onto_node_index - ); - event - })? - .into_inners(); - - conflicts.extend(node_conflicts); - updates.extend(node_updates); - - Ok(petgraph_control) - } - _ => Ok(petgraph::visit::Control::Continue), - } - } - - fn onto_node_difference_from_to_rebase( - &self, - onto_node_index: NodeIndex, - ) -> WorkspaceSnapshotGraphResult> { - let onto_node_weight = self.onto_graph.get_node_weight(onto_node_index)?; - let mut to_rebase_node_indexes = HashSet::new(); - if onto_node_index == self.onto_graph.root_index { - // There can only be one (valid/current) `ContentAddress::Root` at any - // given moment, and the `lineage_id` isn't really relevant as it's not - // globally stable (even though it is locally stable). This matters as we - // may be dealing with a `WorkspaceSnapshotGraph` that is coming to us - // externally from a module that we're attempting to import. The external - // `WorkspaceSnapshotGraph` will be `self`, and the "local" one will be - // `onto`. - to_rebase_node_indexes.insert(self.to_rebase_graph.root()); - } else { - // Only retain node indexes... or indices... if they are part of the current - // graph. There may still be garbage from previous updates to the graph - // laying around. - let mut potential_to_rebase_node_indexes = self - .to_rebase_graph - .get_node_index_by_lineage(onto_node_weight.lineage_id()); - potential_to_rebase_node_indexes - .retain(|node_index| self.to_rebase_graph.has_path_to_root(*node_index)); - - to_rebase_node_indexes.extend(potential_to_rebase_node_indexes); - } - - if to_rebase_node_indexes.is_empty() { - return Ok(Some(OntoNodeDifference::NewNode)); - } - - // If everything with the same `lineage_id` is identical, then we can prune the - // graph traversal, and avoid unnecessary lookups/comparisons. - let mut any_content_with_lineage_is_different = false; - - for to_rebase_node_index in to_rebase_node_indexes { - let to_rebase_node_weight = - self.to_rebase_graph.get_node_weight(to_rebase_node_index)?; - if onto_node_weight.merkle_tree_hash() == to_rebase_node_weight.merkle_tree_hash() { - // If the merkle tree hashes are the same, then the entire sub-graph is - // identical, and we don't need to check any further. - continue; - } - - any_content_with_lineage_is_different = true - } - - Ok(if any_content_with_lineage_is_different { - Some(OntoNodeDifference::MerkleTreeHash) - } else { - None - }) - } - - fn detect_conflicts_and_updates_for_node_index( - &self, - onto_node_index: NodeIndex, - ) -> WorkspaceSnapshotGraphResult { - let mut conflicts = vec![]; - let mut updates = vec![]; - - let onto_node_weight = self.onto_graph.get_node_weight(onto_node_index)?; - - let mut to_rebase_node_indexes = HashSet::new(); - if onto_node_index == self.onto_graph.root_index { - // There can only be one (valid/current) `ContentAddress::Root` at any - // given moment, and the `lineage_id` isn't really relevant as it's not - // globally stable (even though it is locally stable). This matters as we - // may be dealing with a `WorkspaceSnapshotGraph` that is coming to us - // externally from a module that we're attempting to import. The external - // `WorkspaceSnapshotGraph` will be `self`, and the "local" one will be - // `onto`. - to_rebase_node_indexes.insert(self.to_rebase_graph.root()); - } else { - // Only retain node indexes... or indices... if they are part of the current - // graph. There may still be garbage from previous updates to the graph - // laying around. - let mut potential_to_rebase_node_indexes = self - .to_rebase_graph - .get_node_index_by_lineage(onto_node_weight.lineage_id()); - potential_to_rebase_node_indexes - .retain(|node_index| self.to_rebase_graph.has_path_to_root(*node_index)); - to_rebase_node_indexes.extend(potential_to_rebase_node_indexes); - } - - if to_rebase_node_indexes.is_empty() { - // this node exists in onto, but not in to rebase. We should have - // produced a NewNode update already. Now we just need to produce - // new edge updates for all outgoing edges of this node. - for edgeref in self.onto_graph.edges_directed(onto_node_index, Outgoing) { - let edge_info = EdgeInfo { - source_node_index: edgeref.source(), - target_node_index: edgeref.target(), - edge_kind: edgeref.weight().kind().into(), - edge_index: edgeref.id(), - }; - updates.push(Update::new_edge( - self.onto_graph, - &edge_info, - edgeref.weight().to_owned(), - )?); - } - return Ok(ConflictsAndUpdatesControl::Continue(conflicts, updates)); - } - - for to_rebase_node_index in to_rebase_node_indexes { - let to_rebase_node_weight = - self.to_rebase_graph.get_node_weight(to_rebase_node_index)?; - if onto_node_weight.merkle_tree_hash() == to_rebase_node_weight.merkle_tree_hash() { - // If the merkle tree hashes are the same, then the entire sub-graph is - // identical, and we don't need to check any further. - debug!( - "onto {} and to rebase {} merkle tree hashes are the same", - onto_node_weight.merkle_tree_hash(), - to_rebase_node_weight.merkle_tree_hash() - ); - continue; - } - - // Check if there's a difference in the node itself (and whether it is a - // conflict if there is a difference). - if onto_node_weight.node_hash() != to_rebase_node_weight.node_hash() { - if to_rebase_node_weight - .vector_clock_write() - .is_newer_than(onto_node_weight.vector_clock_write()) - { - // The existing node (`to_rebase`) has changes, but has already seen - // all of the changes in `onto`. There is no conflict, and there is - // nothing to update. - } else if onto_node_weight - .vector_clock_write() - .is_newer_than(to_rebase_node_weight.vector_clock_write()) - { - // `onto` has changes, but has already seen all of the changes in - // `to_rebase`. There is no conflict, and we should update to use the - // `onto` node. - updates.push(Update::ReplaceNode { - node_weight: onto_node_weight.to_owned(), - }); - } else { - // There are changes on both sides that have not - // been seen by the other side; this is a conflict. - // There may also be other conflicts in the outgoing - // relationships, the downstream nodes, or both. - - if let ( - NodeWeight::Ordering(onto_ordering), - NodeWeight::Ordering(to_rebase_ordering), - ) = (onto_node_weight, to_rebase_node_weight) - { - // TODO Checking if two ordering arrays are non conflicting - // (if the common elements between two ordering have the same relative positions) - // is logic that could be extracted into its own thing. The following code does that - - // Both `onto` and `to_rebase` have changes that the other has not incorporated. We - // need to find out what the changes are to see what needs to be updated, and what - // conflicts. - let onto_ordering_set: HashSet = - onto_ordering.order().iter().copied().collect(); - let to_rebase_ordering_set: HashSet = - to_rebase_ordering.order().iter().copied().collect(); - - // Make sure that both `onto` and `to_rebase` have the same relative ordering for the - // nodes they have in common. If they don't, then that means that the order changed on - // at least one of them. - let common_items: HashSet = onto_ordering_set - .intersection(&to_rebase_ordering_set) - .copied() - .collect(); - let common_onto_items = { - let mut items = onto_ordering.order().clone(); - items.retain(|i| common_items.contains(i)); - items - }; - let common_to_rebase_items = { - let mut items = to_rebase_ordering.order().clone(); - items.retain(|i| common_items.contains(i)); - items - }; - if common_onto_items != common_to_rebase_items { - let to_rebase_node_information = NodeInformation { - id: to_rebase_node_weight.id().into(), - node_weight_kind: to_rebase_node_weight.into(), - }; - let onto_node_information = NodeInformation { - id: onto_node_weight.id().into(), - node_weight_kind: onto_node_weight.into(), - }; - - conflicts.push(Conflict::ChildOrder { - to_rebase: to_rebase_node_information, - onto: onto_node_information, - }); - } - } else { - let to_rebase_node_information = NodeInformation { - id: to_rebase_node_weight.id().into(), - node_weight_kind: to_rebase_node_weight.into(), - }; - let onto_node_information = NodeInformation { - id: onto_node_weight.id().into(), - node_weight_kind: onto_node_weight.into(), - }; - - conflicts.push(Conflict::NodeContent { - to_rebase: to_rebase_node_information, - onto: onto_node_information, - }); - } - } - } - - let (container_conflicts, container_updates) = self - .find_container_membership_conflicts_and_updates( - to_rebase_node_index, - onto_node_index, - )?; - - updates.extend(container_updates); - conflicts.extend(container_conflicts); - } - - // This function is run in `DfsEvent::Finish`, so regardless of whether there are any - // updates/conflicts, we need to return `Continue`. We shouldn't ever get here if there - // aren't any differences at all, as we prune the graph during `DfsEvent::Discover`, but - // the differences might not result in any changes/conflicts in the direction we're doing - // the comparison. - - Ok(ConflictsAndUpdatesControl::Continue(conflicts, updates)) - } - - fn find_container_membership_conflicts_and_updates( - &self, - to_rebase_container_index: NodeIndex, - onto_container_index: NodeIndex, - ) -> WorkspaceSnapshotGraphResult<(Vec, Vec)> { - #[derive(Debug, Clone, Hash, PartialEq, Eq)] - struct UniqueEdgeInfo { - pub kind: EdgeWeightKind, - pub target_lineage: Ulid, - } - - let mut updates = Vec::new(); - let mut conflicts = Vec::new(); - - let mut to_rebase_edges = HashMap::::new(); - for edgeref in self - .to_rebase_graph - .graph() - .edges_directed(to_rebase_container_index, Outgoing) - { - let target_node_weight = self.to_rebase_graph.get_node_weight(edgeref.target())?; - - to_rebase_edges.insert( - UniqueEdgeInfo { - kind: edgeref.weight().kind().clone(), - target_lineage: target_node_weight.lineage_id(), - }, - EdgeInfo { - source_node_index: edgeref.source(), - target_node_index: edgeref.target(), - edge_kind: edgeref.weight().kind().into(), - edge_index: edgeref.id(), - }, - ); - } - - let mut onto_edges = HashMap::::new(); - for edgeref in self - .onto_graph - .graph - .edges_directed(onto_container_index, Outgoing) - { - let target_node_weight = self.onto_graph.get_node_weight(edgeref.target())?; - - onto_edges.insert( - UniqueEdgeInfo { - kind: edgeref.weight().kind().clone(), - target_lineage: target_node_weight.lineage_id(), - }, - EdgeInfo { - source_node_index: edgeref.source(), - target_node_index: edgeref.target(), - edge_kind: edgeref.weight().kind().into(), - edge_index: edgeref.id(), - }, - ); - } - - let only_to_rebase_edges = { - let mut unique_edges = to_rebase_edges.clone(); - for key in onto_edges.keys() { - unique_edges.remove(key); - } - unique_edges - }; - let only_onto_edges = { - let mut unique_edges = onto_edges.clone(); - for key in to_rebase_edges.keys() { - unique_edges.remove(key); - } - unique_edges - }; - - debug!("only to rebase edges: {:?}", &only_to_rebase_edges); - debug!("only onto edges: {:?}", &only_onto_edges); - - // This is the last time that to_rebase knows about onto having seen the snapshot - let to_rebase_root_node = self - .to_rebase_graph - .get_node_weight(self.to_rebase_graph.root())?; - let onto_root_node = self.onto_graph.get_node_weight(self.onto_graph.root())?; - - let to_rebase_last_seen_by_onto_vector_clock_id = to_rebase_root_node - .vector_clock_recently_seen() - .entry_for(self.onto_vector_clock_id); - - let onto_last_seen_by_to_rebase_vector_clock = onto_root_node - .vector_clock_recently_seen() - .entry_for(self.to_rebase_vector_clock_id); - - for only_to_rebase_edge_info in only_to_rebase_edges.values() { - let to_rebase_edge_weight = self - .to_rebase_graph - .get_edge_weight_opt(only_to_rebase_edge_info.edge_index)? - .ok_or(WorkspaceSnapshotGraphError::EdgeWeightNotFound)?; - let to_rebase_item_weight = self - .to_rebase_graph - .get_node_weight(only_to_rebase_edge_info.target_node_index)?; - - // This is an edge that is only to_rebase. So either: - // -- Onto has seen this edge: - // -- So, if to_rebase has modified the target of the edge since onto last saw the target, - // we should produce a ModifyRemovedItem conflict, - // -- OR, if to_rebase has *not* modified the target, we should produce a RemoveEdge update. - // -- Onto has never seen this edge, becuase it was added *after* onto was forked from to_rebase: - // -- So, either we should just silently let the edge stay in to_rebase, OR, we should produce - // an exclusive edge conflict if silently adding this edge would lead to an incorrect - // graph. - - // This will always be Some(_) - if let Some(edge_first_seen_by_to_rebase) = to_rebase_edge_weight - .vector_clock_first_seen() - .entry_for(self.to_rebase_vector_clock_id) - { - let maybe_seen_by_onto_at = if let Some(onto_last_seen_by_to_rebase_vector_clock) = - onto_last_seen_by_to_rebase_vector_clock - { - if edge_first_seen_by_to_rebase <= onto_last_seen_by_to_rebase_vector_clock { - Some(onto_last_seen_by_to_rebase_vector_clock) - } else { - None - } - } else { - to_rebase_edge_weight - .vector_clock_recently_seen() - .entry_for(self.onto_vector_clock_id) - .or_else(|| { - to_rebase_edge_weight - .vector_clock_first_seen() - .entry_for(self.onto_vector_clock_id) - }) - }; - - if let Some(seen_by_onto_at) = maybe_seen_by_onto_at { - if to_rebase_item_weight - .vector_clock_write() - .has_entries_newer_than(seen_by_onto_at) - { - // Item has been modified in `to_rebase` since - // `onto` last saw `to_rebase` - let node_information = NodeInformation { - id: to_rebase_item_weight.id().into(), - node_weight_kind: to_rebase_item_weight.into(), - }; - let container_node_weight = self - .to_rebase_graph - .get_node_weight(to_rebase_container_index)?; - let container_node_information = NodeInformation { - id: container_node_weight.id().into(), - node_weight_kind: container_node_weight.into(), - }; - - conflicts.push(Conflict::ModifyRemovedItem { - container: container_node_information, - modified_item: node_information, - }); - } else { - let source_node_weight = self - .to_rebase_graph - .get_node_weight(only_to_rebase_edge_info.source_node_index)?; - let target_node_weight = self - .to_rebase_graph - .get_node_weight(only_to_rebase_edge_info.target_node_index)?; - let source_node_information = NodeInformation { - id: source_node_weight.id().into(), - node_weight_kind: source_node_weight.into(), - }; - let target_node_information = NodeInformation { - id: target_node_weight.id().into(), - node_weight_kind: target_node_weight.into(), - }; - updates.push(Update::RemoveEdge { - source: source_node_information, - destination: target_node_information, - edge_kind: only_to_rebase_edge_info.edge_kind, - }); - } - } - } - } - - // - Items unique to `onto`: - for only_onto_edge_info in only_onto_edges.values() { - let onto_edge_weight = self - .onto_graph - .get_edge_weight_opt(only_onto_edge_info.edge_index)? - .ok_or(WorkspaceSnapshotGraphError::EdgeWeightNotFound)?; - let onto_item_weight = self - .onto_graph - .get_node_weight(only_onto_edge_info.target_node_index)?; - - // This is an edge that is only in onto, so, either: - // -- to_rebase has never seen this edge, so we should produce a New Edge update - // -- OR, to_rebase has seen this edge, so: - // -- if onto has modified the target of the edge since the last time onto saw to - // rebase, we should produce a RemoveModifiedItem conflict - // -- OR, onto has not modified the edge, so the edge should stay removed (no - // update necessary since it's already gone in to_rebase) - - // this will always be Some(_) - if let Some(edge_weight_first_seen_by_onto) = onto_edge_weight - .vector_clock_first_seen() - .entry_for(self.onto_vector_clock_id) - { - let maybe_seen_by_to_rebase_at = - if let Some(to_rebase_last_seen_by_onto_vector_clock_id) = - to_rebase_last_seen_by_onto_vector_clock_id - { - if edge_weight_first_seen_by_onto - <= to_rebase_last_seen_by_onto_vector_clock_id - { - Some(to_rebase_last_seen_by_onto_vector_clock_id) - } else { - None - } - } else { - onto_edge_weight - .vector_clock_recently_seen() - .entry_for(self.to_rebase_vector_clock_id) - .or_else(|| { - onto_edge_weight - .vector_clock_first_seen() - .entry_for(self.to_rebase_vector_clock_id) - }) - }; - - match maybe_seen_by_to_rebase_at { - Some(seen_by_to_rebase_at) => { - if onto_item_weight - .vector_clock_write() - .has_entries_newer_than(seen_by_to_rebase_at) - { - let container_node_weight = self - .to_rebase_graph - .get_node_weight(to_rebase_container_index)?; - let onto_node_weight = self - .onto_graph - .get_node_weight(only_onto_edge_info.target_node_index)?; - let _container_node_information = NodeInformation { - id: container_node_weight.id().into(), - node_weight_kind: container_node_weight.into(), - }; - let _removed_item_node_information = NodeInformation { - id: onto_node_weight.id().into(), - node_weight_kind: onto_node_weight.into(), - }; - - // NOTE: The clocks don't actually matter anymore. -- Adam & Fletcher - //conflicts.push(Conflict::RemoveModifiedItem { - // container: container_node_information, - // removed_item: removed_item_node_information, - //}); - } - } - None => { - // This edge has never been seen by to_rebase - updates.push(Update::new_edge( - self.onto_graph, - only_onto_edge_info, - onto_edge_weight.to_owned(), - )?); - } - } - } - } - - // - Sets same: No conflicts/updates - Ok((conflicts, updates)) - } -} diff --git a/lib/dal/src/workspace_snapshot/graph/detect_updates.rs b/lib/dal/src/workspace_snapshot/graph/detect_updates.rs new file mode 100644 index 0000000000..1799cb832f --- /dev/null +++ b/lib/dal/src/workspace_snapshot/graph/detect_updates.rs @@ -0,0 +1,351 @@ +use std::collections::{HashMap, HashSet}; + +use petgraph::{ + prelude::*, + visit::{Control, DfsEvent}, +}; +use serde::{Deserialize, Serialize}; +use si_events::ulid::Ulid; +use strum::EnumDiscriminants; + +use crate::{ + workspace_snapshot::{node_weight::NodeWeight, NodeInformation}, + EdgeWeight, EdgeWeightKind, EdgeWeightKindDiscriminants, +}; + +use super::WorkspaceSnapshotGraphV1; + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, EnumDiscriminants)] +pub enum Update { + NewEdge { + source: NodeInformation, + destination: NodeInformation, + edge_weight: EdgeWeight, + }, + RemoveEdge { + source: NodeInformation, + destination: NodeInformation, + edge_kind: EdgeWeightKindDiscriminants, + }, + ReplaceNode { + node_weight: NodeWeight, + }, + NewNode { + node_weight: NodeWeight, + }, +} + +#[derive(Clone, Debug)] +enum NodeDifference { + NewNode, + MerkleTreeHash(Vec), +} + +pub struct Detector<'a, 'b> { + base_graph: &'a WorkspaceSnapshotGraphV1, + updated_graph: &'b WorkspaceSnapshotGraphV1, +} + +impl<'a, 'b> Detector<'a, 'b> { + pub fn new( + base_graph: &'a WorkspaceSnapshotGraphV1, + updated_graph: &'b WorkspaceSnapshotGraphV1, + ) -> Self { + Self { + base_graph, + updated_graph, + } + } + + fn node_diff_from_base_graph( + &self, + updated_graph_node_index: NodeIndex, + ) -> Option { + self.updated_graph + .get_node_weight_opt(updated_graph_node_index) + .and_then(|updated_graph_node_weight| { + let mut base_graph_node_indexes = HashSet::new(); + if updated_graph_node_index == self.updated_graph.root_index { + // There can only be one (valid/current) `ContentAddress::Root` at any + // given moment, and the `lineage_id` isn't really relevant as it's not + // globally stable (even though it is locally stable). This matters as we + // may be dealing with a `WorkspaceSnapshotGraph` that is coming to us + // externally from a module that we're attempting to import. The external + // `WorkspaceSnapshotGraph` will be `self`, and the "local" one will be + // `onto`. + base_graph_node_indexes.insert(self.base_graph.root()); + } else { + // Only retain node indexes... or indices... if they are part of the current + // graph. There may still be garbage from previous updates to the graph + // laying around. + let mut potential_base_graph_node_indexes = self + .base_graph + .get_node_index_by_lineage(updated_graph_node_weight.lineage_id()); + potential_base_graph_node_indexes + .retain(|node_index| self.base_graph.has_path_to_root(*node_index)); + + base_graph_node_indexes.extend(potential_base_graph_node_indexes); + } + + base_graph_node_indexes + .is_empty() + .then_some(NodeDifference::NewNode) + .or_else(|| { + // If everything with the same `lineage_id` is identical, then + // we can prune the graph traversal, and avoid unnecessary + // lookups/comparisons. + let nodes_with_difference: Vec = base_graph_node_indexes + .iter() + .filter_map(|&base_graph_index| { + self.base_graph + .get_node_weight_opt(base_graph_index) + .and_then(|base_graph_node_weight| { + (base_graph_node_weight.merkle_tree_hash() + != updated_graph_node_weight.merkle_tree_hash()) + .then_some(base_graph_index) + }) + }) + .collect(); + + (!nodes_with_difference.is_empty()) + .then_some(NodeDifference::MerkleTreeHash(nodes_with_difference)) + }) + }) + } + + /// Produces ReplaceNode, NewEdge and RemoveEdge updates. The assumption we + /// make here is that updated_graph has seen everything in base_graph. So if + /// a node has a different hash from the matching one in base_graph, it has + /// been changed and should be replaced. And if an edge is in base_graph but + /// not in updated_graph, that means it's been removed. Finally, if an edge + /// is in new_graph, but not in base_graph, that means it has been added. + fn detect_updates_for_node_index( + &self, + updated_graph_node_index: NodeIndex, + base_graph_indexes: &[NodeIndex], + ) -> Vec { + #[derive(Debug, Clone)] + struct EdgeInfo { + pub source_node: NodeInformation, + pub target_node: NodeInformation, + pub edge_weight: EdgeWeight, + } + + #[derive(Debug, Clone, Hash, PartialEq, Eq)] + struct UniqueEdgeInfo { + pub kind: EdgeWeightKind, + pub target_lineage: Ulid, + } + + let mut updates = vec![]; + + if let Some(updated_graph_node_weight) = self + .updated_graph + .get_node_weight_opt(updated_graph_node_index) + { + for base_graph_index in base_graph_indexes { + let base_graph_index = *base_graph_index; + + if let Some(base_graph_node_weight) = + self.base_graph.get_node_weight_opt(base_graph_index) + { + // if the node hash is different, then the node has been updated and + // needs to be replaced in base_graph (node hash is a hash of the + // content, which is not the same as the merkle tree hash, which + // also gathers up the hashes of the outgoing neighbors) + if updated_graph_node_weight.node_hash() != base_graph_node_weight.node_hash() { + updates.push(Update::ReplaceNode { + node_weight: updated_graph_node_weight.to_owned(), + }); + } + + let base_graph_edges: HashMap = self + .base_graph + .graph() + .edges_directed(base_graph_index, Outgoing) + .filter_map(|edge_ref| { + self.base_graph.get_node_weight_opt(edge_ref.target()).map( + |target_node_weight| { + ( + UniqueEdgeInfo { + kind: edge_ref.weight().kind().clone(), + target_lineage: target_node_weight.lineage_id(), + }, + EdgeInfo { + source_node: NodeInformation { + id: base_graph_node_weight.id().into(), + node_weight_kind: base_graph_node_weight.into(), + }, + target_node: NodeInformation { + id: target_node_weight.id().into(), + node_weight_kind: target_node_weight.into(), + }, + edge_weight: edge_ref.weight().to_owned(), + }, + ) + }, + ) + }) + .collect(); + + let update_graph_edges: HashMap = self + .updated_graph + .graph + .edges_directed(updated_graph_node_index, Outgoing) + .filter_map(|edge_ref| { + self.updated_graph + .get_node_weight_opt(edge_ref.target()) + .map(|target_node_weight| { + ( + UniqueEdgeInfo { + kind: edge_ref.weight().kind().clone(), + target_lineage: target_node_weight.lineage_id(), + }, + EdgeInfo { + source_node: NodeInformation { + id: updated_graph_node_weight.id().into(), + node_weight_kind: updated_graph_node_weight.into(), + }, + target_node: NodeInformation { + id: target_node_weight.id().into(), + node_weight_kind: target_node_weight.into(), + }, + edge_weight: edge_ref.weight().to_owned(), + }, + ) + }) + }) + .collect(); + + updates.extend( + base_graph_edges + .iter() + .filter(|(edge_key, _)| !update_graph_edges.contains_key(edge_key)) + .map(|(_, edge_info)| Update::RemoveEdge { + source: edge_info.source_node, + destination: edge_info.target_node, + edge_kind: edge_info.edge_weight.kind().into(), + }), + ); + + updates.extend( + update_graph_edges + .into_iter() + .filter(|(edge_key, _)| !base_graph_edges.contains_key(edge_key)) + .map(|(_, edge_info)| Update::NewEdge { + source: edge_info.source_node, + destination: edge_info.target_node, + edge_weight: edge_info.edge_weight, + }), + ); + } + } + } + + updates + } + + fn calculate_updates_dfs_event( + &self, + event: DfsEvent, + updates: &mut Vec, + difference_cache: &mut HashMap>, + ) -> Control<()> { + match event { + DfsEvent::Discover(updated_graph_index, _) => { + let node_diff = self.node_diff_from_base_graph(updated_graph_index); + let control = match &node_diff { + Some(NodeDifference::NewNode) => { + if let Some(node_weight) = + self.updated_graph.get_node_weight_opt(updated_graph_index) + { + // NewNode updates are produced here, so that they + // are in the update array *before* the new edge + // updates which refer to them + updates.push(Update::NewNode { + node_weight: node_weight.to_owned(), + }); + } + + Control::Continue + } + Some(NodeDifference::MerkleTreeHash(_)) => Control::Continue, + // Node is neither different, nor new, prune this branch of + // the graph + None => Control::Prune, + }; + + difference_cache.insert(updated_graph_index, node_diff); + + control + } + DfsEvent::Finish(updated_graph_index, _) => { + match difference_cache.get(&updated_graph_index) { + // None should be unreachable here.... + None | Some(None) => Control::Continue, + Some(Some(diff)) => { + match diff { + NodeDifference::NewNode => { + // A new node! Just gather up all the outgoing edges as NewEdge updates + updates.extend( + self.updated_graph + .edges_directed(updated_graph_index, Outgoing) + .map(|edge_ref| { + (edge_ref.target(), edge_ref.weight().to_owned()) + }) + .filter_map(move |(target_index, edge_weight)| { + if let (Some(source_node), Some(destination_node)) = ( + self.updated_graph + .get_node_weight_opt(updated_graph_index), + self.updated_graph + .get_node_weight_opt(target_index), + ) { + Some(Update::NewEdge { + source: NodeInformation { + node_weight_kind: source_node.into(), + id: source_node.id().into(), + }, + destination: NodeInformation { + node_weight_kind: destination_node.into(), + id: destination_node.id().into(), + }, + edge_weight, + }) + } else { + None + } + }), + ); + } + NodeDifference::MerkleTreeHash(base_graph_indexes) => { + updates.extend(self.detect_updates_for_node_index( + updated_graph_index, + base_graph_indexes, + )); + } + } + + Control::Continue + } + } + } + _ => Control::Continue, + } + } + + /// Performs a post order walk of the updated graph, finding the updates + /// made to it when compared to the base graph, using the Merkle tree hash + /// to detect changes and ignore unchanged branches. + pub fn calculate_updates(&self) -> Vec { + let mut updates = vec![]; + let mut difference_cache = HashMap::new(); + + petgraph::visit::depth_first_search( + self.updated_graph.graph(), + Some(self.updated_graph.root()), + |event| self.calculate_updates_dfs_event(event, &mut updates, &mut difference_cache), + ); + + updates + } +} diff --git a/lib/dal/src/workspace_snapshot/graph/tests.rs b/lib/dal/src/workspace_snapshot/graph/tests.rs index 80940ccbed..ce7183448c 100644 --- a/lib/dal/src/workspace_snapshot/graph/tests.rs +++ b/lib/dal/src/workspace_snapshot/graph/tests.rs @@ -105,9 +105,8 @@ mod test { use crate::workspace_snapshot::edge_weight::{ EdgeWeight, EdgeWeightKind, EdgeWeightKindDiscriminants, }; - use crate::workspace_snapshot::graph::ConflictsAndUpdates; + use crate::workspace_snapshot::graph::detect_updates::Update; use crate::workspace_snapshot::node_weight::NodeWeight; - use crate::workspace_snapshot::update::Update; use crate::WorkspaceSnapshotGraphV1; use crate::{ComponentId, FuncId, PropId, SchemaId, SchemaVariantId}; @@ -1289,7 +1288,6 @@ mod test { graph_with_deleted_edge .remove_edge( - new_vector_clock_id, graph_with_deleted_edge .get_node_index_by_id(schema_id) .expect("Unable to get NodeIndex for schema"), @@ -1320,15 +1318,8 @@ mod test { .mark_graph_seen(new_vector_clock_id) .expect("Unable to mark new graph as seen"); - let ConflictsAndUpdates { conflicts, updates } = graph - .detect_conflicts_and_updates( - initial_vector_clock_id, - &graph_with_deleted_edge, - new_vector_clock_id, - ) - .expect("Failed to detect conflicts and updates"); + let updates = graph.detect_updates(&graph_with_deleted_edge); - assert!(conflicts.is_empty()); assert_eq!(1, updates.len()); assert!(matches!( @@ -1436,7 +1427,6 @@ mod test { graph .remove_edge( - vector_clock_id, graph .get_node_index_by_id(schema_id) .expect("Unable to get NodeIndex for schema"), @@ -1705,7 +1695,6 @@ mod test { graph .remove_edge( - vector_clock_id, graph .get_node_index_by_id(root_prop_id) .expect("Unable to get NodeIndex for prop"), diff --git a/lib/dal/src/workspace_snapshot/graph/tests/detect_conflicts_and_updates.rs b/lib/dal/src/workspace_snapshot/graph/tests/detect_conflicts_and_updates.rs index d267a0ca8a..d4f74ef028 100644 --- a/lib/dal/src/workspace_snapshot/graph/tests/detect_conflicts_and_updates.rs +++ b/lib/dal/src/workspace_snapshot/graph/tests/detect_conflicts_and_updates.rs @@ -13,143 +13,12 @@ mod test { use crate::workspace_snapshot::edge_weight::{ EdgeWeight, EdgeWeightKind, EdgeWeightKindDiscriminants, }; - use crate::workspace_snapshot::graph::tests::add_prop_nodes_to_graph; + use crate::workspace_snapshot::graph::detect_updates::Update; use crate::workspace_snapshot::node_weight::NodeWeight; - use crate::workspace_snapshot::update::Update; - use crate::workspace_snapshot::{ - conflict::Conflict, graph::ConflictsAndUpdates, NodeInformation, - }; + use crate::workspace_snapshot::NodeInformation; use crate::NodeWeightDiscriminants; use crate::{PropKind, WorkspaceSnapshotGraphV1}; - fn get_root_node_info(graph: &WorkspaceSnapshotGraphV1) -> NodeInformation { - let root_id = graph - .get_node_weight(graph.root_index) - .expect("Unable to get root node") - .id(); - - NodeInformation { - node_weight_kind: NodeWeightDiscriminants::Content, - id: root_id.into(), - } - } - - #[test] - fn detect_conflicts_and_updates_simple_no_conflicts_no_updates_in_base() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut initial_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let schema_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let schema_variant_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); - - initial_graph - .add_edge( - initial_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - initial_graph - .add_edge( - initial_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); - - initial_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("unable to mark seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = initial_graph.clone(); - - let component_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); - let component_index = new_graph - .add_node( - NodeWeight::new_content( - new_vector_clock_id, - component_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Component A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component A"); - new_graph - .add_edge( - new_graph.root_index, - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - component_index, - ) - .expect("Unable to add root -> component edge"); - new_graph - .add_edge( - new_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - new_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); - - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("unable to mark seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates( - new_vector_clock_id, - &initial_graph, - initial_vector_clock_id, - ) - .expect("Unable to detect conflicts and updates"); - - assert_eq!( - ConflictsAndUpdates { - conflicts: Vec::new(), - updates: Vec::new() - }, - conflicts_and_updates - ); - } - #[test] fn detect_conflicts_and_updates_simple_no_conflicts_with_purely_new_content_in_base() { let actor_id = Ulid::new(); @@ -248,16 +117,12 @@ mod test { .mark_graph_seen(new_vector_clock_id) .expect("unable to mark seen"); - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(new_vector_clock_id, &base_graph, initial_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert!(conflicts_and_updates.conflicts.is_empty()); + let updates = new_graph.detect_updates(&base_graph); let _new_onto_component_index = base_graph .get_node_index_by_id(new_onto_component_id) .expect("Unable to get NodeIndex"); - match conflicts_and_updates.updates.as_slice() { + match updates.as_slice() { [Update::NewNode { .. }, Update::NewEdge { edge_weight, .. }, Update::NewEdge { .. }] => { assert_eq!(&EdgeWeightKind::new_use(), edge_weight.kind()); @@ -267,671 +132,78 @@ mod test { } #[test] - fn detect_conflicts_and_updates_with_purely_new_content_in_new_graph() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - - let mut base_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let component_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let component_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - component_index, - ) - .expect("Unable to add root -> component edge"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("unable to mark seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = base_graph.clone(); - - let new_component_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); - let new_component_index = new_graph - .add_node( - NodeWeight::new_content( - new_vector_clock_id, - new_component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component B")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component B"); - new_graph - .add_edge( - new_graph.root_index, - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - new_component_index, - ) - .expect("Unable to add root -> component edge"); - - new_graph.cleanup(); - new_graph.dot(); - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("unable to mark seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(new_vector_clock_id, &base_graph, initial_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert!(conflicts_and_updates.updates.is_empty()); - assert!(conflicts_and_updates.conflicts.is_empty()); - - let conflicts_and_updates = base_graph - .detect_conflicts_and_updates(initial_vector_clock_id, &new_graph, new_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert!(conflicts_and_updates.conflicts.is_empty()); - - match conflicts_and_updates.updates.as_slice() { - [Update::NewNode { .. }, Update::NewEdge { edge_weight, .. }] => { - assert_eq!(&EdgeWeightKind::new_use(), edge_weight.kind()); - } - other => panic!("Unexpected updates: {:?}", other), - } - } - - #[test] - fn detect_conflicts_and_updates_simple_no_conflicts_with_updates_on_both_sides() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut base_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_variant_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); - - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); - - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("unable to mark seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = base_graph.clone(); - - let component_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); - let component_index = new_graph - .add_node( - NodeWeight::new_content( - new_vector_clock_id, - component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component A"); - new_graph - .add_edge( - new_graph.root_index, - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - component_index, - ) - .expect("Unable to add root -> component edge"); - new_graph - .add_edge( - new_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - new_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); - - new_graph.dot(); - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("unable to mark graph seen"); - - let new_onto_component_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); - let new_onto_component_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - new_onto_component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component B")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component B"); - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - new_onto_component_index, - ) - .expect("Unable to add root -> component edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(new_onto_component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - base_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); - - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("unable to mark graph seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(new_vector_clock_id, &base_graph, initial_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert!(conflicts_and_updates.conflicts.is_empty()); - - let _new_onto_component_index = base_graph - .get_node_index_by_id(new_onto_component_id) - .expect("Unable to get NodeIndex"); - match conflicts_and_updates.updates.as_slice() { - [Update::NewNode { .. }, Update::NewEdge { edge_weight, .. }, Update::NewEdge { .. }] => - { - assert_eq!(&EdgeWeightKind::new_use(), edge_weight.kind()); - } - other => panic!("Unexpected updates: {:?}", other), - } - } - - #[test] - fn detect_conflicts_and_updates_simple_with_content_conflict() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut base_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_variant_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); - - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); - - let component_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let component_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component A"); - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - component_index, - ) - .expect("Unable to add root -> component edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - base_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("mark graph seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = base_graph.clone(); - - new_graph - .update_content( - new_vector_clock_id, - component_id, - ContentHash::from("Updated Component A"), - ) - .expect("Unable to update Component A"); - - new_graph.cleanup(); - new_graph.dot(); - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("mark graph seen"); - - base_graph - .update_content( - initial_vector_clock_id, - component_id, - ContentHash::from("Base Updated Component A"), - ) - .expect("Unable to update Component A"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("mark graph seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(new_vector_clock_id, &base_graph, initial_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert_eq!( - vec![Conflict::NodeContent { - onto: NodeInformation { - id: component_id.into(), - node_weight_kind: NodeWeightDiscriminants::Content, - }, - to_rebase: NodeInformation { - id: component_id.into(), - node_weight_kind: NodeWeightDiscriminants::Content, - }, - }], - conflicts_and_updates.conflicts - ); - assert!(conflicts_and_updates.updates.is_empty()); - } - - #[test] - fn detect_conflicts_and_updates_simple_with_modify_removed_item_conflict() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut base_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_variant_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); - - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); - - let component_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let component_index = base_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - component_id, - Ulid::new(), - ContentAddress::Component(ContentHash::from("Component A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Component A"); - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - component_index, - ) - .expect("Unable to add root -> component edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - base_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("mark graph seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = base_graph.clone(); - - base_graph - .remove_edge( - initial_vector_clock_id, - base_graph.root_index, - base_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeightKindDiscriminants::Use, - ) - .expect("Unable to remove Component A"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("mark graph seen"); - - new_graph - .update_content( - new_vector_clock_id, - component_id, - ContentHash::from("Updated Component A"), - ) - .expect("Unable to update Component A"); - - new_graph.cleanup(); - new_graph.dot(); - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("unable to mark graph seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(new_vector_clock_id, &base_graph, initial_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - let container = get_root_node_info(&new_graph); - - assert_eq!( - vec![Conflict::ModifyRemovedItem { - container, - modified_item: NodeInformation { - id: component_id.into(), - node_weight_kind: NodeWeightDiscriminants::Content, - } - }], - conflicts_and_updates.conflicts - ); - assert!(conflicts_and_updates.updates.is_empty()); - } - - #[test] - fn detect_conflicts_and_updates_simple_with_modify_removed_item_conflict_same_vector_clocks() { - let actor_id = Ulid::new(); - let vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut base_graph = WorkspaceSnapshotGraphV1::new(vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_index = base_graph - .add_node( - NodeWeight::new_content( - vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); - let schema_variant_index = base_graph - .add_node( - NodeWeight::new_content( - vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); + fn detect_conflicts_and_updates_with_purely_new_content_in_new_graph() { + let actor_id = Ulid::new(); + let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - base_graph - .add_edge( - base_graph.root_index, - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); + let mut base_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) + .expect("Unable to create WorkspaceSnapshotGraph"); let component_id = base_graph.generate_ulid().expect("Unable to generate Ulid"); let component_index = base_graph .add_node( NodeWeight::new_content( - vector_clock_id, + initial_vector_clock_id, component_id, Ulid::new(), ContentAddress::Component(ContentHash::from("Component A")), ) .expect("Unable to create NodeWeight"), ) - .expect("Unable to add Component A"); + .expect("Unable to add Schema A"); base_graph .add_edge( base_graph.root_index, - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) + EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) .expect("Unable to create EdgeWeight"), component_index, ) .expect("Unable to add root -> component edge"); - base_graph - .add_edge( - base_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - base_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - ) - .expect("Unable to add component -> schema variant edge"); base_graph.cleanup(); base_graph.dot(); base_graph - .mark_graph_seen(vector_clock_id) - .expect("mark graph seen"); + .mark_graph_seen(initial_vector_clock_id) + .expect("unable to mark seen"); + let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); let mut new_graph = base_graph.clone(); - base_graph - .remove_edge( - vector_clock_id, - base_graph.root_index, - base_graph - .get_node_index_by_id(component_id) - .expect("Unable to get NodeIndex"), - EdgeWeightKindDiscriminants::Use, + let new_component_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); + let new_component_index = new_graph + .add_node( + NodeWeight::new_content( + new_vector_clock_id, + new_component_id, + Ulid::new(), + ContentAddress::Component(ContentHash::from("Component B")), + ) + .expect("Unable to create NodeWeight"), ) - .expect("Unable to remove Component A"); - - base_graph.cleanup(); - base_graph.dot(); - base_graph - .mark_graph_seen(vector_clock_id) - .expect("mark graph seen"); - + .expect("Unable to add Component B"); new_graph - .update_content( - vector_clock_id, - component_id, - ContentHash::from("Updated Component A"), + .add_edge( + new_graph.root_index, + EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) + .expect("Unable to create EdgeWeight"), + new_component_index, ) - .expect("Unable to update Component A"); + .expect("Unable to add root -> component edge"); new_graph.cleanup(); new_graph.dot(); new_graph - .mark_graph_seen(vector_clock_id) - .expect("unable to mark graph seen"); - - let conflicts_and_updates = new_graph - .detect_conflicts_and_updates(vector_clock_id, &base_graph, vector_clock_id) - .expect("Unable to detect conflicts and updates"); + .mark_graph_seen(new_vector_clock_id) + .expect("unable to mark seen"); - let container = get_root_node_info(&new_graph); + let updates = base_graph.detect_updates(&new_graph); - // Even though we have identical vector clocks, this still produces a - // conflict, since this item has been modified in to_rebase after onto - // removed it. - assert_eq!( - vec![Conflict::ModifyRemovedItem { - container, - modified_item: NodeInformation { - id: component_id.into(), - node_weight_kind: NodeWeightDiscriminants::Content, - } - }], - conflicts_and_updates.conflicts - ); - assert!(conflicts_and_updates.updates.is_empty()); + match updates.as_slice() { + [Update::NewNode { .. }, Update::NewEdge { edge_weight, .. }] => { + assert_eq!(&EdgeWeightKind::new_use(), edge_weight.kind()); + } + other => panic!("Unexpected updates: {:?}", other), + } } #[test] @@ -1079,11 +351,7 @@ mod test { .expect("unable to mark graph seen"); // Assert that the new edge to the prototype gets created - let ConflictsAndUpdates { conflicts, updates } = base_graph - .detect_conflicts_and_updates(initial_vector_clock_id, new_graph, new_vector_clock_id) - .expect("Unable to detect conflicts and updates"); - - assert!(conflicts.is_empty()); + let updates = base_graph.detect_updates(new_graph); match updates.as_slice() { [Update::NewEdge { @@ -1099,415 +367,6 @@ mod test { } } - #[test] - fn detect_conflicts_and_updates_simple_ordering_no_conflicts_no_updates_in_base() { - let actor_id = Ulid::new(); - let initial_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut initial_graph = WorkspaceSnapshotGraphV1::new(initial_vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let schema_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let schema_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_id, - Ulid::new(), - ContentAddress::Schema(ContentHash::from("Schema A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema A"); - let schema_variant_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let schema_variant_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - schema_variant_id, - Ulid::new(), - ContentAddress::SchemaVariant(ContentHash::from("Schema Variant A")), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add Schema Variant A"); - - initial_graph - .add_edge( - initial_graph.root_index, - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_index, - ) - .expect("Unable to add root -> schema edge"); - initial_graph - .add_edge( - initial_graph - .get_node_index_by_id(schema_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - schema_variant_index, - ) - .expect("Unable to add schema -> schema variant edge"); - - let container_prop_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let container_prop_index = initial_graph - .add_ordered_node( - initial_vector_clock_id, - NodeWeight::new_content( - initial_vector_clock_id, - container_prop_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - container_prop_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add container prop"); - initial_graph - .add_edge( - initial_graph - .get_node_index_by_id(schema_variant_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - container_prop_index, - ) - .expect("Unable to add schema variant -> container prop edge"); - - let ordered_prop_1_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let ordered_prop_1_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - ordered_prop_1_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - ordered_prop_1_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add ordered prop 1"); - initial_graph - .add_ordered_edge( - initial_vector_clock_id, - initial_graph - .get_node_index_by_id(container_prop_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - ordered_prop_1_index, - ) - .expect("Unable to add container prop -> ordered prop 1 edge"); - - let ordered_prop_2_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let ordered_prop_2_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - ordered_prop_2_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - ordered_prop_2_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add ordered prop 2"); - initial_graph - .add_ordered_edge( - initial_vector_clock_id, - initial_graph - .get_node_index_by_id(container_prop_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - ordered_prop_2_index, - ) - .expect("Unable to add container prop -> ordered prop 2 edge"); - - let ordered_prop_3_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let ordered_prop_3_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - ordered_prop_3_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - ordered_prop_3_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add ordered prop 3"); - initial_graph - .add_ordered_edge( - initial_vector_clock_id, - initial_graph - .get_node_index_by_id(container_prop_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - ordered_prop_3_index, - ) - .expect("Unable to add container prop -> ordered prop 3 edge"); - - let ordered_prop_4_id = initial_graph - .generate_ulid() - .expect("Unable to generate Ulid"); - let ordered_prop_4_index = initial_graph - .add_node( - NodeWeight::new_content( - initial_vector_clock_id, - ordered_prop_4_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - ordered_prop_4_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add ordered prop 4"); - initial_graph - .add_ordered_edge( - initial_vector_clock_id, - initial_graph - .get_node_index_by_id(container_prop_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(initial_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - ordered_prop_4_index, - ) - .expect("Unable to add container prop -> ordered prop 4 edge"); - - initial_graph.cleanup(); - initial_graph.dot(); - - initial_graph - .mark_graph_seen(initial_vector_clock_id) - .expect("unable to mark graph seen"); - - let new_vector_clock_id = VectorClockId::new(Ulid::new(), actor_id); - let mut new_graph = initial_graph.clone(); - - let ordered_prop_5_id = new_graph.generate_ulid().expect("Unable to generate Ulid"); - let ordered_prop_5_index = new_graph - .add_node( - NodeWeight::new_content( - new_vector_clock_id, - ordered_prop_5_id, - Ulid::new(), - ContentAddress::Prop(ContentHash::new( - ordered_prop_5_id.to_string().as_bytes(), - )), - ) - .expect("Unable to create NodeWeight"), - ) - .expect("Unable to add ordered prop 5"); - new_graph - .add_ordered_edge( - new_vector_clock_id, - new_graph - .get_node_index_by_id(container_prop_id) - .expect("Unable to get NodeIndex"), - EdgeWeight::new(new_vector_clock_id, EdgeWeightKind::new_use()) - .expect("Unable to create EdgeWeight"), - ordered_prop_5_index, - ) - .expect("Unable to add container prop -> ordered prop 5 edge"); - - new_graph.cleanup(); - new_graph.dot(); - new_graph - .mark_graph_seen(new_vector_clock_id) - .expect("unable to mark graph seen"); - - let ConflictsAndUpdates { conflicts, updates } = new_graph - .detect_conflicts_and_updates( - new_vector_clock_id, - &initial_graph, - initial_vector_clock_id, - ) - .expect("Unable to detect conflicts and updates"); - - assert_eq!(Vec::::new(), conflicts); - assert_eq!(Vec::::new(), updates); - } - - #[test] - fn simple_ordering_no_conflicts_same_vector_clocks() { - let vector_clock_id = VectorClockId::new(Ulid::new(), Ulid::new()); - let mut to_rebase_graph = WorkspaceSnapshotGraphV1::new(vector_clock_id) - .expect("Unable to create WorkspaceSnapshotGraph"); - - let ordered_node = "ordered_container"; - let initial_children = vec!["a", "b", "c"]; - let onto_children = vec!["d", "e", "f"]; - - let mut node_id_map = - add_prop_nodes_to_graph(&mut to_rebase_graph, vector_clock_id, &[ordered_node], true); - node_id_map.extend(add_prop_nodes_to_graph( - &mut to_rebase_graph, - vector_clock_id, - &initial_children, - false, - )); - let ordered_id = *node_id_map.get(ordered_node).expect("should be there"); - let ordered_idx = to_rebase_graph - .get_node_index_by_id(ordered_id) - .expect("should have a node index"); - let root_idx = to_rebase_graph.root(); - to_rebase_graph - .add_edge( - root_idx, - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("failed to make edge weight"), - ordered_idx, - ) - .expect("should be able to make an edge"); - - for child in &initial_children { - let ordered_idx = to_rebase_graph - .get_node_index_by_id(ordered_id) - .expect("should have a node index"); - let child_id = node_id_map.get(*child).copied().expect("node should exist"); - let child_idx = to_rebase_graph - .get_node_index_by_id(child_id) - .expect("should have a node index"); - to_rebase_graph - .add_ordered_edge( - vector_clock_id, - ordered_idx, - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("failed to make edge weight"), - child_idx, - ) - .expect("should be able to make an edge"); - } - - to_rebase_graph.cleanup(); - to_rebase_graph - .mark_graph_seen(vector_clock_id) - .expect("mark twain"); - - let mut onto_graph = to_rebase_graph.clone(); - for child in &initial_children { - let ordered_idx = onto_graph - .get_node_index_by_id(ordered_id) - .expect("should have a node index"); - let child_id = node_id_map.get(*child).copied().expect("node should exist"); - let child_idx = onto_graph - .get_node_index_by_id(child_id) - .expect("should have a node index"); - onto_graph - .remove_edge( - vector_clock_id, - ordered_idx, - child_idx, - EdgeWeightKindDiscriminants::Use, - ) - .expect("unable to remove edge"); - } - - node_id_map.extend(add_prop_nodes_to_graph( - &mut onto_graph, - vector_clock_id, - &onto_children, - false, - )); - - for child in &onto_children { - let child_id = node_id_map.get(*child).copied().expect("node should exist"); - let ordered_idx = onto_graph - .get_node_index_by_id(ordered_id) - .expect("should have a node index"); - let child_idx = onto_graph - .get_node_index_by_id(child_id) - .expect("should have a node index"); - onto_graph - .add_ordered_edge( - vector_clock_id, - ordered_idx, - EdgeWeight::new(vector_clock_id, EdgeWeightKind::new_use()) - .expect("failed to make edge weight"), - child_idx, - ) - .expect("should be able to make an edge"); - } - - onto_graph.cleanup(); - onto_graph - .mark_graph_seen(vector_clock_id) - .expect("call me mark, mr seen is my father"); - - let conflicts_and_updates = to_rebase_graph - .detect_conflicts_and_updates(vector_clock_id, &onto_graph, vector_clock_id) - .expect("unable to detect conflicts and updates"); - - assert!(conflicts_and_updates.conflicts.is_empty()); - - to_rebase_graph - .perform_updates(vector_clock_id, &conflicts_and_updates.updates) - .expect("unable to perform updates"); - to_rebase_graph.cleanup(); - - let ordered_idx = to_rebase_graph - .get_node_index_by_id(ordered_id) - .expect("should have a node index"); - let ordering_node = to_rebase_graph - .ordering_node_for_container(ordered_idx) - .expect("should not fail") - .expect("ordering node should exist"); - - let expected_order_ids: Vec = onto_children - .iter() - .map(|&name| node_id_map.get(name).copied().expect("get id for name")) - .collect(); - assert_eq!(&expected_order_ids, ordering_node.order()); - - let container_children: Vec = to_rebase_graph - .edges_directed_for_edge_weight_kind( - ordered_idx, - Outgoing, - EdgeWeightKindDiscriminants::Use, - ) - .iter() - .filter_map(|(_, _, target_idx)| to_rebase_graph.node_index_to_id(*target_idx)) - .collect(); - - assert_eq!(expected_order_ids.len(), container_children.len()); - for container_child in &container_children { - assert!(expected_order_ids.contains(container_child)); - } - - let ordering_node_idx = to_rebase_graph - .get_node_index_by_id(ordering_node.id()) - .expect("should have an index for ordering node"); - - let ordering_node_children: Vec = to_rebase_graph - .edges_directed(ordering_node_idx, Outgoing) - .filter_map(|edge_ref| to_rebase_graph.node_index_to_id(edge_ref.target())) - .collect(); - - for child in &ordering_node_children { - assert!(expected_order_ids.contains(child)); - } - } - #[test] fn detect_conflicts_and_updates_single_removal_update() { let nodes = ["a", "b", "c"]; @@ -1638,12 +497,7 @@ mod test { .get_node_index_by_id(c_id) .expect("could not get node index by id"); new_graph - .remove_edge( - new_vector_clock_id, - a_idx, - c_idx, - EdgeWeightKindDiscriminants::Use, - ) + .remove_edge(a_idx, c_idx, EdgeWeightKindDiscriminants::Use) .expect("could not remove edge"); // Remove the second edge involving "c". @@ -1654,12 +508,7 @@ mod test { .get_node_index_by_id(c_id) .expect("could not get node index by id"); new_graph - .remove_edge( - new_vector_clock_id, - c_idx, - b_idx, - EdgeWeightKindDiscriminants::Use, - ) + .remove_edge(c_idx, b_idx, EdgeWeightKindDiscriminants::Use) .expect("could not remove edge"); // Perform the removal @@ -1675,9 +524,7 @@ mod test { // base_graph.tiny_dot_to_file(Some("to_rebase")); // new_graph.tiny_dot_to_file(Some("onto")); - let ConflictsAndUpdates { conflicts, updates } = base_graph - .detect_conflicts_and_updates(base_vector_clock_id, &new_graph, new_vector_clock_id) - .expect("Unable to detect conflicts and updates"); + let updates = base_graph.detect_updates(&new_graph); assert_eq!( vec![Update::RemoveEdge { @@ -1693,7 +540,6 @@ mod test { }], updates ); - assert!(conflicts.is_empty()); } #[test] @@ -1739,7 +585,6 @@ mod test { onto_graph .remove_edge( - onto_vector_clock_id, onto_graph.root(), to_rebase_graph .get_node_index_by_id(prototype_node_id) @@ -1753,15 +598,8 @@ mod test { .mark_graph_seen(onto_vector_clock_id) .expect("mark_graph_seen"); - let ConflictsAndUpdates { conflicts, updates } = to_rebase_graph - .detect_conflicts_and_updates( - to_rebase_vector_clock_id, - &onto_graph, - onto_vector_clock_id, - ) - .expect("detect_conflicts_and_updates"); + let updates = to_rebase_graph.detect_updates(&onto_graph); - assert!(conflicts.is_empty()); assert_eq!(1, updates.len()); assert!(matches!( updates[0], diff --git a/lib/dal/src/workspace_snapshot/graph/tests/rebase.rs b/lib/dal/src/workspace_snapshot/graph/tests/rebase.rs index 5fc2ffbb8b..87435d1a0b 100644 --- a/lib/dal/src/workspace_snapshot/graph/tests/rebase.rs +++ b/lib/dal/src/workspace_snapshot/graph/tests/rebase.rs @@ -8,7 +8,6 @@ mod test { use crate::func::FuncKind; use crate::workspace_snapshot::content_address::ContentAddress; use crate::workspace_snapshot::edge_weight::{EdgeWeight, EdgeWeightKind}; - use crate::workspace_snapshot::graph::ConflictsAndUpdates; use crate::workspace_snapshot::node_weight::category_node_weight::CategoryNodeKind; use crate::workspace_snapshot::node_weight::NodeWeight; use crate::workspace_snapshot::node_weight::{ContentNodeWeight, FuncNodeWeight}; @@ -132,12 +131,7 @@ mod test { .expect("could not add edge"); // Before cleanup, detect conflicts and updates. - let ConflictsAndUpdates { - conflicts: before_cleanup_conflicts, - updates: before_cleanup_updates, - } = to_rebase - .detect_conflicts_and_updates(to_rebase_vector_clock_id, &onto, onto_vector_clock_id) - .expect("could not detect conflicts and updates"); + let before_cleanup_updates = to_rebase.detect_updates(&onto); // Cleanup and check node count. onto.cleanup(); @@ -148,18 +142,11 @@ mod test { ); // Detect conflicts and updates. Ensure cleanup did not affect the results. - let ConflictsAndUpdates { conflicts, updates } = to_rebase - .detect_conflicts_and_updates(to_rebase_vector_clock_id, &onto, onto_vector_clock_id) - .expect("could not detect conflicts and updates"); - assert!(conflicts.is_empty()); + let updates = to_rebase.detect_updates(&onto); assert_eq!( 7, // expected updates.len() // actual ); - assert_eq!( - before_cleanup_conflicts, // expected - conflicts // actual - ); assert_eq!( before_cleanup_updates, // expected updates // actual @@ -176,7 +163,7 @@ mod test { // Perform the updates. In the future, we may want to see if the onto and resulting to // rebase graphs are logically equivalent after updates are performed. to_rebase - .perform_updates(to_rebase_vector_clock_id, &updates) + .perform_updates(&updates) .expect("could not perform updates"); } } diff --git a/lib/dal/src/workspace_snapshot/node_weight/ordering_node_weight.rs b/lib/dal/src/workspace_snapshot/node_weight/ordering_node_weight.rs index 75244eb290..7291d821f5 100644 --- a/lib/dal/src/workspace_snapshot/node_weight/ordering_node_weight.rs +++ b/lib/dal/src/workspace_snapshot/node_weight/ordering_node_weight.rs @@ -99,26 +99,12 @@ impl OrderingNodeWeight { } /// Returns `true` if the id passed was actually removed, `false` if not (because not in the order) - pub fn remove_from_order( - &mut self, - vector_clock_id: VectorClockId, - id: Ulid, - inc_clocks: bool, - ) -> bool { - let mut order = self.order.to_owned(); - order.retain(|&item_id| item_id != id); - if order.len() != self.order().len() { - if inc_clocks { - self.set_order(vector_clock_id, order); - } else { - self.set_order_without_inc_clocks(order); - } - - true - } else { - false - } + pub fn remove_from_order(&mut self, id: Ulid) -> bool { + let order_len = self.order.len(); + self.order.retain(|&item_id| item_id != id); + order_len != self.order().len() } + pub fn get_index_for_id(&self, id: Ulid) -> NodeWeightResult { let index = &self .order diff --git a/lib/dal/src/workspace_snapshot/update.rs b/lib/dal/src/workspace_snapshot/update.rs index 4009bd0f44..8b13789179 100644 --- a/lib/dal/src/workspace_snapshot/update.rs +++ b/lib/dal/src/workspace_snapshot/update.rs @@ -1,62 +1 @@ -use serde::{Deserialize, Serialize}; -use strum::EnumDiscriminants; -use super::{ - edge_info::EdgeInfo, - edge_weight::{EdgeWeight, EdgeWeightKindDiscriminants}, - graph::WorkspaceSnapshotGraphResult, - node_weight::NodeWeight, -}; -use crate::{workspace_snapshot::NodeInformation, WorkspaceSnapshotGraphV1}; - -#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, EnumDiscriminants)] -pub enum Update { - NewEdge { - source: NodeInformation, - // Check if already exists in "onto" (source). Grab node weight from "to_rebase" - // (destination) and see if there is an equivalent node (id and lineage) in "onto". - // If not, use "import_subgraph". - destination: NodeInformation, - edge_weight: EdgeWeight, - }, - RemoveEdge { - source: NodeInformation, - destination: NodeInformation, - edge_kind: EdgeWeightKindDiscriminants, - }, - ReplaceNode { - node_weight: NodeWeight, - }, - NewNode { - node_weight: NodeWeight, - }, -} - -impl Update { - /// Produce a NewEdge update from an edge that exists only in the "onto" graph - pub fn new_edge( - onto_graph: &WorkspaceSnapshotGraphV1, - only_onto_edge_info: &EdgeInfo, - only_onto_edge_weight: EdgeWeight, - ) -> WorkspaceSnapshotGraphResult { - let source_node_weight = - onto_graph.get_node_weight(only_onto_edge_info.source_node_index)?; - let target_node_weight = - onto_graph.get_node_weight(only_onto_edge_info.target_node_index)?; - - let source = NodeInformation { - id: source_node_weight.id().into(), - node_weight_kind: source_node_weight.into(), - }; - let destination = NodeInformation { - id: target_node_weight.id().into(), - node_weight_kind: target_node_weight.into(), - }; - - Ok(Update::NewEdge { - source, - destination, - edge_weight: only_onto_edge_weight, - }) - } -} diff --git a/lib/rebaser-server/src/rebase.rs b/lib/rebaser-server/src/rebase.rs index ed45e1c3be..52942c233f 100644 --- a/lib/rebaser-server/src/rebase.rs +++ b/lib/rebaser-server/src/rebase.rs @@ -1,8 +1,6 @@ use dal::change_set::{ChangeSet, ChangeSetError, ChangeSetId}; -use dal::workspace_snapshot::graph::ConflictsAndUpdates; -use dal::workspace_snapshot::vector_clock::VectorClockId; use dal::workspace_snapshot::WorkspaceSnapshotError; -use dal::{DalContext, TransactionsError, WorkspacePk, WorkspaceSnapshot, WsEventError}; +use dal::{DalContext, TransactionsError, WorkspaceSnapshot, WsEventError}; use si_events::rebase_batch_address::RebaseBatchAddress; use si_events::WorkspaceSnapshotAddress; use si_layer_cache::activities::rebase::RebaseStatus; @@ -93,69 +91,25 @@ pub async fn perform_rebase( ); debug!("after snapshot fetch and parse: {:?}", start.elapsed()); - // Choose the most recent vector clock for the to_rebase change set for conflict detection - let to_rebase_vector_clock_id = to_rebase_workspace_snapshot - .max_recently_seen_clock_id(Some(to_rebase_change_set.id)) - .await? - .ok_or(RebaseError::MissingVectorClockForChangeSet( - to_rebase_change_set.id, - ))?; + to_rebase_workspace_snapshot + .perform_updates(rebase_batch.updates()) + .await?; - let conflicts_and_updates = ConflictsAndUpdates { - ..Default::default() - }; + debug!("updates complete: {:?}", start.elapsed()); - // If there are conflicts, immediately assemble a reply message that conflicts were found. - // Otherwise, we can perform updates and assemble a "success" reply message. - let message: RebaseStatus = if conflicts_and_updates.conflicts.is_empty() { - to_rebase_workspace_snapshot - .perform_updates(to_rebase_vector_clock_id, rebase_batch.updates()) + if !rebase_batch.updates().is_empty() { + // Once all updates have been performed, we can write out, mark everything as recently seen + // and update the pointer. + to_rebase_workspace_snapshot.write(ctx).await?; + debug!("snapshot written: {:?}", start.elapsed()); + to_rebase_change_set + .update_pointer(ctx, to_rebase_workspace_snapshot.id().await) .await?; - debug!("updates complete: {:?}", start.elapsed()); - - if !rebase_batch.updates().is_empty() { - // Once all updates have been performed, we can write out, mark everything as recently seen - // and update the pointer. - let workspace_pk = ctx.tenancy().workspace_pk().unwrap_or(WorkspacePk::NONE); - let vector_clock_id = VectorClockId::new( - to_rebase_change_set.id.into_inner(), - workspace_pk.into_inner(), - ); - - to_rebase_workspace_snapshot - .collapse_vector_clocks(ctx) - .await?; - - to_rebase_workspace_snapshot - .write(ctx, vector_clock_id) - .await?; - debug!("snapshot written: {:?}", start.elapsed()); - to_rebase_change_set - .update_pointer(ctx, to_rebase_workspace_snapshot.id().await) - .await?; - - debug!("pointer updated: {:?}", start.elapsed()); - } - let updates_count = rebase_batch.updates().len(); - //let updates_performed = serde_json::to_value(rebase_batch.updates())?.to_string(); - - //span.record("si.updates", updates_performed.clone()); - span.record("si.updates.count", updates_count.to_string()); - RebaseStatus::Success { - updates_performed: message.payload.rebase_batch_address, - } - } else { - let conflicts_count = conflicts_and_updates.conflicts.len(); - let conflicts_found = serde_json::to_value(conflicts_and_updates.conflicts)?.to_string(); - span.record("si.conflicts", conflicts_found.clone()); - span.record("si.conflicts.count", conflicts_count.to_string()); - RebaseStatus::ConflictsFound { - conflicts_found, - updates_found_and_skipped: serde_json::to_value(conflicts_and_updates.updates)? - .to_string(), - } - }; + debug!("pointer updated: {:?}", start.elapsed()); + } + let updates_count = rebase_batch.updates().len(); + span.record("si.updates.count", updates_count.to_string()); info!("rebase performed: {:?}", start.elapsed()); @@ -179,7 +133,9 @@ pub async fn perform_rebase( }); } - Ok(message) + Ok(RebaseStatus::Success { + updates_performed: message.payload.rebase_batch_address, + }) } pub(crate) async fn evict_unused_snapshots( diff --git a/lib/sdf-server/src/server.rs b/lib/sdf-server/src/server.rs index cd7f1b7d89..11133836f5 100644 --- a/lib/sdf-server/src/server.rs +++ b/lib/sdf-server/src/server.rs @@ -31,10 +31,7 @@ macro_rules! impl_default_error_into_response { ) => { impl axum::response::IntoResponse for $error_type { fn into_response(self) -> Response { - let (status, error_message) = match self { - $error_type::Transactions(TransactionsError::ConflictsOccurred(_)) => (axum::http::StatusCode::CONFLICT, self.to_string()), - _ => (axum::http::StatusCode::INTERNAL_SERVER_ERROR, self.to_string()), - }; + let (status, error_message) = (axum::http::StatusCode::INTERNAL_SERVER_ERROR, self.to_string()); let body = Json( serde_json::json!({ "error": { "message": error_message, "code": 42, "statusCode": status.as_u16() } }), diff --git a/lib/sdf-server/src/server/service/change_set.rs b/lib/sdf-server/src/server/service/change_set.rs index 271da8fc94..ec1ceb8880 100644 --- a/lib/sdf-server/src/server/service/change_set.rs +++ b/lib/sdf-server/src/server/service/change_set.rs @@ -74,9 +74,6 @@ impl IntoResponse for ChangeSetError { fn into_response(self) -> Response { let (status, error_message) = match self { ChangeSetError::ChangeSetNotFound => (StatusCode::NOT_FOUND, self.to_string()), - ChangeSetError::Transactions(TransactionsError::ConflictsOccurred(_)) => { - (StatusCode::CONFLICT, self.to_string()) - } ChangeSetError::DalChangeSetApply(_) => (StatusCode::CONFLICT, self.to_string()), _ => (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()), }; diff --git a/lib/sdf-server/src/server/service/change_set/rebase_on_base.rs b/lib/sdf-server/src/server/service/change_set/rebase_on_base.rs index 605c31e427..d741b3fc3f 100644 --- a/lib/sdf-server/src/server/service/change_set/rebase_on_base.rs +++ b/lib/sdf-server/src/server/service/change_set/rebase_on_base.rs @@ -48,7 +48,6 @@ pub async fn rebase_on_base( let base_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, base_change_set.id).await?; if let Some(rebase_batch) = WorkspaceSnapshot::calculate_rebase_batch( - ctx.change_set_id(), ctx.workspace_snapshot()?, Arc::new(base_snapshot), ) diff --git a/lib/sdf-server/src/server/service/change_set/status_with_base.rs b/lib/sdf-server/src/server/service/change_set/status_with_base.rs index 40bb918ef2..31621abc52 100644 --- a/lib/sdf-server/src/server/service/change_set/status_with_base.rs +++ b/lib/sdf-server/src/server/service/change_set/status_with_base.rs @@ -1,7 +1,7 @@ use axum::Json; use serde::{Deserialize, Serialize}; -use dal::{ChangeSet, Visibility, WorkspaceSnapshot, WorkspaceSnapshotError}; +use dal::Visibility; use super::ChangeSetResult; use crate::server::extract::{AccessBuilder, HandlerContext}; @@ -26,57 +26,48 @@ pub async fn status_with_base( AccessBuilder(request_ctx): AccessBuilder, Json(request): Json, ) -> ChangeSetResult> { - let ctx = builder.build(request_ctx.build(request.visibility)).await?; + let _ctx = builder.build(request_ctx.build(request.visibility)).await?; - let change_set = ChangeSet::find(&ctx, request.visibility.change_set_id) - .await? - .ok_or(dal::ChangeSetError::ChangeSetNotFound( - request.visibility.change_set_id, - ))?; - let cs_workspace_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, change_set.id).await?; - let cs_vector_clock_id = cs_workspace_snapshot - .max_recently_seen_clock_id(Some(change_set.id)) - .await? - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - change_set.id, - ))?; - let base_change_set = if let Some(base_change_set_id) = change_set.base_change_set_id { - ChangeSet::find(&ctx, base_change_set_id) - .await? - .ok_or(dal::ChangeSetError::ChangeSetNotFound(base_change_set_id))? - } else { - return Err(dal::ChangeSetError::NoBaseChangeSet(request.visibility.change_set_id).into()); - }; - let base_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, base_change_set.id).await?; - let base_vector_clock_id = base_snapshot - .max_recently_seen_clock_id(Some(base_change_set.id)) - .await? - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set.id, - ))?; - let conflicts_and_updates_change_set_into_base = base_snapshot - .detect_conflicts_and_updates( - base_vector_clock_id, - &cs_workspace_snapshot, - cs_vector_clock_id, - ) - .await?; - let conflicts_and_updates_base_into_change_set = cs_workspace_snapshot - .detect_conflicts_and_updates(cs_vector_clock_id, &base_snapshot, base_vector_clock_id) - .await?; + // let change_set = ChangeSet::find(&ctx, request.visibility.change_set_id) + // .await? + // .ok_or(dal::ChangeSetError::ChangeSetNotFound( + // request.visibility.change_set_id, + // ))?; + // let cs_workspace_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, change_set.id).await?; + // let cs_vector_clock_id = cs_workspace_snapshot + // .max_recently_seen_clock_id(Some(change_set.id)) + // .await? + // .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( + // change_set.id, + // ))?; + // let base_change_set = if let Some(base_change_set_id) = change_set.base_change_set_id { + // ChangeSet::find(&ctx, base_change_set_id) + // .await? + // .ok_or(dal::ChangeSetError::ChangeSetNotFound(base_change_set_id))? + // } else { + // return Err(dal::ChangeSetError::NoBaseChangeSet(request.visibility.change_set_id).into()); + // }; + // let base_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, base_change_set.id).await?; + // let base_vector_clock_id = base_snapshot + // .max_recently_seen_clock_id(Some(base_change_set.id)) + // .await? + // .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( + // base_change_set.id, + // ))?; + // let conflicts_and_updates_change_set_into_base = base_snapshot + // .detect_conflicts_and_updates( + // base_vector_clock_id, + // &cs_workspace_snapshot, + // cs_vector_clock_id, + // ) + // .await?; + // let conflicts_and_updates_base_into_change_set = cs_workspace_snapshot + // .detect_conflicts_and_updates(cs_vector_clock_id, &base_snapshot, base_vector_clock_id) + // .await?; Ok(Json(StatusWithBaseResponse { - base_has_updates: !conflicts_and_updates_base_into_change_set - .updates - .is_empty(), - change_set_has_updates: !conflicts_and_updates_change_set_into_base - .updates - .is_empty(), - conflicts_with_base: !conflicts_and_updates_base_into_change_set - .conflicts - .is_empty() - && !conflicts_and_updates_change_set_into_base - .conflicts - .is_empty(), + base_has_updates: false, + change_set_has_updates: false, + conflicts_with_base: false, })) } diff --git a/lib/sdf-server/src/server/service/component.rs b/lib/sdf-server/src/server/service/component.rs index b30011a0ff..82e7442c8f 100644 --- a/lib/sdf-server/src/server/service/component.rs +++ b/lib/sdf-server/src/server/service/component.rs @@ -115,9 +115,6 @@ impl IntoResponse for ComponentError { | ComponentError::PropNotFound(_) | ComponentError::SchemaVariantNotFound | ComponentError::NotFound(_) => (StatusCode::NOT_FOUND, self.to_string()), - ComponentError::Transactions(TransactionsError::ConflictsOccurred(_)) => { - (StatusCode::CONFLICT, self.to_string()) - } _ => (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()), }; diff --git a/lib/sdf-server/src/server/service/component/conflicts_for_component.rs b/lib/sdf-server/src/server/service/component/conflicts_for_component.rs index eff91a7f9e..3ff85279e0 100644 --- a/lib/sdf-server/src/server/service/component/conflicts_for_component.rs +++ b/lib/sdf-server/src/server/service/component/conflicts_for_component.rs @@ -1,12 +1,8 @@ use std::collections::HashMap; use axum::Json; -use dal::{ - workspace_snapshot::conflict::Conflict, AttributeValue, AttributeValueId, ChangeSet, - ComponentId, Visibility, WorkspaceSnapshot, WorkspaceSnapshotError, -}; +use dal::{AttributeValueId, ComponentId, Visibility}; use serde::{Deserialize, Serialize}; -use ulid::Ulid; use crate::server::extract::{AccessBuilder, HandlerContext}; use crate::service::component::ComponentResult; @@ -22,109 +18,15 @@ pub struct ConflictsForComponentRequest { pub type ConflictsForComponentResponse = HashMap; -// TODO get visibility and component id via path as the V2 endpoints do pub async fn conflicts_for_component( HandlerContext(builder): HandlerContext, AccessBuilder(request_ctx): AccessBuilder, Json(ConflictsForComponentRequest { - component_id, + component_id: _, visibility, }): Json, ) -> ComponentResult> { - let ctx = builder.build(request_ctx.build(visibility)).await?; + let _ctx = builder.build(request_ctx.build(visibility)).await?; - // Get change set snapshot - let change_set = ChangeSet::find(&ctx, visibility.change_set_id) - .await? - .ok_or(dal::ChangeSetError::ChangeSetNotFound( - visibility.change_set_id, - ))?; - let cs_workspace_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, change_set.id).await?; - let cs_vector_clock_id = cs_workspace_snapshot - .max_recently_seen_clock_id(Some(change_set.id)) - .await? - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - change_set.id, - ))?; - - // Get base snapshot - let base_change_set = if let Some(base_change_set_id) = change_set.base_change_set_id { - ChangeSet::find(&ctx, base_change_set_id) - .await? - .ok_or(dal::ChangeSetError::ChangeSetNotFound(base_change_set_id))? - } else { - return Err(dal::ChangeSetError::NoBaseChangeSet(visibility.change_set_id).into()); - }; - let base_snapshot = WorkspaceSnapshot::find_for_change_set(&ctx, base_change_set.id).await?; - let base_vector_clock_id = base_snapshot - .max_recently_seen_clock_id(Some(base_change_set.id)) - .await? - .ok_or(WorkspaceSnapshotError::RecentlySeenClocksMissing( - base_change_set.id, - ))?; - - let conflicts_and_updates_change_set_into_base = base_snapshot - .detect_conflicts_and_updates( - base_vector_clock_id, - &cs_workspace_snapshot, - cs_vector_clock_id, - ) - .await?; - - // TODO move this to the dal for ease of testing and write tests - let mut conflicts_for_av_id = ConflictsForComponentResponse::new(); - - for conflict in conflicts_and_updates_change_set_into_base.conflicts { - let node_index = cs_workspace_snapshot - .get_node_index_by_id(node_ulid_for_conflict(conflict)) - .await?; - - let node_weight = cs_workspace_snapshot.get_node_weight(node_index).await?; - - let Some(this_av_id) = cs_workspace_snapshot - .associated_attribute_value_id(node_weight) - .await? - else { - continue; - }; - - if AttributeValue::component_id(&ctx, this_av_id).await? != component_id { - continue; - } - - let frontend_conflict = match conflict { - Conflict::ChildOrder { .. } - | Conflict::ExclusiveEdgeMismatch { .. } - | Conflict::NodeContent { .. } => si_frontend_types::ConflictWithHead::Untreated { - raw: serde_json::json!(conflict).to_string(), - }, - - Conflict::ModifyRemovedItem { .. } => { - si_frontend_types::ConflictWithHead::RemovedWhatHeadModified { - container_av_id: this_av_id.into(), - } - } - - Conflict::RemoveModifiedItem { .. } => { - si_frontend_types::ConflictWithHead::ModifiedWhatHeadRemoved { - modified_av_id: this_av_id.into(), - } - } - }; - - conflicts_for_av_id.insert(this_av_id, frontend_conflict); - } - - Ok(Json(conflicts_for_av_id)) -} - -fn node_ulid_for_conflict(conflict: Conflict) -> Ulid { - match conflict { - Conflict::ChildOrder { onto, .. } => onto.id, - Conflict::ExclusiveEdgeMismatch { destination, .. } => destination.id, - Conflict::ModifyRemovedItem { container, .. } => container.id, - Conflict::NodeContent { onto, .. } => onto.id, - Conflict::RemoveModifiedItem { removed_item, .. } => removed_item.id, - } - .into() + Ok(Json(HashMap::new())) } diff --git a/lib/sdf-server/src/server/service/diagram.rs b/lib/sdf-server/src/server/service/diagram.rs index 0cbcdd1bd4..204f06f88d 100644 --- a/lib/sdf-server/src/server/service/diagram.rs +++ b/lib/sdf-server/src/server/service/diagram.rs @@ -109,9 +109,6 @@ impl IntoResponse for DiagramError { | DiagramError::FrameSocketNotFound(_) | DiagramError::EdgeNotFound | DiagramError::SocketNotFound => (StatusCode::NOT_FOUND, self.to_string()), - DiagramError::ContextTransaction(TransactionsError::ConflictsOccurred(_)) => { - (StatusCode::CONFLICT, self.to_string()) - } _ => (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()), }; diff --git a/lib/sdf-server/src/server/service/v2/func.rs b/lib/sdf-server/src/server/service/v2/func.rs index 4f9d67fa57..df94472e8f 100644 --- a/lib/sdf-server/src/server/service/v2/func.rs +++ b/lib/sdf-server/src/server/service/v2/func.rs @@ -100,10 +100,7 @@ impl IntoResponse for FuncAPIError { | Self::SchemaVariant(dal::SchemaVariantError::SchemaVariantLocked(_)) => { StatusCode::BAD_REQUEST } - // Return 409 when we see a conflict - Self::Transactions(dal::TransactionsError::ConflictsOccurred(_)) => { - StatusCode::CONFLICT - } + // Return 404 when the func is not found Self::FuncNotFound(_) => StatusCode::NOT_FOUND, // When a graph node cannot be found for a schema variant, it is not found diff --git a/lib/sdf-server/src/server/service/v2/module.rs b/lib/sdf-server/src/server/service/v2/module.rs index dd6d46697b..ebe7b9f963 100644 --- a/lib/sdf-server/src/server/service/v2/module.rs +++ b/lib/sdf-server/src/server/service/v2/module.rs @@ -40,9 +40,6 @@ impl IntoResponse for ModulesAPIError { Self::Transactions(dal::TransactionsError::BadWorkspaceAndChangeSet) => { StatusCode::FORBIDDEN } - Self::Transactions(dal::TransactionsError::ConflictsOccurred(_)) => { - StatusCode::CONFLICT - } Self::SchemaVariant(dal::SchemaVariantError::NotFound(schema_variant_id)) => { error!(%schema_variant_id, "schema variant not found"); StatusCode::NOT_FOUND diff --git a/lib/sdf-server/src/server/service/v2/variant.rs b/lib/sdf-server/src/server/service/v2/variant.rs index a801554c75..fd053b91a7 100644 --- a/lib/sdf-server/src/server/service/v2/variant.rs +++ b/lib/sdf-server/src/server/service/v2/variant.rs @@ -46,10 +46,6 @@ impl IntoResponse for SchemaVariantsAPIError { Self::Transactions(dal::TransactionsError::BadWorkspaceAndChangeSet) => { StatusCode::FORBIDDEN } - // Return 409 when we see a conflict - Self::Transactions(dal::TransactionsError::ConflictsOccurred(_)) => { - StatusCode::CONFLICT - } // When a graph node cannot be found for a schema variant, it is not found Self::SchemaVariant(dal::SchemaVariantError::NotFound(_)) => StatusCode::NOT_FOUND, _ => ApiError::DEFAULT_ERROR_STATUS_CODE, diff --git a/lib/sdf-server/src/server/service/variant.rs b/lib/sdf-server/src/server/service/variant.rs index 1643a6c4d6..7749293b30 100644 --- a/lib/sdf-server/src/server/service/variant.rs +++ b/lib/sdf-server/src/server/service/variant.rs @@ -82,10 +82,7 @@ impl IntoResponse for SchemaVariantError { | SchemaVariantError::VariantNotFound(_) => (StatusCode::NOT_FOUND, self.to_string()), SchemaVariantError::VariantAuthoring(VariantAuthoringError::DuplicatedSchemaName( _, - )) - | SchemaVariantError::Transactions(TransactionsError::ConflictsOccurred(_)) => { - (StatusCode::CONFLICT, self.to_string()) - } + )) => (StatusCode::CONFLICT, self.to_string()), SchemaVariantError::VariantAuthoring( VariantAuthoringError::AssetTypeNotReturnedForAssetFunc(_, _), ) => ( diff --git a/lib/si-layer-cache/src/activities/rebase.rs b/lib/si-layer-cache/src/activities/rebase.rs index 83a6f03f26..a6f5b2b057 100644 --- a/lib/si-layer-cache/src/activities/rebase.rs +++ b/lib/si-layer-cache/src/activities/rebase.rs @@ -78,14 +78,6 @@ pub enum RebaseStatus { /// The serialized updates performed when rebasing. updates_performed: RebaseBatchAddress, }, - /// Conflicts found when processing the request. - ConflictsFound { - /// A serialized list of the conflicts found during detection. - conflicts_found: String, - /// A serialized list of the updates found during detection and skipped because at least - /// once conflict was found. - updates_found_and_skipped: String, - }, /// Error encountered when processing the request. Error { /// The error message.