From 1469c02998a81649d87b000c898452be4fadf9e1 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 26 Sep 2023 13:29:23 -0700 Subject: [PATCH 01/20] Add observed_channel_notes table and implement note diffing --- .../20221109000000_test_schema.sql | 11 + .../20230925210437_add_observed_notes.sql | 9 + crates/collab/src/db/queries/buffers.rs | 235 ++++++++++++++++-- crates/collab/src/db/tables.rs | 1 + .../src/db/tables/observed_note_edits.rs | 42 ++++ crates/collab/src/db/tests/buffer_tests.rs | 108 ++++++++ 6 files changed, 384 insertions(+), 22 deletions(-) create mode 100644 crates/collab/migrations/20230925210437_add_observed_notes.sql create mode 100644 crates/collab/src/db/tables/observed_note_edits.rs diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index d8325755f80e0baa90903d39326754ae713c7180..982d2b290ecf56a56d1ccd796ce98c98f901d7ca 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -289,3 +289,14 @@ CREATE TABLE "user_features" ( CREATE UNIQUE INDEX "index_user_features_user_id_and_feature_id" ON "user_features" ("user_id", "feature_id"); CREATE INDEX "index_user_features_on_user_id" ON "user_features" ("user_id"); CREATE INDEX "index_user_features_on_feature_id" ON "user_features" ("feature_id"); + + +CREATE TABLE "observed_channel_note_edits" ( + "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "epoch" INTEGER NOT NULL, + "lamport_timestamp" INTEGER NOT NULL, + PRIMARY KEY (user_id, channel_id) +); + +CREATE UNIQUE INDEX "index_observed_notes_user_and_channel_id" ON "observed_channel_note_edits" ("user_id", "channel_id"); diff --git a/crates/collab/migrations/20230925210437_add_observed_notes.sql b/crates/collab/migrations/20230925210437_add_observed_notes.sql new file mode 100644 index 0000000000000000000000000000000000000000..ce3547b2810d958bcb296cf56f11110eea568adc --- /dev/null +++ b/crates/collab/migrations/20230925210437_add_observed_notes.sql @@ -0,0 +1,9 @@ +CREATE TABLE "observed_channel_note_edits" ( + "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "epoch" INTEGER NOT NULL, + "lamport_timestamp" INTEGER NOT NULL, + PRIMARY KEY (user_id, channel_id) +); + +CREATE UNIQUE INDEX "index_observed_notes_user_and_channel_id" ON "observed_channel_note_edits" ("user_id", "channel_id"); diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 4b149faf2abd5f22651b7ae8aad2ed8477fb7ab9..51848f1e61d071b1aedf5d0f4f39f0905fff5680 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -1,5 +1,6 @@ use super::*; use prost::Message; +use sea_query::Order::Desc; use text::{EditOperation, UndoOperation}; pub struct LeftChannelBuffer { @@ -74,7 +75,62 @@ impl Database { .await?; collaborators.push(collaborator); - let (base_text, operations) = self.get_buffer_state(&buffer, &tx).await?; + let (base_text, operations, max_operation) = + self.get_buffer_state(&buffer, &tx).await?; + + // Save the last observed operation + if let Some(max_operation) = max_operation { + observed_note_edits::Entity::insert(observed_note_edits::ActiveModel { + user_id: ActiveValue::Set(user_id), + channel_id: ActiveValue::Set(channel_id), + epoch: ActiveValue::Set(max_operation.0), + lamport_timestamp: ActiveValue::Set(max_operation.1), + }) + .on_conflict( + OnConflict::columns([ + observed_note_edits::Column::UserId, + observed_note_edits::Column::ChannelId, + ]) + .update_columns([ + observed_note_edits::Column::Epoch, + observed_note_edits::Column::LamportTimestamp, + ]) + .to_owned(), + ) + .exec(&*tx) + .await?; + } else { + let buffer_max = buffer_operation::Entity::find() + .filter(buffer_operation::Column::BufferId.eq(buffer.id)) + .filter(buffer_operation::Column::Epoch.eq(buffer.epoch.saturating_sub(1))) + .order_by(buffer_operation::Column::Epoch, Desc) + .order_by(buffer_operation::Column::LamportTimestamp, Desc) + .one(&*tx) + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); + + if let Some(buffer_max) = buffer_max { + observed_note_edits::Entity::insert(observed_note_edits::ActiveModel { + user_id: ActiveValue::Set(user_id), + channel_id: ActiveValue::Set(channel_id), + epoch: ActiveValue::Set(buffer_max.0), + lamport_timestamp: ActiveValue::Set(buffer_max.1), + }) + .on_conflict( + OnConflict::columns([ + observed_note_edits::Column::UserId, + observed_note_edits::Column::ChannelId, + ]) + .update_columns([ + observed_note_edits::Column::Epoch, + observed_note_edits::Column::LamportTimestamp, + ]) + .to_owned(), + ) + .exec(&*tx) + .await?; + } + } Ok(proto::JoinChannelBufferResponse { buffer_id: buffer.id.to_proto(), @@ -373,27 +429,35 @@ impl Database { channel_id: ChannelId, ) -> Result> { self.transaction(|tx| async move { - #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] - enum QueryUserIds { - UserId, - } - - let users: Vec = channel_buffer_collaborator::Entity::find() - .select_only() - .column(channel_buffer_collaborator::Column::UserId) - .filter( - Condition::all() - .add(channel_buffer_collaborator::Column::ChannelId.eq(channel_id)), - ) - .into_values::<_, QueryUserIds>() - .all(&*tx) - .await?; - - Ok(users) + self.get_channel_buffer_collaborators_internal(channel_id, &*tx) + .await }) .await } + async fn get_channel_buffer_collaborators_internal( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] + enum QueryUserIds { + UserId, + } + + let users: Vec = channel_buffer_collaborator::Entity::find() + .select_only() + .column(channel_buffer_collaborator::Column::UserId) + .filter( + Condition::all().add(channel_buffer_collaborator::Column::ChannelId.eq(channel_id)), + ) + .into_values::<_, QueryUserIds>() + .all(&*tx) + .await?; + + Ok(users) + } + pub async fn update_channel_buffer( &self, channel_id: ChannelId, @@ -418,7 +482,12 @@ impl Database { .iter() .filter_map(|op| operation_to_storage(op, &buffer, serialization_version)) .collect::>(); + if !operations.is_empty() { + // get current channel participants and save the max operation above + self.save_max_operation_for_collaborators(operations.as_slice(), channel_id, &*tx) + .await?; + buffer_operation::Entity::insert_many(operations) .on_conflict( OnConflict::columns([ @@ -455,6 +524,60 @@ impl Database { .await } + async fn save_max_operation_for_collaborators( + &self, + operations: &[buffer_operation::ActiveModel], + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result<()> { + let max_operation = operations + .iter() + .map(|storage_model| { + ( + storage_model.epoch.clone(), + storage_model.lamport_timestamp.clone(), + ) + }) + .max_by( + |(epoch_a, lamport_timestamp_a), (epoch_b, lamport_timestamp_b)| { + epoch_a.as_ref().cmp(epoch_b.as_ref()).then( + lamport_timestamp_a + .as_ref() + .cmp(lamport_timestamp_b.as_ref()), + ) + }, + ) + .unwrap(); + + let users = self + .get_channel_buffer_collaborators_internal(channel_id, tx) + .await?; + + observed_note_edits::Entity::insert_many(users.iter().map(|id| { + observed_note_edits::ActiveModel { + user_id: ActiveValue::Set(*id), + channel_id: ActiveValue::Set(channel_id), + epoch: max_operation.0.clone(), + lamport_timestamp: ActiveValue::Set(*max_operation.1.as_ref()), + } + })) + .on_conflict( + OnConflict::columns([ + observed_note_edits::Column::UserId, + observed_note_edits::Column::ChannelId, + ]) + .update_columns([ + observed_note_edits::Column::Epoch, + observed_note_edits::Column::LamportTimestamp, + ]) + .to_owned(), + ) + .exec(tx) + .await?; + + Ok(()) + } + async fn get_buffer_operation_serialization_version( &self, buffer_id: BufferId, @@ -491,7 +614,7 @@ impl Database { &self, buffer: &buffer::Model, tx: &DatabaseTransaction, - ) -> Result<(String, Vec)> { + ) -> Result<(String, Vec, Option<(i32, i32)>)> { let id = buffer.id; let (base_text, version) = if buffer.epoch > 0 { let snapshot = buffer_snapshot::Entity::find() @@ -519,13 +642,21 @@ impl Database { .stream(&*tx) .await?; let mut operations = Vec::new(); + + let mut max_epoch: Option = None; + let mut max_timestamp: Option = None; while let Some(row) = rows.next().await { + let row = row?; + + max_assign(&mut max_epoch, row.epoch); + max_assign(&mut max_timestamp, row.lamport_timestamp); + operations.push(proto::Operation { - variant: Some(operation_from_storage(row?, version)?), + variant: Some(operation_from_storage(row, version)?), }) } - Ok((base_text, operations)) + Ok((base_text, operations, max_epoch.zip(max_timestamp))) } async fn snapshot_channel_buffer( @@ -534,7 +665,7 @@ impl Database { tx: &DatabaseTransaction, ) -> Result<()> { let buffer = self.get_channel_buffer(channel_id, tx).await?; - let (base_text, operations) = self.get_buffer_state(&buffer, tx).await?; + let (base_text, operations, _) = self.get_buffer_state(&buffer, tx).await?; if operations.is_empty() { return Ok(()); } @@ -567,6 +698,66 @@ impl Database { Ok(()) } + + pub async fn has_buffer_changed(&self, user_id: UserId, channel_id: ChannelId) -> Result { + self.transaction(|tx| async move { + let user_max = observed_note_edits::Entity::find() + .filter(observed_note_edits::Column::UserId.eq(user_id)) + .filter(observed_note_edits::Column::ChannelId.eq(channel_id)) + .one(&*tx) + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); + + let channel_buffer = channel::Model { + id: channel_id, + ..Default::default() + } + .find_related(buffer::Entity) + .one(&*tx) + .await?; + + let Some(channel_buffer) = channel_buffer else { + return Ok(false); + }; + + let mut channel_max = buffer_operation::Entity::find() + .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) + .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch)) + .order_by(buffer_operation::Column::Epoch, Desc) + .order_by(buffer_operation::Column::LamportTimestamp, Desc) + .one(&*tx) + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); + + // If there are no edits in this epoch + if channel_max.is_none() { + // check if this user observed the last edit of the previous epoch + channel_max = buffer_operation::Entity::find() + .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) + .filter( + buffer_operation::Column::Epoch.eq(channel_buffer.epoch.saturating_sub(1)), + ) + .order_by(buffer_operation::Column::Epoch, Desc) + .order_by(buffer_operation::Column::LamportTimestamp, Desc) + .one(&*tx) + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); + } + + Ok(user_max != channel_max) + }) + .await + } +} + +fn max_assign(max: &mut Option, val: T) { + if let Some(max_val) = max { + if val > *max_val { + *max = Some(val); + } + } else { + *max = Some(val); + } } fn operation_to_storage( diff --git a/crates/collab/src/db/tables.rs b/crates/collab/src/db/tables.rs index 81200df3e7b51a72a5ca33a100ff08ecbc80b75c..e8deb1501e155e33a1083b90cf9435de2a67fec6 100644 --- a/crates/collab/src/db/tables.rs +++ b/crates/collab/src/db/tables.rs @@ -12,6 +12,7 @@ pub mod contact; pub mod feature_flag; pub mod follower; pub mod language_server; +pub mod observed_note_edits; pub mod project; pub mod project_collaborator; pub mod room; diff --git a/crates/collab/src/db/tables/observed_note_edits.rs b/crates/collab/src/db/tables/observed_note_edits.rs new file mode 100644 index 0000000000000000000000000000000000000000..01e0e212ef4886ca194fc6257ef1d9630ed9e2a3 --- /dev/null +++ b/crates/collab/src/db/tables/observed_note_edits.rs @@ -0,0 +1,42 @@ +use crate::db::{ChannelId, UserId}; +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "observed_channel_note_edits")] +pub struct Model { + #[sea_orm(primary_key)] + pub user_id: UserId, + pub channel_id: ChannelId, + pub epoch: i32, + pub lamport_timestamp: i32, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::channel::Entity", + from = "Column::ChannelId", + to = "super::channel::Column::Id" + )] + Channel, + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::UserId", + to = "super::user::Column::Id" + )] + User, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Channel.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::User.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 9808a9955be090b012bab28a877e2bfa6b61b001..76edc131360c38f3f28c5759b31d604555fb20c0 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -163,3 +163,111 @@ async fn test_channel_buffers(db: &Arc) { assert_eq!(buffer_response_b.base_text, "hello, cruel world"); assert_eq!(buffer_response_b.operations, &[]); } + +test_both_dbs!( + test_channel_buffers_diffs, + test_channel_buffers_diffs_postgres, + test_channel_buffers_diffs_sqlite +); + +async fn test_channel_buffers_diffs(db: &Database) { + let a_id = db + .create_user( + "user_a@example.com", + false, + NewUserParams { + github_login: "user_a".into(), + github_user_id: 101, + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id; + let b_id = db + .create_user( + "user_b@example.com", + false, + NewUserParams { + github_login: "user_b".into(), + github_user_id: 102, + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id; + + let owner_id = db.create_server("production").await.unwrap().0 as u32; + + let zed_id = db.create_root_channel("zed", "1", a_id).await.unwrap(); + + db.invite_channel_member(zed_id, b_id, a_id, false) + .await + .unwrap(); + + db.respond_to_channel_invite(zed_id, b_id, true) + .await + .unwrap(); + + let connection_id_a = ConnectionId { + owner_id, + id: a_id.0 as u32, + }; + let connection_id_b = ConnectionId { + owner_id, + id: b_id.0 as u32, + }; + + // Zero test: A should not register as changed on an unitialized channel buffer + assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + + let _ = db + .join_channel_buffer(zed_id, a_id, connection_id_a) + .await + .unwrap(); + + // Zero test: A should register as changed on an empty channel buffer + assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + + let mut buffer_a = Buffer::new(0, 0, "".to_string()); + let mut operations = Vec::new(); + operations.push(buffer_a.edit([(0..0, "hello world")])); + assert_eq!(buffer_a.text(), "hello world"); + + let operations = operations + .into_iter() + .map(|op| proto::serialize_operation(&language::Operation::Buffer(op))) + .collect::>(); + + db.update_channel_buffer(zed_id, a_id, &operations) + .await + .unwrap(); + + // Smoke test: Does B register as changed, A as unchanged? + assert!(db.has_buffer_changed(b_id, zed_id).await.unwrap()); + assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + + db.leave_channel_buffer(zed_id, connection_id_a) + .await + .unwrap(); + + // Snapshotting from leaving the channel buffer should not have a diff + assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + + let _ = db + .join_channel_buffer(zed_id, b_id, connection_id_b) + .await + .unwrap(); + + // B has opened the channel buffer, so we shouldn't have any diff + assert!(!db.has_buffer_changed(b_id, zed_id).await.unwrap()); + + db.leave_channel_buffer(zed_id, connection_id_b) + .await + .unwrap(); + + // Since B just opened and closed the buffer without editing, neither should have a diff + assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + assert!(!db.has_buffer_changed(b_id, zed_id).await.unwrap()); +} From 9ba975d6adf45a92bda3e51e32c23454f0329046 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sat, 30 Sep 2023 22:30:36 -0700 Subject: [PATCH 02/20] Channel notifications from the server works --- crates/channel/src/channel_store.rs | 16 +- .../src/channel_store/channel_index.rs | 7 + .../20221109000000_test_schema.sql | 8 +- .../20230925210437_add_observed_notes.sql | 8 +- crates/collab/src/db.rs | 1 + crates/collab/src/db/queries/buffers.rs | 155 +++++++++++------- crates/collab/src/db/queries/channels.rs | 15 +- crates/collab/src/db/tables.rs | 2 +- ...note_edits.rs => observed_buffer_edits.rs} | 18 +- crates/collab/src/db/tests/buffer_tests.rs | 17 +- crates/collab/src/rpc.rs | 27 ++- .../collab/src/tests/channel_buffer_tests.rs | 63 ++++++- crates/collab_ui/src/collab_panel.rs | 19 ++- crates/rpc/proto/zed.proto | 1 + crates/theme/src/theme.rs | 2 +- styles/src/style_tree/collab_panel.ts | 14 +- 16 files changed, 266 insertions(+), 107 deletions(-) rename crates/collab/src/db/tables/{observed_note_edits.rs => observed_buffer_edits.rs} (66%) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index a8f6dd67b6b6246e78617f029da19e50fe3ba75a..19fc4f35ee85dec1ed9a9b2d9ba114a14c2c197e 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -43,6 +43,7 @@ pub type ChannelData = (Channel, ChannelPath); pub struct Channel { pub id: ChannelId, pub name: String, + pub has_changed: bool, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] @@ -207,6 +208,13 @@ impl ChannelStore { ) } + pub fn has_channel_buffer_changed(&self, channel_id: ChannelId) -> Option { + self.channel_index + .by_id() + .get(&channel_id) + .map(|channel| channel.has_changed) + } + pub fn open_channel_chat( &mut self, channel_id: ChannelId, @@ -779,6 +787,7 @@ impl ChannelStore { Arc::new(Channel { id: channel.id, name: channel.name, + has_changed: false, }), ), } @@ -787,7 +796,8 @@ impl ChannelStore { let channels_changed = !payload.channels.is_empty() || !payload.delete_channels.is_empty() || !payload.insert_edge.is_empty() - || !payload.delete_edge.is_empty(); + || !payload.delete_edge.is_empty() + || !payload.notes_changed.is_empty(); if channels_changed { if !payload.delete_channels.is_empty() { @@ -814,6 +824,10 @@ impl ChannelStore { index.insert(channel) } + for id_changed in payload.notes_changed { + index.has_changed(id_changed); + } + for edge in payload.insert_edge { index.insert_edge(edge.channel_id, edge.parent_id); } diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index d0c49dc298be882acc22cc33bdeccf21872e4c91..88c6b856983d014fd66f727cd48b496a90d36c6f 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -76,6 +76,12 @@ impl<'a> ChannelPathsInsertGuard<'a> { } } + pub fn has_changed(&mut self, channel_id: ChannelId) { + if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { + Arc::make_mut(channel).has_changed = true; + } + } + pub fn insert(&mut self, channel_proto: proto::Channel) { if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) { Arc::make_mut(existing_channel).name = channel_proto.name; @@ -85,6 +91,7 @@ impl<'a> ChannelPathsInsertGuard<'a> { Arc::new(Channel { id: channel_proto.id, name: channel_proto.name, + has_changed: false, }), ); self.insert_root(channel_proto.id); diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 982d2b290ecf56a56d1ccd796ce98c98f901d7ca..7553392f396b5ca228041e602543902936a5e2c9 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -291,12 +291,12 @@ CREATE INDEX "index_user_features_on_user_id" ON "user_features" ("user_id"); CREATE INDEX "index_user_features_on_feature_id" ON "user_features" ("feature_id"); -CREATE TABLE "observed_channel_note_edits" ( +CREATE TABLE "observed_buffer_edits" ( "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, - "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, "epoch" INTEGER NOT NULL, "lamport_timestamp" INTEGER NOT NULL, - PRIMARY KEY (user_id, channel_id) + PRIMARY KEY (user_id, buffer_id) ); -CREATE UNIQUE INDEX "index_observed_notes_user_and_channel_id" ON "observed_channel_note_edits" ("user_id", "channel_id"); +CREATE UNIQUE INDEX "index_observed_buffers_user_and_buffer_id" ON "observed_buffer_edits" ("user_id", "buffer_id"); diff --git a/crates/collab/migrations/20230925210437_add_observed_notes.sql b/crates/collab/migrations/20230925210437_add_observed_notes.sql index ce3547b2810d958bcb296cf56f11110eea568adc..4574fe215ba009dc76dc8ef62937b733e436c94b 100644 --- a/crates/collab/migrations/20230925210437_add_observed_notes.sql +++ b/crates/collab/migrations/20230925210437_add_observed_notes.sql @@ -1,9 +1,9 @@ -CREATE TABLE "observed_channel_note_edits" ( +CREATE TABLE "observed_buffer_edits" ( "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, - "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, "epoch" INTEGER NOT NULL, "lamport_timestamp" INTEGER NOT NULL, - PRIMARY KEY (user_id, channel_id) + PRIMARY KEY (user_id, buffer_id) ); -CREATE UNIQUE INDEX "index_observed_notes_user_and_channel_id" ON "observed_channel_note_edits" ("user_id", "channel_id"); +CREATE UNIQUE INDEX "index_observed_buffer_user_and_buffer_id" ON "observed_buffer_edits" ("user_id", "buffer_id"); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index ab2fbe39451b9a7230719fa8ce6a127e34e4908d..f0896f873219b958ee33a723b933920d653c0132 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -436,6 +436,7 @@ pub struct Channel { pub struct ChannelsForUser { pub channels: ChannelGraph, pub channel_participants: HashMap>, + pub channels_with_changed_notes: HashSet, pub channels_with_admin_privileges: HashSet, } diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 51848f1e61d071b1aedf5d0f4f39f0905fff5680..f2993c516d0c53a1c548c9156e6d6c0b72daf909 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -80,20 +80,20 @@ impl Database { // Save the last observed operation if let Some(max_operation) = max_operation { - observed_note_edits::Entity::insert(observed_note_edits::ActiveModel { + observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { user_id: ActiveValue::Set(user_id), - channel_id: ActiveValue::Set(channel_id), + buffer_id: ActiveValue::Set(buffer.id), epoch: ActiveValue::Set(max_operation.0), lamport_timestamp: ActiveValue::Set(max_operation.1), }) .on_conflict( OnConflict::columns([ - observed_note_edits::Column::UserId, - observed_note_edits::Column::ChannelId, + observed_buffer_edits::Column::UserId, + observed_buffer_edits::Column::BufferId, ]) .update_columns([ - observed_note_edits::Column::Epoch, - observed_note_edits::Column::LamportTimestamp, + observed_buffer_edits::Column::Epoch, + observed_buffer_edits::Column::LamportTimestamp, ]) .to_owned(), ) @@ -110,20 +110,20 @@ impl Database { .map(|model| (model.epoch, model.lamport_timestamp)); if let Some(buffer_max) = buffer_max { - observed_note_edits::Entity::insert(observed_note_edits::ActiveModel { + observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { user_id: ActiveValue::Set(user_id), - channel_id: ActiveValue::Set(channel_id), + buffer_id: ActiveValue::Set(buffer.id), epoch: ActiveValue::Set(buffer_max.0), lamport_timestamp: ActiveValue::Set(buffer_max.1), }) .on_conflict( OnConflict::columns([ - observed_note_edits::Column::UserId, - observed_note_edits::Column::ChannelId, + observed_buffer_edits::Column::UserId, + observed_buffer_edits::Column::BufferId, ]) .update_columns([ - observed_note_edits::Column::Epoch, - observed_note_edits::Column::LamportTimestamp, + observed_buffer_edits::Column::Epoch, + observed_buffer_edits::Column::LamportTimestamp, ]) .to_owned(), ) @@ -463,7 +463,7 @@ impl Database { channel_id: ChannelId, user: UserId, operations: &[proto::Operation], - ) -> Result> { + ) -> Result<(Vec, Vec)> { self.transaction(move |tx| async move { self.check_user_is_channel_member(channel_id, user, &*tx) .await?; @@ -483,10 +483,23 @@ impl Database { .filter_map(|op| operation_to_storage(op, &buffer, serialization_version)) .collect::>(); + let mut channel_members; + if !operations.is_empty() { // get current channel participants and save the max operation above - self.save_max_operation_for_collaborators(operations.as_slice(), channel_id, &*tx) + self.save_max_operation_for_collaborators( + operations.as_slice(), + channel_id, + buffer.id, + &*tx, + ) + .await?; + + channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; + let collaborators = self + .get_channel_buffer_collaborators_internal(channel_id, &*tx) .await?; + channel_members.retain(|member| !collaborators.contains(member)); buffer_operation::Entity::insert_many(operations) .on_conflict( @@ -501,6 +514,8 @@ impl Database { ) .exec(&*tx) .await?; + } else { + channel_members = Vec::new(); } let mut connections = Vec::new(); @@ -519,7 +534,7 @@ impl Database { }); } - Ok(connections) + Ok((connections, channel_members)) }) .await } @@ -528,6 +543,7 @@ impl Database { &self, operations: &[buffer_operation::ActiveModel], channel_id: ChannelId, + buffer_id: BufferId, tx: &DatabaseTransaction, ) -> Result<()> { let max_operation = operations @@ -553,22 +569,22 @@ impl Database { .get_channel_buffer_collaborators_internal(channel_id, tx) .await?; - observed_note_edits::Entity::insert_many(users.iter().map(|id| { - observed_note_edits::ActiveModel { + observed_buffer_edits::Entity::insert_many(users.iter().map(|id| { + observed_buffer_edits::ActiveModel { user_id: ActiveValue::Set(*id), - channel_id: ActiveValue::Set(channel_id), + buffer_id: ActiveValue::Set(buffer_id), epoch: max_operation.0.clone(), lamport_timestamp: ActiveValue::Set(*max_operation.1.as_ref()), } })) .on_conflict( OnConflict::columns([ - observed_note_edits::Column::UserId, - observed_note_edits::Column::ChannelId, + observed_buffer_edits::Column::UserId, + observed_buffer_edits::Column::BufferId, ]) .update_columns([ - observed_note_edits::Column::Epoch, - observed_note_edits::Column::LamportTimestamp, + observed_buffer_edits::Column::Epoch, + observed_buffer_edits::Column::LamportTimestamp, ]) .to_owned(), ) @@ -699,54 +715,75 @@ impl Database { Ok(()) } - pub async fn has_buffer_changed(&self, user_id: UserId, channel_id: ChannelId) -> Result { - self.transaction(|tx| async move { - let user_max = observed_note_edits::Entity::find() - .filter(observed_note_edits::Column::UserId.eq(user_id)) - .filter(observed_note_edits::Column::ChannelId.eq(channel_id)) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); + #[cfg(test)] + pub async fn test_has_note_changed( + &self, + user_id: UserId, + channel_id: ChannelId, + ) -> Result { + self.transaction(|tx| async move { self.has_note_changed(user_id, channel_id, &*tx).await }) + .await + } - let channel_buffer = channel::Model { - id: channel_id, - ..Default::default() - } - .find_related(buffer::Entity) + pub async fn has_note_changed( + &self, + user_id: UserId, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result { + let Some(buffer_id) = channel::Model { + id: channel_id, + ..Default::default() + } + .find_related(buffer::Entity) + .one(&*tx) + .await? + .map(|buffer| buffer.id) else { + return Ok(false); + }; + + let user_max = observed_buffer_edits::Entity::find() + .filter(observed_buffer_edits::Column::UserId.eq(user_id)) + .filter(observed_buffer_edits::Column::BufferId.eq(buffer_id)) .one(&*tx) - .await?; + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); - let Some(channel_buffer) = channel_buffer else { - return Ok(false); - }; + let channel_buffer = channel::Model { + id: channel_id, + ..Default::default() + } + .find_related(buffer::Entity) + .one(&*tx) + .await?; - let mut channel_max = buffer_operation::Entity::find() + let Some(channel_buffer) = channel_buffer else { + return Ok(false); + }; + + let mut channel_max = buffer_operation::Entity::find() + .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) + .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch)) + .order_by(buffer_operation::Column::Epoch, Desc) + .order_by(buffer_operation::Column::LamportTimestamp, Desc) + .one(&*tx) + .await? + .map(|model| (model.epoch, model.lamport_timestamp)); + + // If there are no edits in this epoch + if channel_max.is_none() { + // check if this user observed the last edit of the previous epoch + channel_max = buffer_operation::Entity::find() .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) - .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch)) + .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch.saturating_sub(1))) .order_by(buffer_operation::Column::Epoch, Desc) .order_by(buffer_operation::Column::LamportTimestamp, Desc) .one(&*tx) .await? .map(|model| (model.epoch, model.lamport_timestamp)); + } - // If there are no edits in this epoch - if channel_max.is_none() { - // check if this user observed the last edit of the previous epoch - channel_max = buffer_operation::Entity::find() - .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) - .filter( - buffer_operation::Column::Epoch.eq(channel_buffer.epoch.saturating_sub(1)), - ) - .order_by(buffer_operation::Column::Epoch, Desc) - .order_by(buffer_operation::Column::LamportTimestamp, Desc) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); - } - - Ok(user_max != channel_max) - }) - .await + Ok(user_max != channel_max) } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 16a0891d160ffad81abfda6d46d0e4300112961e..6274550c25dd7702af492c531f7dab72553a965c 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -391,7 +391,8 @@ impl Database { .all(&*tx) .await?; - self.get_user_channels(channel_memberships, &tx).await + self.get_user_channels(user_id, channel_memberships, &tx) + .await }) .await } @@ -414,13 +415,15 @@ impl Database { .all(&*tx) .await?; - self.get_user_channels(channel_membership, &tx).await + self.get_user_channels(user_id, channel_membership, &tx) + .await }) .await } pub async fn get_user_channels( &self, + user_id: UserId, channel_memberships: Vec, tx: &DatabaseTransaction, ) -> Result { @@ -460,10 +463,18 @@ impl Database { } } + let mut channels_with_changed_notes = HashSet::default(); + for channel in graph.channels.iter() { + if self.has_note_changed(user_id, channel.id, tx).await? { + channels_with_changed_notes.insert(channel.id); + } + } + Ok(ChannelsForUser { channels: graph, channel_participants, channels_with_admin_privileges, + channels_with_changed_notes, }) } diff --git a/crates/collab/src/db/tables.rs b/crates/collab/src/db/tables.rs index e8deb1501e155e33a1083b90cf9435de2a67fec6..40686065467051ab127587ec7069b28b0bd007e1 100644 --- a/crates/collab/src/db/tables.rs +++ b/crates/collab/src/db/tables.rs @@ -12,7 +12,7 @@ pub mod contact; pub mod feature_flag; pub mod follower; pub mod language_server; -pub mod observed_note_edits; +pub mod observed_buffer_edits; pub mod project; pub mod project_collaborator; pub mod room; diff --git a/crates/collab/src/db/tables/observed_note_edits.rs b/crates/collab/src/db/tables/observed_buffer_edits.rs similarity index 66% rename from crates/collab/src/db/tables/observed_note_edits.rs rename to crates/collab/src/db/tables/observed_buffer_edits.rs index 01e0e212ef4886ca194fc6257ef1d9630ed9e2a3..db027f78b2b868e668433d23d5ef657cdd4825ef 100644 --- a/crates/collab/src/db/tables/observed_note_edits.rs +++ b/crates/collab/src/db/tables/observed_buffer_edits.rs @@ -1,12 +1,12 @@ -use crate::db::{ChannelId, UserId}; +use crate::db::{BufferId, UserId}; use sea_orm::entity::prelude::*; #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] -#[sea_orm(table_name = "observed_channel_note_edits")] +#[sea_orm(table_name = "observed_buffer_edits")] pub struct Model { #[sea_orm(primary_key)] pub user_id: UserId, - pub channel_id: ChannelId, + pub buffer_id: BufferId, pub epoch: i32, pub lamport_timestamp: i32, } @@ -14,11 +14,11 @@ pub struct Model { #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] pub enum Relation { #[sea_orm( - belongs_to = "super::channel::Entity", - from = "Column::ChannelId", - to = "super::channel::Column::Id" + belongs_to = "super::buffer::Entity", + from = "Column::BufferId", + to = "super::buffer::Column::Id" )] - Channel, + Buffer, #[sea_orm( belongs_to = "super::user::Entity", from = "Column::UserId", @@ -27,9 +27,9 @@ pub enum Relation { User, } -impl Related for Entity { +impl Related for Entity { fn to() -> RelationDef { - Relation::Channel.def() + Relation::Buffer.def() } } diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 76edc131360c38f3f28c5759b31d604555fb20c0..115a20ffa6fc31a809625827d05e024082bd8e7d 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -220,7 +220,7 @@ async fn test_channel_buffers_diffs(db: &Database) { }; // Zero test: A should not register as changed on an unitialized channel buffer - assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); let _ = db .join_channel_buffer(zed_id, a_id, connection_id_a) @@ -228,7 +228,7 @@ async fn test_channel_buffers_diffs(db: &Database) { .unwrap(); // Zero test: A should register as changed on an empty channel buffer - assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); let mut buffer_a = Buffer::new(0, 0, "".to_string()); let mut operations = Vec::new(); @@ -245,15 +245,16 @@ async fn test_channel_buffers_diffs(db: &Database) { .unwrap(); // Smoke test: Does B register as changed, A as unchanged? - assert!(db.has_buffer_changed(b_id, zed_id).await.unwrap()); - assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + assert!(db.test_has_note_changed(b_id, zed_id).await.unwrap()); + + assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); db.leave_channel_buffer(zed_id, connection_id_a) .await .unwrap(); // Snapshotting from leaving the channel buffer should not have a diff - assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); let _ = db .join_channel_buffer(zed_id, b_id, connection_id_b) @@ -261,13 +262,13 @@ async fn test_channel_buffers_diffs(db: &Database) { .unwrap(); // B has opened the channel buffer, so we shouldn't have any diff - assert!(!db.has_buffer_changed(b_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(b_id, zed_id).await.unwrap()); db.leave_channel_buffer(zed_id, connection_id_b) .await .unwrap(); // Since B just opened and closed the buffer without editing, neither should have a diff - assert!(!db.has_buffer_changed(a_id, zed_id).await.unwrap()); - assert!(!db.has_buffer_changed(b_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); + assert!(!db.test_has_note_changed(b_id, zed_id).await.unwrap()); } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 9e14c4847397465758cd75a570a6ea53a1db44d7..b9dae999cd1eb06bddc29c5d2cafae2def54a2bf 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -2691,7 +2691,7 @@ async fn update_channel_buffer( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let collaborators = db + let (collaborators, non_collaborators) = db .update_channel_buffer(channel_id, session.user_id, &request.operations) .await?; @@ -2704,6 +2704,25 @@ async fn update_channel_buffer( }, &session.peer, ); + + let pool = &*session.connection_pool().await; + + broadcast( + None, + non_collaborators + .iter() + .flat_map(|user_id| pool.user_connection_ids(*user_id)), + |peer_id| { + session.peer.send( + peer_id.into(), + proto::UpdateChannels { + notes_changed: vec![channel_id.to_proto()], + ..Default::default() + }, + ) + }, + ); + Ok(()) } @@ -2986,6 +3005,12 @@ fn build_initial_channels_update( }); } + update.notes_changed = channels + .channels_with_changed_notes + .iter() + .map(|channel_id| channel_id.to_proto()) + .collect(); + update.insert_edge = channels.channels.edges; for (channel_id, participants) in channels.channel_participants { diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 05abda5af3c7e77530860dc4cc3bcde490cd7e9e..0f87aeb48f670a1981e6f5791e9aa0627c89d80c 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -410,10 +410,7 @@ async fn test_channel_buffer_disconnect( channel_buffer_a.update(cx_a, |buffer, _| { assert_eq!( buffer.channel().as_ref(), - &Channel { - id: channel_id, - name: "the-channel".to_string() - } + &channel(channel_id, "the-channel") ); assert!(!buffer.is_connected()); }); @@ -438,15 +435,20 @@ async fn test_channel_buffer_disconnect( channel_buffer_b.update(cx_b, |buffer, _| { assert_eq!( buffer.channel().as_ref(), - &Channel { - id: channel_id, - name: "the-channel".to_string() - } + &channel(channel_id, "the-channel") ); assert!(!buffer.is_connected()); }); } +fn channel(id: u64, name: &'static str) -> Channel { + Channel { + id, + name: name.to_string(), + has_changed: false, + } +} + #[gpui::test] async fn test_rejoin_channel_buffer( deterministic: Arc, @@ -627,6 +629,7 @@ async fn test_following_to_channel_notes_without_a_shared_project( let mut server = TestServer::start(&deterministic).await; let client_a = server.create_client(cx_a, "user_a").await; let client_b = server.create_client(cx_b, "user_b").await; + let client_c = server.create_client(cx_c, "user_c").await; cx_a.update(editor::init); @@ -757,6 +760,50 @@ async fn test_following_to_channel_notes_without_a_shared_project( }); } +#[gpui::test] +async fn test_channel_buffer_changes( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + let channel_id = server + .make_channel( + "the-channel", + None, + (&client_a, cx_a), + &mut [(&client_b, cx_b)], + ) + .await; + + let channel_buffer_a = client_a + .channel_store() + .update(cx_a, |store, cx| store.open_channel_buffer(channel_id, cx)) + .await + .unwrap(); + + channel_buffer_a.update(cx_a, |buffer, cx| { + buffer.buffer().update(cx, |buffer, cx| { + buffer.edit([(0..0, "1")], None, cx); + }) + }); + deterministic.run_until_parked(); + + let has_buffer_changed = cx_b.read(|cx| { + client_b + .channel_store() + .read(cx) + .has_channel_buffer_changed(channel_id) + .unwrap() + }); + + assert!(has_buffer_changed); +} + #[track_caller] fn assert_collaborators(collaborators: &HashMap, ids: &[Option]) { let mut user_ids = collaborators diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 16a9ec563b16199ec24c4b63e3933d19d330f19b..4f81d07ea72b6f376d22b7b01fc185c35ccc1583 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1816,12 +1816,19 @@ impl CollabPanel { .left(), ) .with_child( - Label::new(channel.name.clone(), theme.channel_name.text.clone()) - .contained() - .with_style(theme.channel_name.container) - .aligned() - .left() - .flex(1., true), + Label::new( + channel.name.clone(), + theme + .channel_name + .in_state(channel.has_changed) + .text + .clone(), + ) + .contained() + .with_style(theme.channel_name.container) + .aligned() + .left() + .flex(1., true), ) .with_child( MouseEventHandler::new::(ix, cx, move |_, cx| { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index a62a9f06c3f39939b69013dbc9f85d2945ff3ee4..c53db447d3d811e566fce9081ad0c21ad3fec6f0 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -955,6 +955,7 @@ message UpdateChannels { repeated uint64 remove_channel_invitations = 6; repeated ChannelParticipants channel_participants = 7; repeated ChannelPermission channel_permissions = 8; + repeated uint64 notes_changed = 9; } message ChannelEdge { diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 1ca2d839c09b08117a9024065b970f13f837eae8..6c32bfd1293d680514bd387d5aaa708ce4a8ad28 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -251,7 +251,7 @@ pub struct CollabPanel { pub leave_call: Interactive, pub contact_row: Toggleable>, pub channel_row: Toggleable>, - pub channel_name: ContainedText, + pub channel_name: Toggleable, pub row_height: f32, pub project_row: Toggleable>, pub tree_branch: Toggleable>, diff --git a/styles/src/style_tree/collab_panel.ts b/styles/src/style_tree/collab_panel.ts index 4d605d118c370569b7fed3247aa35e781d65df4a..33b36e36f4b0e52c6cfb42385398fc5d46bae456 100644 --- a/styles/src/style_tree/collab_panel.ts +++ b/styles/src/style_tree/collab_panel.ts @@ -267,10 +267,18 @@ export default function contacts_panel(): any { }), channel_row: item_row, channel_name: { - ...text(layer, "sans", { size: "sm" }), - margin: { - left: CHANNEL_SPACING, + active: { + ...text(layer, "sans", { size: "sm", weight: "bold" }), + margin: { + left: CHANNEL_SPACING, + }, }, + inactive: { + ...text(layer, "sans", { size: "sm" }), + margin: { + left: CHANNEL_SPACING, + }, + } }, list_empty_label_container: { margin: { From e0ff7ba18082373ab15bb46e44ce43167e08635c Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 1 Oct 2023 19:53:32 -0700 Subject: [PATCH 03/20] Add channel note indicator and clear changed status --- crates/channel/src/channel_store.rs | 20 ++++-- .../src/channel_store/channel_index.rs | 12 +++- .../collab/src/tests/channel_buffer_tests.rs | 63 ++++++++++++++++++- crates/collab_ui/src/collab_panel.rs | 33 +++++++++- crates/theme/src/theme.rs | 1 + styles/src/style_tree/collab_panel.ts | 1 + 6 files changed, 119 insertions(+), 11 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 19fc4f35ee85dec1ed9a9b2d9ba114a14c2c197e..417f486b9e2407622ac24f367c59bc344f60344e 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -43,7 +43,7 @@ pub type ChannelData = (Channel, ChannelPath); pub struct Channel { pub id: ChannelId, pub name: String, - pub has_changed: bool, + pub has_note_changed: bool, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] @@ -200,19 +200,27 @@ impl ChannelStore { ) -> Task>> { let client = self.client.clone(); let user_store = self.user_store.clone(); - self.open_channel_resource( + let open_channel_buffer = self.open_channel_resource( channel_id, |this| &mut this.opened_buffers, |channel, cx| ChannelBuffer::new(channel, client, user_store, cx), cx, - ) + ); + cx.spawn(|this, mut cx| async move { + let buffer = open_channel_buffer.await?; + this.update(&mut cx, |this, cx| { + this.channel_index.clear_note_changed(channel_id); + cx.notify(); + }); + Ok(buffer) + }) } pub fn has_channel_buffer_changed(&self, channel_id: ChannelId) -> Option { self.channel_index .by_id() .get(&channel_id) - .map(|channel| channel.has_changed) + .map(|channel| channel.has_note_changed) } pub fn open_channel_chat( @@ -787,7 +795,7 @@ impl ChannelStore { Arc::new(Channel { id: channel.id, name: channel.name, - has_changed: false, + has_note_changed: false, }), ), } @@ -825,7 +833,7 @@ impl ChannelStore { } for id_changed in payload.notes_changed { - index.has_changed(id_changed); + index.note_changed(id_changed); } for edge in payload.insert_edge { diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 88c6b856983d014fd66f727cd48b496a90d36c6f..42474f1d85da1cf3d581daaae25e58bd2c580d5a 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -38,6 +38,12 @@ impl ChannelIndex { channels_by_id: &mut self.channels_by_id, } } + + pub fn clear_note_changed(&mut self, channel_id: ChannelId) { + if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { + Arc::make_mut(channel).has_note_changed = false; + } + } } impl Deref for ChannelIndex { @@ -76,9 +82,9 @@ impl<'a> ChannelPathsInsertGuard<'a> { } } - pub fn has_changed(&mut self, channel_id: ChannelId) { + pub fn note_changed(&mut self, channel_id: ChannelId) { if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - Arc::make_mut(channel).has_changed = true; + Arc::make_mut(channel).has_note_changed = true; } } @@ -91,7 +97,7 @@ impl<'a> ChannelPathsInsertGuard<'a> { Arc::new(Channel { id: channel_proto.id, name: channel_proto.name, - has_changed: false, + has_note_changed: false, }), ); self.insert_root(channel_proto.id); diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 0f87aeb48f670a1981e6f5791e9aa0627c89d80c..7ca6f0db3f6b3c48ec1bdd249748225e040f7cec 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -445,7 +445,7 @@ fn channel(id: u64, name: &'static str) -> Channel { Channel { id, name: name.to_string(), - has_changed: false, + has_note_changed: false, } } @@ -786,6 +786,7 @@ async fn test_channel_buffer_changes( .await .unwrap(); + // Client A makes an edit, and client B should see that the note has changed. channel_buffer_a.update(cx_a, |buffer, cx| { buffer.buffer().update(cx, |buffer, cx| { buffer.edit([(0..0, "1")], None, cx); @@ -802,6 +803,66 @@ async fn test_channel_buffer_changes( }); assert!(has_buffer_changed); + + // Opening the buffer should clear the changed flag. + let channel_buffer_b = client_b + .channel_store() + .update(cx_b, |store, cx| store.open_channel_buffer(channel_id, cx)) + .await + .unwrap(); + deterministic.run_until_parked(); + + let has_buffer_changed = cx_b.read(|cx| { + client_b + .channel_store() + .read(cx) + .has_channel_buffer_changed(channel_id) + .unwrap() + }); + + assert!(!has_buffer_changed); + + // Editing the channel while the buffer is open shuold not show that the buffer has changed. + channel_buffer_a.update(cx_a, |buffer, cx| { + buffer.buffer().update(cx, |buffer, cx| { + buffer.edit([(0..0, "2")], None, cx); + }) + }); + deterministic.run_until_parked(); + + let has_buffer_changed = cx_b.read(|cx| { + client_b + .channel_store() + .read(cx) + .has_channel_buffer_changed(channel_id) + .unwrap() + }); + + assert!(!has_buffer_changed); + + // Closing the buffer should re-enable change tracking + cx_b.update(|_| { + drop(channel_buffer_b); + }); + + deterministic.run_until_parked(); + + channel_buffer_a.update(cx_a, |buffer, cx| { + buffer.buffer().update(cx, |buffer, cx| { + buffer.edit([(0..0, "3")], None, cx); + }) + }); + deterministic.run_until_parked(); + + let has_buffer_changed = cx_b.read(|cx| { + client_b + .channel_store() + .read(cx) + .has_channel_buffer_changed(channel_id) + .unwrap() + }); + + assert!(has_buffer_changed); } #[track_caller] diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 4f81d07ea72b6f376d22b7b01fc185c35ccc1583..0fd265d0aa0c7e22408950f3114bc724a5e9fd98 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1774,6 +1774,7 @@ impl CollabPanel { const FACEPILE_LIMIT: usize = 3; enum ChannelCall {} + enum ChannelNote {} let mut is_dragged_over = false; if cx @@ -1820,7 +1821,7 @@ impl CollabPanel { channel.name.clone(), theme .channel_name - .in_state(channel.has_changed) + .in_state(channel.has_note_changed) .text .clone(), ) @@ -1863,6 +1864,8 @@ impl CollabPanel { .with_color(theme.channel_hash.color) .constrained() .with_width(theme.channel_hash.width) + .contained() + .with_margin_right(theme.channel_hash.container.margin.left) .into_any() } else { Empty::new().into_any() @@ -1872,6 +1875,34 @@ impl CollabPanel { this.join_channel_call(channel_id, cx); }), ) + .with_child( + MouseEventHandler::new::(ix, cx, move |_, cx| { + let participants = + self.channel_store.read(cx).channel_participants(channel_id); + if participants.is_empty() { + if channel.has_note_changed { + Svg::new("icons/terminal.svg") + .with_color(theme.channel_note_active_color) + .constrained() + .with_width(theme.channel_hash.width) + .into_any() + } else if row_hovered { + Svg::new("icons/terminal.svg") + .with_color(theme.channel_hash.color) + .constrained() + .with_width(theme.channel_hash.width) + .into_any() + } else { + Empty::new().into_any() + } + } else { + Empty::new().into_any() + } + }) + .on_click(MouseButton::Left, move |_, this, cx| { + this.open_channel_notes(&OpenChannelNotes { channel_id }, cx); + }), + ) .align_children_center() .styleable_component() .disclosable( diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 6c32bfd1293d680514bd387d5aaa708ce4a8ad28..875fdd892e2daa6203f50f4c8ce3bbc7dd57ebc5 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -238,6 +238,7 @@ pub struct CollabPanel { pub log_in_button: Interactive, pub channel_editor: ContainerStyle, pub channel_hash: Icon, + pub channel_note_active_color: Color, pub tabbed_modal: TabbedModal, pub contact_finder: ContactFinder, pub channel_modal: ChannelModal, diff --git a/styles/src/style_tree/collab_panel.ts b/styles/src/style_tree/collab_panel.ts index 33b36e36f4b0e52c6cfb42385398fc5d46bae456..2a7702842a121e7caf8cf3c5d2e7dd3192b7b48d 100644 --- a/styles/src/style_tree/collab_panel.ts +++ b/styles/src/style_tree/collab_panel.ts @@ -194,6 +194,7 @@ export default function contacts_panel(): any { }, user_query_editor: filter_input, channel_hash: icon_style, + channel_note_active_color: foreground(layer, "active"), user_query_editor_height: 33, add_contact_button: header_icon_button, add_channel_button: header_icon_button, From 51cf6a5ff34e188685f17d70c242c52b0ff5c20a Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 1 Oct 2023 21:31:36 -0700 Subject: [PATCH 04/20] Add database implementation of channel message change tracking --- .../20221109000000_test_schema.sql | 9 ++ .../20230925210437_add_channel_changes.sql | 18 +++ .../20230925210437_add_observed_notes.sql | 9 -- crates/collab/src/db/queries.rs | 10 ++ crates/collab/src/db/queries/buffers.rs | 10 -- crates/collab/src/db/queries/messages.rs | 121 +++++++++++++++ crates/collab/src/db/tables.rs | 1 + .../db/tables/observed_channel_messages.rs | 41 ++++++ crates/collab/src/db/tests/message_tests.rs | 139 ++++++++++++++++++ 9 files changed, 339 insertions(+), 19 deletions(-) create mode 100644 crates/collab/migrations/20230925210437_add_channel_changes.sql delete mode 100644 crates/collab/migrations/20230925210437_add_observed_notes.sql create mode 100644 crates/collab/src/db/tables/observed_channel_messages.rs diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 7553392f396b5ca228041e602543902936a5e2c9..277a78d2d643c7141ff7252c867c8bf1d73b6448 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -300,3 +300,12 @@ CREATE TABLE "observed_buffer_edits" ( ); CREATE UNIQUE INDEX "index_observed_buffers_user_and_buffer_id" ON "observed_buffer_edits" ("user_id", "buffer_id"); + +CREATE TABLE IF NOT EXISTS "observed_channel_messages" ( + "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "channel_message_id" INTEGER NOT NULL, + PRIMARY KEY (user_id, channel_id) +); + +CREATE UNIQUE INDEX "index_observed_channel_messages_user_and_channel_id" ON "observed_channel_messages" ("user_id", "channel_id"); diff --git a/crates/collab/migrations/20230925210437_add_channel_changes.sql b/crates/collab/migrations/20230925210437_add_channel_changes.sql new file mode 100644 index 0000000000000000000000000000000000000000..7787975c1c385263131599545eb8931a1b77dd40 --- /dev/null +++ b/crates/collab/migrations/20230925210437_add_channel_changes.sql @@ -0,0 +1,18 @@ +CREATE TABLE IF NOT EXISTS "observed_buffer_edits" ( + "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, + "epoch" INTEGER NOT NULL, + "lamport_timestamp" INTEGER NOT NULL, + PRIMARY KEY (user_id, buffer_id) +); + +CREATE UNIQUE INDEX "index_observed_buffer_user_and_buffer_id" ON "observed_buffer_edits" ("user_id", "buffer_id"); + +CREATE TABLE IF NOT EXISTS "observed_channel_messages" ( + "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, + "channel_message_id" INTEGER NOT NULL, + PRIMARY KEY (user_id, channel_id) +); + +CREATE UNIQUE INDEX "index_observed_channel_messages_user_and_channel_id" ON "observed_channel_messages" ("user_id", "channel_id"); diff --git a/crates/collab/migrations/20230925210437_add_observed_notes.sql b/crates/collab/migrations/20230925210437_add_observed_notes.sql deleted file mode 100644 index 4574fe215ba009dc76dc8ef62937b733e436c94b..0000000000000000000000000000000000000000 --- a/crates/collab/migrations/20230925210437_add_observed_notes.sql +++ /dev/null @@ -1,9 +0,0 @@ -CREATE TABLE "observed_buffer_edits" ( - "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, - "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, - "epoch" INTEGER NOT NULL, - "lamport_timestamp" INTEGER NOT NULL, - PRIMARY KEY (user_id, buffer_id) -); - -CREATE UNIQUE INDEX "index_observed_buffer_user_and_buffer_id" ON "observed_buffer_edits" ("user_id", "buffer_id"); diff --git a/crates/collab/src/db/queries.rs b/crates/collab/src/db/queries.rs index 80bd8704b27704361241a93c56f5945ef51ef3cc..59face1f331ef89502bc2d6a4b99f6207730d3ea 100644 --- a/crates/collab/src/db/queries.rs +++ b/crates/collab/src/db/queries.rs @@ -9,3 +9,13 @@ pub mod projects; pub mod rooms; pub mod servers; pub mod users; + +fn max_assign(max: &mut Option, val: T) { + if let Some(max_val) = max { + if val > *max_val { + *max = Some(val); + } + } else { + *max = Some(val); + } +} diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index f2993c516d0c53a1c548c9156e6d6c0b72daf909..1e8dd30c6b3e950206583b3fd87e668f8c3f4e7f 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -787,16 +787,6 @@ impl Database { } } -fn max_assign(max: &mut Option, val: T) { - if let Some(max_val) = max { - if val > *max_val { - *max = Some(val); - } - } else { - *max = Some(val); - } -} - fn operation_to_storage( operation: &proto::Operation, buffer: &buffer::Model, diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 0b88df6716c0068755e0345513d50784386d2e82..328737dd0ad5098c9062b07659598197e120e061 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -93,9 +93,13 @@ impl Database { .stream(&*tx) .await?; + let mut max_id = None; let mut messages = Vec::new(); while let Some(row) = rows.next().await { let row = row?; + dbg!(&max_id); + max_assign(&mut max_id, row.id); + let nonce = row.nonce.as_u64_pair(); messages.push(proto::ChannelMessage { id: row.id.to_proto(), @@ -108,6 +112,55 @@ impl Database { }), }); } + drop(rows); + dbg!(&max_id); + + if let Some(max_id) = max_id { + let has_older_message = dbg!( + observed_channel_messages::Entity::find() + .filter( + observed_channel_messages::Column::UserId + .eq(user_id) + .and(observed_channel_messages::Column::ChannelId.eq(channel_id)) + .and( + observed_channel_messages::Column::ChannelMessageId.lt(max_id) + ), + ) + .one(&*tx) + .await + )? + .is_some(); + + if has_older_message { + observed_channel_messages::Entity::update( + observed_channel_messages::ActiveModel { + user_id: ActiveValue::Unchanged(user_id), + channel_id: ActiveValue::Unchanged(channel_id), + channel_message_id: ActiveValue::Set(max_id), + }, + ) + .exec(&*tx) + .await?; + } else { + observed_channel_messages::Entity::insert( + observed_channel_messages::ActiveModel { + user_id: ActiveValue::Set(user_id), + channel_id: ActiveValue::Set(channel_id), + channel_message_id: ActiveValue::Set(max_id), + }, + ) + .on_conflict( + OnConflict::columns([ + observed_channel_messages::Column::UserId, + observed_channel_messages::Column::ChannelId, + ]) + .update_columns([observed_channel_messages::Column::ChannelMessageId]) + .to_owned(), + ) + .exec(&*tx) + .await?; + } + } Ok(messages) }) @@ -130,11 +183,13 @@ impl Database { let mut is_participant = false; let mut participant_connection_ids = Vec::new(); + let mut participant_user_ids = Vec::new(); while let Some(row) = rows.next().await { let row = row?; if row.user_id == user_id { is_participant = true; } + participant_user_ids.push(row.user_id); participant_connection_ids.push(row.connection()); } drop(rows); @@ -167,11 +222,77 @@ impl Database { ConnectionId, } + // Observe this message for all participants + observed_channel_messages::Entity::insert_many(participant_user_ids.iter().map( + |pariticpant_id| observed_channel_messages::ActiveModel { + user_id: ActiveValue::Set(*pariticpant_id), + channel_id: ActiveValue::Set(channel_id), + channel_message_id: ActiveValue::Set(message.last_insert_id), + }, + )) + .on_conflict( + OnConflict::columns([ + observed_channel_messages::Column::ChannelId, + observed_channel_messages::Column::UserId, + ]) + .update_column(observed_channel_messages::Column::ChannelMessageId) + .to_owned(), + ) + .exec(&*tx) + .await?; + Ok((message.last_insert_id, participant_connection_ids)) }) .await } + #[cfg(test)] + pub async fn has_new_message_tx(&self, channel_id: ChannelId, user_id: UserId) -> Result { + self.transaction(|tx| async move { self.has_new_message(channel_id, user_id, &*tx).await }) + .await + } + + #[cfg(test)] + pub async fn dbg_print_messages(&self) -> Result<()> { + self.transaction(|tx| async move { + dbg!(observed_channel_messages::Entity::find() + .all(&*tx) + .await + .unwrap()); + dbg!(channel_message::Entity::find().all(&*tx).await.unwrap()); + + Ok(()) + }) + .await + } + + pub async fn has_new_message( + &self, + channel_id: ChannelId, + user_id: UserId, + tx: &DatabaseTransaction, + ) -> Result { + self.check_user_is_channel_member(channel_id, user_id, &*tx) + .await?; + + let latest_message_id = channel_message::Entity::find() + .filter(Condition::all().add(channel_message::Column::ChannelId.eq(channel_id))) + .order_by(channel_message::Column::SentAt, sea_query::Order::Desc) + .limit(1 as u64) + .one(&*tx) + .await? + .map(|model| model.id); + + let last_message_read = observed_channel_messages::Entity::find() + .filter(observed_channel_messages::Column::ChannelId.eq(channel_id)) + .filter(observed_channel_messages::Column::UserId.eq(user_id)) + .one(&*tx) + .await? + .map(|model| model.channel_message_id); + + Ok(dbg!(last_message_read) != dbg!(latest_message_id)) + } + pub async fn remove_channel_message( &self, channel_id: ChannelId, diff --git a/crates/collab/src/db/tables.rs b/crates/collab/src/db/tables.rs index 40686065467051ab127587ec7069b28b0bd007e1..e19391da7dd513970b0fa593d7977fa7689c0510 100644 --- a/crates/collab/src/db/tables.rs +++ b/crates/collab/src/db/tables.rs @@ -13,6 +13,7 @@ pub mod feature_flag; pub mod follower; pub mod language_server; pub mod observed_buffer_edits; +pub mod observed_channel_messages; pub mod project; pub mod project_collaborator; pub mod room; diff --git a/crates/collab/src/db/tables/observed_channel_messages.rs b/crates/collab/src/db/tables/observed_channel_messages.rs new file mode 100644 index 0000000000000000000000000000000000000000..18259f844274750ebcb463c7d69d619457055d89 --- /dev/null +++ b/crates/collab/src/db/tables/observed_channel_messages.rs @@ -0,0 +1,41 @@ +use crate::db::{ChannelId, MessageId, UserId}; +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "observed_channel_messages")] +pub struct Model { + #[sea_orm(primary_key)] + pub user_id: UserId, + pub channel_id: ChannelId, + pub channel_message_id: MessageId, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::channel::Entity", + from = "Column::ChannelId", + to = "super::channel::Column::Id" + )] + Channel, + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::UserId", + to = "super::user::Column::Id" + )] + User, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Channel.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::User.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index c40e53d3559e4e65ab68e5fb061901418c02ab3f..2c745bb8aeb1a3f6e2fe45e991a02e113b0609b9 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -57,3 +57,142 @@ async fn test_channel_message_nonces(db: &Arc) { assert_eq!(msg1_id, msg3_id); assert_eq!(msg2_id, msg4_id); } + +test_both_dbs!( + test_channel_message_new_notification, + test_channel_message_new_notification_postgres, + test_channel_message_new_notification_sqlite +); + +async fn test_channel_message_new_notification(db: &Arc) { + let user_a = db + .create_user( + "user_a@example.com", + false, + NewUserParams { + github_login: "user_a".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id; + let user_b = db + .create_user( + "user_b@example.com", + false, + NewUserParams { + github_login: "user_b".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id; + + let channel = db + .create_channel("channel", None, "room", user_a) + .await + .unwrap(); + + db.invite_channel_member(channel, user_b, user_a, false) + .await + .unwrap(); + + db.respond_to_channel_invite(channel, user_b, true) + .await + .unwrap(); + + let owner_id = db.create_server("test").await.unwrap().0 as u32; + + // Zero case: no messages at all + assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + + let a_connection_id = rpc::ConnectionId { owner_id, id: 0 }; + db.join_channel_chat(channel, a_connection_id, user_a) + .await + .unwrap(); + + let _ = db + .create_channel_message(channel, user_a, "1", OffsetDateTime::now_utc(), 1) + .await + .unwrap(); + + let (second_message, _) = db + .create_channel_message(channel, user_a, "2", OffsetDateTime::now_utc(), 2) + .await + .unwrap(); + + let _ = db + .create_channel_message(channel, user_a, "3", OffsetDateTime::now_utc(), 3) + .await + .unwrap(); + + // Smoke test: can we detect a new message? + assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + + let b_connection_id = rpc::ConnectionId { owner_id, id: 1 }; + db.join_channel_chat(channel, b_connection_id, user_b) + .await + .unwrap(); + + // Joining the channel should _not_ update us to the latest message + assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + + // Reading the earlier messages should not change that we have new messages + let _ = db + .get_channel_messages(channel, user_b, 1, Some(second_message)) + .await + .unwrap(); + + assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + + // This constraint is currently inexpressible, creating a message implicitly broadcasts + // it to all participants + // + // Creating new messages when we haven't read the latest one should not change the flag + // let _ = db + // .create_channel_message(channel, user_a, "4", OffsetDateTime::now_utc(), 4) + // .await + // .unwrap(); + // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + + // But reading the latest message should clear the flag + let _ = db + .get_channel_messages(channel, user_b, 4, None) + .await + .unwrap(); + + assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + + // And future messages should not reset the flag + let _ = db + .create_channel_message(channel, user_a, "5", OffsetDateTime::now_utc(), 5) + .await + .unwrap(); + + assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + + let _ = db + .create_channel_message(channel, user_b, "6", OffsetDateTime::now_utc(), 6) + .await + .unwrap(); + + assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + + // And we should start seeing the flag again after we've left the channel + db.leave_channel_chat(channel, b_connection_id, user_b) + .await + .unwrap(); + + assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + + let _ = db + .create_channel_message(channel, user_a, "7", OffsetDateTime::now_utc(), 7) + .await + .unwrap(); + + assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); +} From 1d5b665f13886f1b7c56002d6f09110b9efa7483 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 1 Oct 2023 22:21:27 -0700 Subject: [PATCH 05/20] Implement channel changes for messages --- crates/channel/src/channel_store.rs | 28 ++++- .../src/channel_store/channel_index.rs | 13 +++ crates/collab/src/db.rs | 3 +- crates/collab/src/db/queries/channels.rs | 5 + crates/collab/src/db/queries/messages.rs | 40 +++---- crates/collab/src/db/tests/message_tests.rs | 2 +- crates/collab/src/rpc.rs | 38 ++++++- .../collab/src/tests/channel_buffer_tests.rs | 1 + .../collab/src/tests/channel_message_tests.rs | 105 +++++++++++++++++- crates/collab_ui/src/collab_panel.rs | 2 +- crates/gpui/src/app.rs | 2 +- crates/rpc/proto/zed.proto | 1 + 12 files changed, 212 insertions(+), 28 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 417f486b9e2407622ac24f367c59bc344f60344e..f0f66f4839ea6ec19d042c7cce3caaf0994c28bb 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -44,6 +44,7 @@ pub struct Channel { pub id: ChannelId, pub name: String, pub has_note_changed: bool, + pub has_new_messages: bool, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] @@ -223,6 +224,13 @@ impl ChannelStore { .map(|channel| channel.has_note_changed) } + pub fn has_new_messages(&self, channel_id: ChannelId) -> Option { + self.channel_index + .by_id() + .get(&channel_id) + .map(|channel| channel.has_new_messages) + } + pub fn open_channel_chat( &mut self, channel_id: ChannelId, @@ -230,12 +238,20 @@ impl ChannelStore { ) -> Task>> { let client = self.client.clone(); let user_store = self.user_store.clone(); - self.open_channel_resource( + let open_channel_chat = self.open_channel_resource( channel_id, |this| &mut this.opened_chats, |channel, cx| ChannelChat::new(channel, user_store, client, cx), cx, - ) + ); + cx.spawn(|this, mut cx| async move { + let chat = open_channel_chat.await?; + this.update(&mut cx, |this, cx| { + this.channel_index.clear_message_changed(channel_id); + cx.notify(); + }); + Ok(chat) + }) } /// Asynchronously open a given resource associated with a channel. @@ -796,6 +812,7 @@ impl ChannelStore { id: channel.id, name: channel.name, has_note_changed: false, + has_new_messages: false, }), ), } @@ -805,7 +822,8 @@ impl ChannelStore { || !payload.delete_channels.is_empty() || !payload.insert_edge.is_empty() || !payload.delete_edge.is_empty() - || !payload.notes_changed.is_empty(); + || !payload.notes_changed.is_empty() + || !payload.new_messages.is_empty(); if channels_changed { if !payload.delete_channels.is_empty() { @@ -836,6 +854,10 @@ impl ChannelStore { index.note_changed(id_changed); } + for id_changed in payload.new_messages { + index.new_messages(id_changed); + } + for edge in payload.insert_edge { index.insert_edge(edge.channel_id, edge.parent_id); } diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 42474f1d85da1cf3d581daaae25e58bd2c580d5a..513e20e3a771185f0b8a517b6bc3c6186c9e132c 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -44,6 +44,12 @@ impl ChannelIndex { Arc::make_mut(channel).has_note_changed = false; } } + + pub fn clear_message_changed(&mut self, channel_id: ChannelId) { + if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { + Arc::make_mut(channel).has_new_messages = false; + } + } } impl Deref for ChannelIndex { @@ -88,6 +94,12 @@ impl<'a> ChannelPathsInsertGuard<'a> { } } + pub fn new_messages(&mut self, channel_id: ChannelId) { + if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { + Arc::make_mut(channel).has_new_messages = true; + } + } + pub fn insert(&mut self, channel_proto: proto::Channel) { if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) { Arc::make_mut(existing_channel).name = channel_proto.name; @@ -98,6 +110,7 @@ impl<'a> ChannelPathsInsertGuard<'a> { id: channel_proto.id, name: channel_proto.name, has_note_changed: false, + has_new_messages: false, }), ); self.insert_root(channel_proto.id); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index f0896f873219b958ee33a723b933920d653c0132..8f7f9cc9751254155ad588766c0541a6921a21b0 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -436,8 +436,9 @@ pub struct Channel { pub struct ChannelsForUser { pub channels: ChannelGraph, pub channel_participants: HashMap>, - pub channels_with_changed_notes: HashSet, pub channels_with_admin_privileges: HashSet, + pub channels_with_changed_notes: HashSet, + pub channels_with_new_messages: HashSet, } #[derive(Debug)] diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 6274550c25dd7702af492c531f7dab72553a965c..ea9f64fe5efa267730275b2d3cf25d42e40ec151 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -464,10 +464,14 @@ impl Database { } let mut channels_with_changed_notes = HashSet::default(); + let mut channels_with_new_messages = HashSet::default(); for channel in graph.channels.iter() { if self.has_note_changed(user_id, channel.id, tx).await? { channels_with_changed_notes.insert(channel.id); } + if self.has_new_message(channel.id, user_id, tx).await? { + channels_with_new_messages.insert(channel.id); + } } Ok(ChannelsForUser { @@ -475,6 +479,7 @@ impl Database { channel_participants, channels_with_admin_privileges, channels_with_changed_notes, + channels_with_new_messages, }) } diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 328737dd0ad5098c9062b07659598197e120e061..8e3c92d916ea082d3892428c5fee4589122b3255 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -97,7 +97,7 @@ impl Database { let mut messages = Vec::new(); while let Some(row) = rows.next().await { let row = row?; - dbg!(&max_id); + max_assign(&mut max_id, row.id); let nonce = row.nonce.as_u64_pair(); @@ -113,23 +113,18 @@ impl Database { }); } drop(rows); - dbg!(&max_id); if let Some(max_id) = max_id { - let has_older_message = dbg!( - observed_channel_messages::Entity::find() - .filter( - observed_channel_messages::Column::UserId - .eq(user_id) - .and(observed_channel_messages::Column::ChannelId.eq(channel_id)) - .and( - observed_channel_messages::Column::ChannelMessageId.lt(max_id) - ), - ) - .one(&*tx) - .await - )? - .is_some(); + let has_older_message = observed_channel_messages::Entity::find() + .filter( + observed_channel_messages::Column::UserId + .eq(user_id) + .and(observed_channel_messages::Column::ChannelId.eq(channel_id)) + .and(observed_channel_messages::Column::ChannelMessageId.lt(max_id)), + ) + .one(&*tx) + .await? + .is_some(); if has_older_message { observed_channel_messages::Entity::update( @@ -174,7 +169,7 @@ impl Database { body: &str, timestamp: OffsetDateTime, nonce: u128, - ) -> Result<(MessageId, Vec)> { + ) -> Result<(MessageId, Vec, Vec)> { self.transaction(|tx| async move { let mut rows = channel_chat_participant::Entity::find() .filter(channel_chat_participant::Column::ChannelId.eq(channel_id)) @@ -241,7 +236,14 @@ impl Database { .exec(&*tx) .await?; - Ok((message.last_insert_id, participant_connection_ids)) + let mut channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; + channel_members.retain(|member| !participant_user_ids.contains(member)); + + Ok(( + message.last_insert_id, + participant_connection_ids, + channel_members, + )) }) .await } @@ -290,7 +292,7 @@ impl Database { .await? .map(|model| model.channel_message_id); - Ok(dbg!(last_message_read) != dbg!(latest_message_id)) + Ok(last_message_read != latest_message_id) } pub async fn remove_channel_message( diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index 2c745bb8aeb1a3f6e2fe45e991a02e113b0609b9..98b8cc603767990b510ce9ed6d3fd7ca55a81ab0 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -120,7 +120,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - let (second_message, _) = db + let (second_message, _, _) = db .create_channel_message(channel, user_a, "2", OffsetDateTime::now_utc(), 2) .await .unwrap(); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index b9dae999cd1eb06bddc29c5d2cafae2def54a2bf..371d0466c1fc956a64c3fa9142fe7fea6ad0dd18 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -2568,6 +2568,16 @@ async fn respond_to_channel_invite( name: channel.name, }), ); + update.notes_changed = result + .channels_with_changed_notes + .iter() + .map(|id| id.to_proto()) + .collect(); + update.new_messages = result + .channels_with_new_messages + .iter() + .map(|id| id.to_proto()) + .collect(); update.insert_edge = result.channels.edges; update .channel_participants @@ -2818,7 +2828,7 @@ async fn send_channel_message( .ok_or_else(|| anyhow!("nonce can't be blank"))?; let channel_id = ChannelId::from_proto(request.channel_id); - let (message_id, connection_ids) = session + let (message_id, connection_ids, non_participants) = session .db() .await .create_channel_message( @@ -2848,6 +2858,26 @@ async fn send_channel_message( response.send(proto::SendChannelMessageResponse { message: Some(message), })?; + + dbg!(&non_participants); + let pool = &*session.connection_pool().await; + + broadcast( + None, + non_participants + .iter() + .flat_map(|user_id| pool.user_connection_ids(*user_id)), + |peer_id| { + session.peer.send( + peer_id.into(), + proto::UpdateChannels { + new_messages: vec![channel_id.to_proto()], + ..Default::default() + }, + ) + }, + ); + Ok(()) } @@ -3011,6 +3041,12 @@ fn build_initial_channels_update( .map(|channel_id| channel_id.to_proto()) .collect(); + update.new_messages = channels + .channels_with_new_messages + .iter() + .map(|channel_id| channel_id.to_proto()) + .collect(); + update.insert_edge = channels.channels.edges; for (channel_id, participants) in channels.channel_participants { diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 7ca6f0db3f6b3c48ec1bdd249748225e040f7cec..68acffacf8fc0271a92a796b5b7603643b771896 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -446,6 +446,7 @@ fn channel(id: u64, name: &'static str) -> Channel { id, name: name.to_string(), has_note_changed: false, + has_new_messages: false, } } diff --git a/crates/collab/src/tests/channel_message_tests.rs b/crates/collab/src/tests/channel_message_tests.rs index 58494c538b4a57c4ee8117136b77df190a8744f9..1b3a54dc428dfdcae10b3ebc5f4dcb451bf6a514 100644 --- a/crates/collab/src/tests/channel_message_tests.rs +++ b/crates/collab/src/tests/channel_message_tests.rs @@ -1,6 +1,6 @@ use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer}; use channel::{ChannelChat, ChannelMessageId}; -use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; +use gpui::{executor::Deterministic, BorrowAppContext, ModelHandle, TestAppContext}; use std::sync::Arc; #[gpui::test] @@ -223,3 +223,106 @@ fn assert_messages(chat: &ModelHandle, messages: &[&str], cx: &mut messages ); } + +#[gpui::test] +async fn test_channel_message_changes( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + let channel_id = server + .make_channel( + "the-channel", + None, + (&client_a, cx_a), + &mut [(&client_b, cx_b)], + ) + .await; + + // Client A sends a message, client B should see that there is a new message. + let channel_chat_a = client_a + .channel_store() + .update(cx_a, |store, cx| store.open_channel_chat(channel_id, cx)) + .await + .unwrap(); + + channel_chat_a + .update(cx_a, |c, cx| c.send_message("one".into(), cx).unwrap()) + .await + .unwrap(); + + deterministic.run_until_parked(); + + let b_has_messages = cx_b.read_with(|cx| { + client_b + .channel_store() + .read(cx) + .has_new_messages(channel_id) + .unwrap() + }); + + assert!(b_has_messages); + + // Opening the chat should clear the changed flag. + let channel_chat_b = client_b + .channel_store() + .update(cx_b, |store, cx| store.open_channel_chat(channel_id, cx)) + .await + .unwrap(); + + let b_has_messages = cx_b.read_with(|cx| { + client_b + .channel_store() + .read(cx) + .has_new_messages(channel_id) + .unwrap() + }); + + assert!(!b_has_messages); + + // Sending a message while the chat is open should not change the flag. + channel_chat_a + .update(cx_a, |c, cx| c.send_message("two".into(), cx).unwrap()) + .await + .unwrap(); + + deterministic.run_until_parked(); + + let b_has_messages = cx_b.read_with(|cx| { + client_b + .channel_store() + .read(cx) + .has_new_messages(channel_id) + .unwrap() + }); + + assert!(!b_has_messages); + + // Closing the chat should re-enable change tracking + + cx_b.update(|_| { + drop(channel_chat_b); + }); + + deterministic.run_until_parked(); + + channel_chat_a + .update(cx_a, |c, cx| c.send_message("three".into(), cx).unwrap()) + .await + .unwrap(); + + let b_has_messages = cx_b.read_with(|cx| { + client_b + .channel_store() + .read(cx) + .has_new_messages(channel_id) + .unwrap() + }); + + assert!(b_has_messages); +} diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 0fd265d0aa0c7e22408950f3114bc724a5e9fd98..53d5140a12a43172a25a64cf554a9efd68fd2bbb 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1821,7 +1821,7 @@ impl CollabPanel { channel.name.clone(), theme .channel_name - .in_state(channel.has_note_changed) + .in_state(channel.has_new_messages) .text .clone(), ) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 6b3cd03e8fa57e34284c36bc8020ae8222898241..edcc7ad6f60fd86fe47beadfebc8fab7ebd32120 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1252,7 +1252,7 @@ impl AppContext { result }) } else { - panic!("circular model update"); + panic!("circular model update for {}", std::any::type_name::()); } } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index c53db447d3d811e566fce9081ad0c21ad3fec6f0..d0c9d73fc4433731f88ac069a9e3f35deaf44dea 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -956,6 +956,7 @@ message UpdateChannels { repeated ChannelParticipants channel_participants = 7; repeated ChannelPermission channel_permissions = 8; repeated uint64 notes_changed = 9; + repeated uint64 new_messages = 10; } message ChannelEdge { From 84c4db13fb52543c1ef22465d885cf33e19f24c6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 2 Oct 2023 15:57:59 -0700 Subject: [PATCH 06/20] Avoid spurious notifies in chat channel select Co-authored-by: Mikayla --- crates/gpui/src/views/select.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/gpui/src/views/select.rs b/crates/gpui/src/views/select.rs index bad65ccfc8e0d9bf583847ac12058d3e08ad2865..b1ea201fdfcb48be9bfe6ff81e879d5b6196b165 100644 --- a/crates/gpui/src/views/select.rs +++ b/crates/gpui/src/views/select.rs @@ -53,8 +53,10 @@ impl Select { } pub fn set_item_count(&mut self, count: usize, cx: &mut ViewContext) { - self.item_count = count; - cx.notify(); + if count != self.item_count { + self.item_count = count; + cx.notify(); + } } fn toggle(&mut self, cx: &mut ViewContext) { @@ -63,9 +65,11 @@ impl Select { } pub fn set_selected_index(&mut self, ix: usize, cx: &mut ViewContext) { - self.selected_item_ix = ix; - self.is_open = false; - cx.notify(); + if ix != self.selected_item_ix || self.is_open { + self.selected_item_ix = ix; + self.is_open = false; + cx.notify(); + } } pub fn selected_index(&self) -> usize { From d9d997b218712b6efdf673841edbe68a0d03297e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 2 Oct 2023 15:58:34 -0700 Subject: [PATCH 07/20] Avoid N+1 query for channels with notes changes Also, start work on new timing for recording observed notes edits. Co-authored-by: Mikayla --- .../20221109000000_test_schema.sql | 1 + .../20230925210437_add_channel_changes.sql | 1 + crates/collab/src/db.rs | 4 +- crates/collab/src/db/queries/buffers.rs | 269 ++++++++++++------ crates/collab/src/db/queries/channels.rs | 12 +- .../src/db/tables/observed_buffer_edits.rs | 1 + crates/collab/src/db/tests/buffer_tests.rs | 190 +++++++++++++ 7 files changed, 381 insertions(+), 97 deletions(-) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 277a78d2d643c7141ff7252c867c8bf1d73b6448..2d963ff15fa4717aa7faee092356f4b06d8a5814 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -296,6 +296,7 @@ CREATE TABLE "observed_buffer_edits" ( "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, "epoch" INTEGER NOT NULL, "lamport_timestamp" INTEGER NOT NULL, + "replica_id" INTEGER NOT NULL, PRIMARY KEY (user_id, buffer_id) ); diff --git a/crates/collab/migrations/20230925210437_add_channel_changes.sql b/crates/collab/migrations/20230925210437_add_channel_changes.sql index 7787975c1c385263131599545eb8931a1b77dd40..250a9ac731b59489e85cf34a6754307bfac543ee 100644 --- a/crates/collab/migrations/20230925210437_add_channel_changes.sql +++ b/crates/collab/migrations/20230925210437_add_channel_changes.sql @@ -3,6 +3,7 @@ CREATE TABLE IF NOT EXISTS "observed_buffer_edits" ( "buffer_id" INTEGER NOT NULL REFERENCES buffers (id) ON DELETE CASCADE, "epoch" INTEGER NOT NULL, "lamport_timestamp" INTEGER NOT NULL, + "replica_id" INTEGER NOT NULL, PRIMARY KEY (user_id, buffer_id) ); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 8f7f9cc9751254155ad588766c0541a6921a21b0..b0223bbf27523d384902ef13e67e5df0d9023f5d 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -119,7 +119,7 @@ impl Database { Ok(new_migrations) } - async fn transaction(&self, f: F) -> Result + pub async fn transaction(&self, f: F) -> Result where F: Send + Fn(TransactionHandle) -> Fut, Fut: Send + Future>, @@ -321,7 +321,7 @@ fn is_serialization_error(error: &Error) -> bool { } } -struct TransactionHandle(Arc>); +pub struct TransactionHandle(Arc>); impl Deref for TransactionHandle { type Target = DatabaseTransaction; diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 1e8dd30c6b3e950206583b3fd87e668f8c3f4e7f..b22bfc80cfed20e7cdf74cad24e8bbd320b76105 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -79,12 +79,13 @@ impl Database { self.get_buffer_state(&buffer, &tx).await?; // Save the last observed operation - if let Some(max_operation) = max_operation { + if let Some(op) = max_operation { observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { user_id: ActiveValue::Set(user_id), buffer_id: ActiveValue::Set(buffer.id), - epoch: ActiveValue::Set(max_operation.0), - lamport_timestamp: ActiveValue::Set(max_operation.1), + epoch: ActiveValue::Set(op.epoch), + lamport_timestamp: ActiveValue::Set(op.lamport_timestamp), + replica_id: ActiveValue::Set(op.replica_id), }) .on_conflict( OnConflict::columns([ @@ -99,37 +100,6 @@ impl Database { ) .exec(&*tx) .await?; - } else { - let buffer_max = buffer_operation::Entity::find() - .filter(buffer_operation::Column::BufferId.eq(buffer.id)) - .filter(buffer_operation::Column::Epoch.eq(buffer.epoch.saturating_sub(1))) - .order_by(buffer_operation::Column::Epoch, Desc) - .order_by(buffer_operation::Column::LamportTimestamp, Desc) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); - - if let Some(buffer_max) = buffer_max { - observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { - user_id: ActiveValue::Set(user_id), - buffer_id: ActiveValue::Set(buffer.id), - epoch: ActiveValue::Set(buffer_max.0), - lamport_timestamp: ActiveValue::Set(buffer_max.1), - }) - .on_conflict( - OnConflict::columns([ - observed_buffer_edits::Column::UserId, - observed_buffer_edits::Column::BufferId, - ]) - .update_columns([ - observed_buffer_edits::Column::Epoch, - observed_buffer_edits::Column::LamportTimestamp, - ]) - .to_owned(), - ) - .exec(&*tx) - .await?; - } } Ok(proto::JoinChannelBufferResponse { @@ -487,13 +457,8 @@ impl Database { if !operations.is_empty() { // get current channel participants and save the max operation above - self.save_max_operation_for_collaborators( - operations.as_slice(), - channel_id, - buffer.id, - &*tx, - ) - .await?; + self.save_max_operation(user, buffer.id, buffer.epoch, operations.as_slice(), &*tx) + .await?; channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; let collaborators = self @@ -539,54 +504,55 @@ impl Database { .await } - async fn save_max_operation_for_collaborators( + async fn save_max_operation( &self, - operations: &[buffer_operation::ActiveModel], - channel_id: ChannelId, + user_id: UserId, buffer_id: BufferId, + epoch: i32, + operations: &[buffer_operation::ActiveModel], tx: &DatabaseTransaction, ) -> Result<()> { + use observed_buffer_edits::Column; + let max_operation = operations .iter() - .map(|storage_model| { - ( - storage_model.epoch.clone(), - storage_model.lamport_timestamp.clone(), - ) - }) - .max_by( - |(epoch_a, lamport_timestamp_a), (epoch_b, lamport_timestamp_b)| { - epoch_a.as_ref().cmp(epoch_b.as_ref()).then( - lamport_timestamp_a - .as_ref() - .cmp(lamport_timestamp_b.as_ref()), - ) - }, - ) + .max_by_key(|op| (op.lamport_timestamp.as_ref(), op.replica_id.as_ref())) .unwrap(); - let users = self - .get_channel_buffer_collaborators_internal(channel_id, tx) - .await?; - - observed_buffer_edits::Entity::insert_many(users.iter().map(|id| { - observed_buffer_edits::ActiveModel { - user_id: ActiveValue::Set(*id), - buffer_id: ActiveValue::Set(buffer_id), - epoch: max_operation.0.clone(), - lamport_timestamp: ActiveValue::Set(*max_operation.1.as_ref()), - } - })) + observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { + user_id: ActiveValue::Set(user_id), + buffer_id: ActiveValue::Set(buffer_id), + epoch: ActiveValue::Set(epoch), + replica_id: max_operation.replica_id.clone(), + lamport_timestamp: max_operation.lamport_timestamp.clone(), + }) .on_conflict( - OnConflict::columns([ - observed_buffer_edits::Column::UserId, - observed_buffer_edits::Column::BufferId, - ]) - .update_columns([ - observed_buffer_edits::Column::Epoch, - observed_buffer_edits::Column::LamportTimestamp, - ]) - .to_owned(), + OnConflict::columns([Column::UserId, Column::BufferId]) + .update_columns([Column::Epoch, Column::LamportTimestamp, Column::ReplicaId]) + .target_cond_where( + Condition::any() + .add(Column::Epoch.lt(*max_operation.epoch.as_ref())) + .add( + Condition::all() + .add(Column::Epoch.eq(*max_operation.epoch.as_ref())) + .add( + Condition::any() + .add( + Column::LamportTimestamp + .lt(*max_operation.lamport_timestamp.as_ref()), + ) + .add( + Column::LamportTimestamp + .eq(*max_operation.lamport_timestamp.as_ref()) + .and( + Column::ReplicaId + .lt(*max_operation.replica_id.as_ref()), + ), + ), + ), + ), + ) + .to_owned(), ) .exec(tx) .await?; @@ -611,7 +577,7 @@ impl Database { .ok_or_else(|| anyhow!("missing buffer snapshot"))?) } - async fn get_channel_buffer( + pub async fn get_channel_buffer( &self, channel_id: ChannelId, tx: &DatabaseTransaction, @@ -630,7 +596,11 @@ impl Database { &self, buffer: &buffer::Model, tx: &DatabaseTransaction, - ) -> Result<(String, Vec, Option<(i32, i32)>)> { + ) -> Result<( + String, + Vec, + Option, + )> { let id = buffer.id; let (base_text, version) = if buffer.epoch > 0 { let snapshot = buffer_snapshot::Entity::find() @@ -655,24 +625,28 @@ impl Database { .eq(id) .and(buffer_operation::Column::Epoch.eq(buffer.epoch)), ) + .order_by_asc(buffer_operation::Column::LamportTimestamp) + .order_by_asc(buffer_operation::Column::ReplicaId) .stream(&*tx) .await?; - let mut operations = Vec::new(); - let mut max_epoch: Option = None; - let mut max_timestamp: Option = None; + let mut operations = Vec::new(); + let mut last_row = None; while let Some(row) = rows.next().await { let row = row?; - - max_assign(&mut max_epoch, row.epoch); - max_assign(&mut max_timestamp, row.lamport_timestamp); - + last_row = Some(buffer_operation::Model { + buffer_id: row.buffer_id, + epoch: row.epoch, + lamport_timestamp: row.lamport_timestamp, + replica_id: row.lamport_timestamp, + value: Default::default(), + }); operations.push(proto::Operation { variant: Some(operation_from_storage(row, version)?), - }) + }); } - Ok((base_text, operations, max_epoch.zip(max_timestamp))) + Ok((base_text, operations, last_row)) } async fn snapshot_channel_buffer( @@ -725,6 +699,119 @@ impl Database { .await } + pub async fn channels_with_changed_notes( + &self, + user_id: UserId, + channel_ids: impl IntoIterator, + tx: &DatabaseTransaction, + ) -> Result> { + #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] + enum QueryIds { + ChannelId, + Id, + } + + let mut channel_ids_by_buffer_id = HashMap::default(); + let mut rows = buffer::Entity::find() + .filter(buffer::Column::ChannelId.is_in(channel_ids)) + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row = row?; + channel_ids_by_buffer_id.insert(row.id, row.channel_id); + } + drop(rows); + + let mut observed_edits_by_buffer_id = HashMap::default(); + let mut rows = observed_buffer_edits::Entity::find() + .filter(observed_buffer_edits::Column::UserId.eq(user_id)) + .filter( + observed_buffer_edits::Column::BufferId + .is_in(channel_ids_by_buffer_id.keys().copied()), + ) + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row = row?; + observed_edits_by_buffer_id.insert(row.buffer_id, row); + } + drop(rows); + + let last_operations = self + .get_last_operations_for_buffers(channel_ids_by_buffer_id.keys().copied(), &*tx) + .await?; + + let mut channels_with_new_changes = HashSet::default(); + for last_operation in last_operations { + if let Some(observed_edit) = observed_edits_by_buffer_id.get(&last_operation.buffer_id) + { + if observed_edit.epoch == last_operation.epoch + && observed_edit.lamport_timestamp == last_operation.lamport_timestamp + && observed_edit.replica_id == last_operation.replica_id + { + continue; + } + } + + if let Some(channel_id) = channel_ids_by_buffer_id.get(&last_operation.buffer_id) { + channels_with_new_changes.insert(*channel_id); + } + } + + Ok(channels_with_new_changes) + } + + pub async fn get_last_operations_for_buffers( + &self, + channel_ids: impl IntoIterator, + tx: &DatabaseTransaction, + ) -> Result> { + let mut values = String::new(); + for id in channel_ids { + if !values.is_empty() { + values.push_str(", "); + } + write!(&mut values, "({})", id).unwrap(); + } + + if values.is_empty() { + return Ok(Vec::default()); + } + + let sql = format!( + r#" + SELECT + * + FROM ( + SELECT + buffer_id, + epoch, + lamport_timestamp, + replica_id, + value, + row_number() OVER ( + PARTITION BY buffer_id + ORDER BY + epoch DESC, + lamport_timestamp DESC, + replica_id DESC + ) as row_number + FROM buffer_operations + WHERE + buffer_id in ({values}) + ) AS operations + WHERE + row_number = 1 + "#, + ); + + let stmt = Statement::from_string(self.pool.get_database_backend(), sql); + let operations = buffer_operation::Model::find_by_statement(stmt) + .all(&*tx) + .await?; + Ok(operations) + } + pub async fn has_note_changed( &self, user_id: UserId, diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index ea9f64fe5efa267730275b2d3cf25d42e40ec151..6d976b310e6ee41ae85dd95bde13fb6ca071c32e 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -463,12 +463,16 @@ impl Database { } } - let mut channels_with_changed_notes = HashSet::default(); + let channels_with_changed_notes = self + .channels_with_changed_notes( + user_id, + graph.channels.iter().map(|channel| channel.id), + &*tx, + ) + .await?; + let mut channels_with_new_messages = HashSet::default(); for channel in graph.channels.iter() { - if self.has_note_changed(user_id, channel.id, tx).await? { - channels_with_changed_notes.insert(channel.id); - } if self.has_new_message(channel.id, user_id, tx).await? { channels_with_new_messages.insert(channel.id); } diff --git a/crates/collab/src/db/tables/observed_buffer_edits.rs b/crates/collab/src/db/tables/observed_buffer_edits.rs index db027f78b2b868e668433d23d5ef657cdd4825ef..e8e7aafaa285cc6584577994b682bbc5d30a3a7c 100644 --- a/crates/collab/src/db/tables/observed_buffer_edits.rs +++ b/crates/collab/src/db/tables/observed_buffer_edits.rs @@ -9,6 +9,7 @@ pub struct Model { pub buffer_id: BufferId, pub epoch: i32, pub lamport_timestamp: i32, + pub replica_id: i32, } #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 115a20ffa6fc31a809625827d05e024082bd8e7d..5a5fe6a8129142bdffb4f0594728b42b9c0a098d 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -272,3 +272,193 @@ async fn test_channel_buffers_diffs(db: &Database) { assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); assert!(!db.test_has_note_changed(b_id, zed_id).await.unwrap()); } + +test_both_dbs!( + test_channel_buffers_last_operations, + test_channel_buffers_last_operations_postgres, + test_channel_buffers_last_operations_sqlite +); + +async fn test_channel_buffers_last_operations(db: &Database) { + let user_id = db + .create_user( + "user_a@example.com", + false, + NewUserParams { + github_login: "user_a".into(), + github_user_id: 101, + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id; + let owner_id = db.create_server("production").await.unwrap().0 as u32; + let connection_id = ConnectionId { + owner_id, + id: user_id.0 as u32, + }; + + let mut buffers = Vec::new(); + let mut text_buffers = Vec::new(); + for i in 0..3 { + let channel = db + .create_root_channel(&format!("channel-{i}"), &format!("room-{i}"), user_id) + .await + .unwrap(); + + db.join_channel_buffer(channel, user_id, connection_id) + .await + .unwrap(); + + buffers.push( + db.transaction(|tx| async move { db.get_channel_buffer(channel, &*tx).await }) + .await + .unwrap(), + ); + + text_buffers.push(Buffer::new(0, 0, "".to_string())); + } + + let operations = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.get_last_operations_for_buffers([buffers[0].id, buffers[2].id], &*tx) + .await + } + }) + .await + .unwrap(); + + assert!(operations.is_empty()); + + update_buffer( + buffers[0].channel_id, + user_id, + db, + vec![ + text_buffers[0].edit([(0..0, "a")]), + text_buffers[0].edit([(0..0, "b")]), + text_buffers[0].edit([(0..0, "c")]), + ], + ) + .await; + + update_buffer( + buffers[1].channel_id, + user_id, + db, + vec![ + text_buffers[1].edit([(0..0, "d")]), + text_buffers[1].edit([(1..1, "e")]), + text_buffers[1].edit([(2..2, "f")]), + ], + ) + .await; + + // cause buffer 1's epoch to increment. + db.leave_channel_buffer(buffers[1].channel_id, connection_id) + .await + .unwrap(); + db.join_channel_buffer(buffers[1].channel_id, user_id, connection_id) + .await + .unwrap(); + text_buffers[1] = Buffer::new(1, 0, "def".to_string()); + update_buffer( + buffers[1].channel_id, + user_id, + db, + vec![ + text_buffers[1].edit([(0..0, "g")]), + text_buffers[1].edit([(0..0, "h")]), + ], + ) + .await; + + update_buffer( + buffers[2].channel_id, + user_id, + db, + vec![text_buffers[2].edit([(0..0, "i")])], + ) + .await; + + let operations = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.get_last_operations_for_buffers([buffers[1].id, buffers[2].id], &*tx) + .await + } + }) + .await + .unwrap(); + assert_operations( + &operations, + &[ + (buffers[1].id, 1, &text_buffers[1]), + (buffers[2].id, 0, &text_buffers[2]), + ], + ); + + let operations = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.get_last_operations_for_buffers([buffers[0].id, buffers[1].id], &*tx) + .await + } + }) + .await + .unwrap(); + assert_operations( + &operations, + &[ + (buffers[0].id, 0, &text_buffers[0]), + (buffers[1].id, 1, &text_buffers[1]), + ], + ); + + async fn update_buffer( + channel_id: ChannelId, + user_id: UserId, + db: &Database, + operations: Vec, + ) { + let operations = operations + .into_iter() + .map(|op| proto::serialize_operation(&language::Operation::Buffer(op))) + .collect::>(); + db.update_channel_buffer(channel_id, user_id, &operations) + .await + .unwrap(); + } + + fn assert_operations( + operations: &[buffer_operation::Model], + expected: &[(BufferId, i32, &text::Buffer)], + ) { + let actual = operations + .iter() + .map(|op| buffer_operation::Model { + buffer_id: op.buffer_id, + epoch: op.epoch, + lamport_timestamp: op.lamport_timestamp, + replica_id: op.replica_id, + value: vec![], + }) + .collect::>(); + let expected = expected + .iter() + .map(|(buffer_id, epoch, buffer)| buffer_operation::Model { + buffer_id: *buffer_id, + epoch: *epoch, + lamport_timestamp: buffer.lamport_clock.value as i32 - 1, + replica_id: buffer.replica_id() as i32, + value: vec![], + }) + .collect::>(); + assert_eq!(actual, expected, "unexpected operations") + } +} From 0db4b294524c718fce6c1b04bffe0bfe1484375d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 2 Oct 2023 16:22:28 -0700 Subject: [PATCH 08/20] Avoid N+1 query for channels with new messages Co-authored-by: Mikayla --- crates/collab/src/db/queries/buffers.rs | 6 +- crates/collab/src/db/queries/channels.rs | 16 +-- crates/collab/src/db/queries/messages.rs | 113 ++++++++++++-------- crates/collab/src/db/tests/buffer_tests.rs | 2 + crates/collab/src/db/tests/message_tests.rs | 20 ++-- 5 files changed, 88 insertions(+), 69 deletions(-) diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index b22bfc80cfed20e7cdf74cad24e8bbd320b76105..6a74ae4d446913328a3f81cc4618092ab9ca9778 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -529,7 +529,7 @@ impl Database { .on_conflict( OnConflict::columns([Column::UserId, Column::BufferId]) .update_columns([Column::Epoch, Column::LamportTimestamp, Column::ReplicaId]) - .target_cond_where( + .action_cond_where( Condition::any() .add(Column::Epoch.lt(*max_operation.epoch.as_ref())) .add( @@ -702,7 +702,7 @@ impl Database { pub async fn channels_with_changed_notes( &self, user_id: UserId, - channel_ids: impl IntoIterator, + channel_ids: &[ChannelId], tx: &DatabaseTransaction, ) -> Result> { #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] @@ -713,7 +713,7 @@ impl Database { let mut channel_ids_by_buffer_id = HashMap::default(); let mut rows = buffer::Entity::find() - .filter(buffer::Column::ChannelId.is_in(channel_ids)) + .filter(buffer::Column::ChannelId.is_in(channel_ids.iter().copied())) .stream(&*tx) .await?; while let Some(row) = rows.next().await { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 6d976b310e6ee41ae85dd95bde13fb6ca071c32e..8292e9dbcbc06b2e19895d670c2b8066eef71d29 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -463,20 +463,14 @@ impl Database { } } + let channel_ids = graph.channels.iter().map(|c| c.id).collect::>(); let channels_with_changed_notes = self - .channels_with_changed_notes( - user_id, - graph.channels.iter().map(|channel| channel.id), - &*tx, - ) + .channels_with_changed_notes(user_id, &channel_ids, &*tx) .await?; - let mut channels_with_new_messages = HashSet::default(); - for channel in graph.channels.iter() { - if self.has_new_message(channel.id, user_id, tx).await? { - channels_with_new_messages.insert(channel.id); - } - } + let channels_with_new_messages = self + .channels_with_new_messages(user_id, &channel_ids, &*tx) + .await?; Ok(ChannelsForUser { channels: graph, diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 8e3c92d916ea082d3892428c5fee4589122b3255..484509f68542c4320688dcba7ff34f4254eace61 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -217,14 +217,12 @@ impl Database { ConnectionId, } - // Observe this message for all participants - observed_channel_messages::Entity::insert_many(participant_user_ids.iter().map( - |pariticpant_id| observed_channel_messages::ActiveModel { - user_id: ActiveValue::Set(*pariticpant_id), - channel_id: ActiveValue::Set(channel_id), - channel_message_id: ActiveValue::Set(message.last_insert_id), - }, - )) + // Observe this message for the sender + observed_channel_messages::Entity::insert(observed_channel_messages::ActiveModel { + user_id: ActiveValue::Set(user_id), + channel_id: ActiveValue::Set(channel_id), + channel_message_id: ActiveValue::Set(message.last_insert_id), + }) .on_conflict( OnConflict::columns([ observed_channel_messages::Column::ChannelId, @@ -248,51 +246,74 @@ impl Database { .await } - #[cfg(test)] - pub async fn has_new_message_tx(&self, channel_id: ChannelId, user_id: UserId) -> Result { - self.transaction(|tx| async move { self.has_new_message(channel_id, user_id, &*tx).await }) - .await - } - - #[cfg(test)] - pub async fn dbg_print_messages(&self) -> Result<()> { - self.transaction(|tx| async move { - dbg!(observed_channel_messages::Entity::find() - .all(&*tx) - .await - .unwrap()); - dbg!(channel_message::Entity::find().all(&*tx).await.unwrap()); - - Ok(()) - }) - .await - } - - pub async fn has_new_message( + pub async fn channels_with_new_messages( &self, - channel_id: ChannelId, user_id: UserId, + channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result { - self.check_user_is_channel_member(channel_id, user_id, &*tx) + ) -> Result> { + let mut observed_messages_by_channel_id = HashMap::default(); + let mut rows = observed_channel_messages::Entity::find() + .filter(observed_channel_messages::Column::UserId.eq(user_id)) + .filter(observed_channel_messages::Column::ChannelId.is_in(channel_ids.iter().copied())) + .stream(&*tx) .await?; - let latest_message_id = channel_message::Entity::find() - .filter(Condition::all().add(channel_message::Column::ChannelId.eq(channel_id))) - .order_by(channel_message::Column::SentAt, sea_query::Order::Desc) - .limit(1 as u64) - .one(&*tx) - .await? - .map(|model| model.id); + while let Some(row) = rows.next().await { + let row = row?; + observed_messages_by_channel_id.insert(row.channel_id, row); + } + drop(rows); + let mut values = String::new(); + for id in channel_ids { + if !values.is_empty() { + values.push_str(", "); + } + write!(&mut values, "({})", id).unwrap(); + } + + if values.is_empty() { + return Ok(Default::default()); + } + + let sql = format!( + r#" + SELECT + * + FROM ( + SELECT + *, + row_number() OVER ( + PARTITION BY channel_id + ORDER BY id DESC + ) as row_number + FROM channel_messages + WHERE + channel_id in ({values}) + ) AS messages + WHERE + row_number = 1 + "#, + ); + + let stmt = Statement::from_string(self.pool.get_database_backend(), sql); + let last_messages = channel_message::Model::find_by_statement(stmt) + .all(&*tx) + .await?; - let last_message_read = observed_channel_messages::Entity::find() - .filter(observed_channel_messages::Column::ChannelId.eq(channel_id)) - .filter(observed_channel_messages::Column::UserId.eq(user_id)) - .one(&*tx) - .await? - .map(|model| model.channel_message_id); + let mut channels_with_new_changes = HashSet::default(); + for last_message in last_messages { + if let Some(observed_message) = + observed_messages_by_channel_id.get(&last_message.channel_id) + { + if observed_message.channel_message_id == last_message.id { + continue; + } + } + channels_with_new_changes.insert(last_message.channel_id); + } - Ok(last_message_read != latest_message_id) + Ok(channels_with_new_changes) } pub async fn remove_channel_message( diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 5a5fe6a8129142bdffb4f0594728b42b9c0a098d..d8edef963ab564ebcad7a839b0c3a23c5ee2700b 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -171,6 +171,8 @@ test_both_dbs!( ); async fn test_channel_buffers_diffs(db: &Database) { + panic!("Rewriting the way this works"); + let a_id = db .create_user( "user_a@example.com", diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index 98b8cc603767990b510ce9ed6d3fd7ca55a81ab0..e212c36466a89479fe512f96840e7f0a7441762b 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -65,6 +65,8 @@ test_both_dbs!( ); async fn test_channel_message_new_notification(db: &Arc) { + panic!("Rewriting the way this works"); + let user_a = db .create_user( "user_a@example.com", @@ -108,7 +110,7 @@ async fn test_channel_message_new_notification(db: &Arc) { let owner_id = db.create_server("test").await.unwrap().0 as u32; // Zero case: no messages at all - assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); let a_connection_id = rpc::ConnectionId { owner_id, id: 0 }; db.join_channel_chat(channel, a_connection_id, user_a) @@ -131,7 +133,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .unwrap(); // Smoke test: can we detect a new message? - assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); let b_connection_id = rpc::ConnectionId { owner_id, id: 1 }; db.join_channel_chat(channel, b_connection_id, user_b) @@ -139,7 +141,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .unwrap(); // Joining the channel should _not_ update us to the latest message - assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); // Reading the earlier messages should not change that we have new messages let _ = db @@ -147,7 +149,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); // This constraint is currently inexpressible, creating a message implicitly broadcasts // it to all participants @@ -165,7 +167,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); // And future messages should not reset the flag let _ = db @@ -173,26 +175,26 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); let _ = db .create_channel_message(channel, user_b, "6", OffsetDateTime::now_utc(), 6) .await .unwrap(); - assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); // And we should start seeing the flag again after we've left the channel db.leave_channel_chat(channel, b_connection_id, user_b) .await .unwrap(); - assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); let _ = db .create_channel_message(channel, user_a, "7", OffsetDateTime::now_utc(), 7) .await .unwrap(); - assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); } From 32b4b4d24d29ee988764651d817d9ee254abdde7 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 2 Oct 2023 17:10:03 -0700 Subject: [PATCH 09/20] Add message and operation ACK messages to protos --- crates/rpc/proto/zed.proto | 16 +++++++++++++++- crates/rpc/src/proto.rs | 4 +++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index d0c9d73fc4433731f88ac069a9e3f35deaf44dea..0f289edcf8bd91b79405d3df8c7ef7e265702c3f 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -155,6 +155,7 @@ message Envelope { UpdateChannelBufferCollaborators update_channel_buffer_collaborators = 128; RejoinChannelBuffers rejoin_channel_buffers = 129; RejoinChannelBuffersResponse rejoin_channel_buffers_response = 130; + AckBufferOperation ack_buffer_operation = 143; JoinChannelChat join_channel_chat = 131; JoinChannelChatResponse join_channel_chat_response = 132; @@ -165,10 +166,11 @@ message Envelope { GetChannelMessages get_channel_messages = 137; GetChannelMessagesResponse get_channel_messages_response = 138; RemoveChannelMessage remove_channel_message = 139; + AckChannelMessage ack_channel_message = 144; LinkChannel link_channel = 140; UnlinkChannel unlink_channel = 141; - MoveChannel move_channel = 142; + MoveChannel move_channel = 142; // current max: 144 } } @@ -1062,6 +1064,11 @@ message RemoveChannelMessage { uint64 message_id = 2; } +message AckChannelMessage { + uint64 channel_id = 1; + uint64 message_id = 2; +} + message SendChannelMessageResponse { ChannelMessage message = 1; } @@ -1117,6 +1124,13 @@ message RejoinChannelBuffersResponse { repeated RejoinedChannelBuffer buffers = 1; } +message AckBufferOperation { + uint64 buffer_id = 1; + uint64 epoch = 2; + uint64 lamport_timestamp = 3; + uint64 replica_id = 4; +} + message JoinChannelBufferResponse { uint64 buffer_id = 1; uint32 replica_id = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 6d0a0f85d12447e8b57c90dfc02d8ce1110b7a31..f0d7937f6f9592b2574bff7b93d3097ceba24d94 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -271,6 +271,8 @@ messages!( (LeaveChannelBuffer, Background), (UpdateChannelBuffer, Foreground), (UpdateChannelBufferCollaborators, Foreground), + (AckBufferOperation, Background), + (AckChannelMessage, Background), ); request_messages!( @@ -406,7 +408,7 @@ entity_messages!( ChannelMessageSent, UpdateChannelBuffer, RemoveChannelMessage, - UpdateChannelBufferCollaborators + UpdateChannelBufferCollaborators, ); const KIB: usize = 1024; From 32c413875891b249f11fb005f10d59dc8bc4276a Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 11:39:59 -0700 Subject: [PATCH 10/20] Added db message and edit operation observation Co-authored-by: Max --- crates/collab/src/db/queries/buffers.rs | 146 ++++------ crates/collab/src/db/queries/messages.rs | 61 +++- crates/collab/src/db/tests/buffer_tests.rs | 298 ++++++++++---------- crates/collab/src/db/tests/message_tests.rs | 139 +++++---- 4 files changed, 331 insertions(+), 313 deletions(-) diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 6a74ae4d446913328a3f81cc4618092ab9ca9778..78ccd9e54af5d8c2f9266054e20e5f3872404803 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -1,6 +1,5 @@ use super::*; use prost::Message; -use sea_query::Order::Desc; use text::{EditOperation, UndoOperation}; pub struct LeftChannelBuffer { @@ -456,9 +455,21 @@ impl Database { let mut channel_members; if !operations.is_empty() { + let max_operation = operations + .iter() + .max_by_key(|op| (op.lamport_timestamp.as_ref(), op.replica_id.as_ref())) + .unwrap(); + // get current channel participants and save the max operation above - self.save_max_operation(user, buffer.id, buffer.epoch, operations.as_slice(), &*tx) - .await?; + self.save_max_operation( + user, + buffer.id, + buffer.epoch, + *max_operation.replica_id.as_ref(), + *max_operation.lamport_timestamp.as_ref(), + &*tx, + ) + .await?; channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; let collaborators = self @@ -509,52 +520,38 @@ impl Database { user_id: UserId, buffer_id: BufferId, epoch: i32, - operations: &[buffer_operation::ActiveModel], + replica_id: i32, + lamport_timestamp: i32, tx: &DatabaseTransaction, ) -> Result<()> { use observed_buffer_edits::Column; - let max_operation = operations - .iter() - .max_by_key(|op| (op.lamport_timestamp.as_ref(), op.replica_id.as_ref())) - .unwrap(); - observed_buffer_edits::Entity::insert(observed_buffer_edits::ActiveModel { user_id: ActiveValue::Set(user_id), buffer_id: ActiveValue::Set(buffer_id), epoch: ActiveValue::Set(epoch), - replica_id: max_operation.replica_id.clone(), - lamport_timestamp: max_operation.lamport_timestamp.clone(), + replica_id: ActiveValue::Set(replica_id), + lamport_timestamp: ActiveValue::Set(lamport_timestamp), }) .on_conflict( OnConflict::columns([Column::UserId, Column::BufferId]) .update_columns([Column::Epoch, Column::LamportTimestamp, Column::ReplicaId]) .action_cond_where( - Condition::any() - .add(Column::Epoch.lt(*max_operation.epoch.as_ref())) - .add( - Condition::all() - .add(Column::Epoch.eq(*max_operation.epoch.as_ref())) + Condition::any().add(Column::Epoch.lt(epoch)).add( + Condition::all().add(Column::Epoch.eq(epoch)).add( + Condition::any() + .add(Column::LamportTimestamp.lt(lamport_timestamp)) .add( - Condition::any() - .add( - Column::LamportTimestamp - .lt(*max_operation.lamport_timestamp.as_ref()), - ) - .add( - Column::LamportTimestamp - .eq(*max_operation.lamport_timestamp.as_ref()) - .and( - Column::ReplicaId - .lt(*max_operation.replica_id.as_ref()), - ), - ), + Column::LamportTimestamp + .eq(lamport_timestamp) + .and(Column::ReplicaId.lt(replica_id)), ), ), + ), ) .to_owned(), ) - .exec(tx) + .exec_without_returning(tx) .await?; Ok(()) @@ -689,14 +686,30 @@ impl Database { Ok(()) } - #[cfg(test)] - pub async fn test_has_note_changed( + pub async fn observe_buffer_version( &self, + buffer_id: BufferId, user_id: UserId, - channel_id: ChannelId, - ) -> Result { - self.transaction(|tx| async move { self.has_note_changed(user_id, channel_id, &*tx).await }) - .await + epoch: i32, + version: &[proto::VectorClockEntry], + ) -> Result<()> { + self.transaction(|tx| async move { + // For now, combine concurrent operations. + let Some(component) = version.iter().max_by_key(|version| version.timestamp) else { + return Ok(()); + }; + self.save_max_operation( + user_id, + buffer_id, + epoch, + component.replica_id as i32, + component.timestamp as i32, + &*tx, + ) + .await?; + Ok(()) + }) + .await } pub async fn channels_with_changed_notes( @@ -811,67 +824,6 @@ impl Database { .await?; Ok(operations) } - - pub async fn has_note_changed( - &self, - user_id: UserId, - channel_id: ChannelId, - tx: &DatabaseTransaction, - ) -> Result { - let Some(buffer_id) = channel::Model { - id: channel_id, - ..Default::default() - } - .find_related(buffer::Entity) - .one(&*tx) - .await? - .map(|buffer| buffer.id) else { - return Ok(false); - }; - - let user_max = observed_buffer_edits::Entity::find() - .filter(observed_buffer_edits::Column::UserId.eq(user_id)) - .filter(observed_buffer_edits::Column::BufferId.eq(buffer_id)) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); - - let channel_buffer = channel::Model { - id: channel_id, - ..Default::default() - } - .find_related(buffer::Entity) - .one(&*tx) - .await?; - - let Some(channel_buffer) = channel_buffer else { - return Ok(false); - }; - - let mut channel_max = buffer_operation::Entity::find() - .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) - .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch)) - .order_by(buffer_operation::Column::Epoch, Desc) - .order_by(buffer_operation::Column::LamportTimestamp, Desc) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); - - // If there are no edits in this epoch - if channel_max.is_none() { - // check if this user observed the last edit of the previous epoch - channel_max = buffer_operation::Entity::find() - .filter(buffer_operation::Column::BufferId.eq(channel_buffer.id)) - .filter(buffer_operation::Column::Epoch.eq(channel_buffer.epoch.saturating_sub(1))) - .order_by(buffer_operation::Column::Epoch, Desc) - .order_by(buffer_operation::Column::LamportTimestamp, Desc) - .one(&*tx) - .await? - .map(|model| (model.epoch, model.lamport_timestamp)); - } - - Ok(user_max != channel_max) - } } fn operation_to_storage( diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 484509f68542c4320688dcba7ff34f4254eace61..893c1726da3b44ee27e83c1d1b5e1ac35fad9697 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -218,20 +218,12 @@ impl Database { } // Observe this message for the sender - observed_channel_messages::Entity::insert(observed_channel_messages::ActiveModel { - user_id: ActiveValue::Set(user_id), - channel_id: ActiveValue::Set(channel_id), - channel_message_id: ActiveValue::Set(message.last_insert_id), - }) - .on_conflict( - OnConflict::columns([ - observed_channel_messages::Column::ChannelId, - observed_channel_messages::Column::UserId, - ]) - .update_column(observed_channel_messages::Column::ChannelMessageId) - .to_owned(), + self.observe_channel_message_internal( + channel_id, + user_id, + message.last_insert_id, + &*tx, ) - .exec(&*tx) .await?; let mut channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; @@ -246,12 +238,53 @@ impl Database { .await } + pub async fn observe_channel_message( + &self, + channel_id: ChannelId, + user_id: UserId, + message_id: MessageId, + ) -> Result<()> { + self.transaction(|tx| async move { + self.observe_channel_message_internal(channel_id, user_id, message_id, &*tx) + .await?; + Ok(()) + }) + .await + } + + async fn observe_channel_message_internal( + &self, + channel_id: ChannelId, + user_id: UserId, + message_id: MessageId, + tx: &DatabaseTransaction, + ) -> Result<()> { + observed_channel_messages::Entity::insert(observed_channel_messages::ActiveModel { + user_id: ActiveValue::Set(user_id), + channel_id: ActiveValue::Set(channel_id), + channel_message_id: ActiveValue::Set(message_id), + }) + .on_conflict( + OnConflict::columns([ + observed_channel_messages::Column::ChannelId, + observed_channel_messages::Column::UserId, + ]) + .update_column(observed_channel_messages::Column::ChannelMessageId) + .action_cond_where(observed_channel_messages::Column::ChannelMessageId.lt(message_id)) + .to_owned(), + ) + // TODO: Try to upgrade SeaORM so we don't have to do this hack around their bug + .exec_without_returning(&*tx) + .await?; + Ok(()) + } + pub async fn channels_with_new_messages( &self, user_id: UserId, channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result> { + ) -> Result> { let mut observed_messages_by_channel_id = HashMap::default(); let mut rows = observed_channel_messages::Entity::find() .filter(observed_channel_messages::Column::UserId.eq(user_id)) diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index d8edef963ab564ebcad7a839b0c3a23c5ee2700b..407cc2210830ff89e04f38ec0efc430a0b8df01e 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -1,6 +1,6 @@ use super::*; use crate::test_both_dbs; -use language::proto; +use language::proto::{self, serialize_version}; use text::Buffer; test_both_dbs!( @@ -165,15 +165,13 @@ async fn test_channel_buffers(db: &Arc) { } test_both_dbs!( - test_channel_buffers_diffs, - test_channel_buffers_diffs_postgres, - test_channel_buffers_diffs_sqlite + test_channel_buffers_last_operations, + test_channel_buffers_last_operations_postgres, + test_channel_buffers_last_operations_sqlite ); -async fn test_channel_buffers_diffs(db: &Database) { - panic!("Rewriting the way this works"); - - let a_id = db +async fn test_channel_buffers_last_operations(db: &Database) { + let user_id = db .create_user( "user_a@example.com", false, @@ -186,7 +184,7 @@ async fn test_channel_buffers_diffs(db: &Database) { .await .unwrap() .user_id; - let b_id = db + let observer_id = db .create_user( "user_b@example.com", false, @@ -199,102 +197,6 @@ async fn test_channel_buffers_diffs(db: &Database) { .await .unwrap() .user_id; - - let owner_id = db.create_server("production").await.unwrap().0 as u32; - - let zed_id = db.create_root_channel("zed", "1", a_id).await.unwrap(); - - db.invite_channel_member(zed_id, b_id, a_id, false) - .await - .unwrap(); - - db.respond_to_channel_invite(zed_id, b_id, true) - .await - .unwrap(); - - let connection_id_a = ConnectionId { - owner_id, - id: a_id.0 as u32, - }; - let connection_id_b = ConnectionId { - owner_id, - id: b_id.0 as u32, - }; - - // Zero test: A should not register as changed on an unitialized channel buffer - assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); - - let _ = db - .join_channel_buffer(zed_id, a_id, connection_id_a) - .await - .unwrap(); - - // Zero test: A should register as changed on an empty channel buffer - assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); - - let mut buffer_a = Buffer::new(0, 0, "".to_string()); - let mut operations = Vec::new(); - operations.push(buffer_a.edit([(0..0, "hello world")])); - assert_eq!(buffer_a.text(), "hello world"); - - let operations = operations - .into_iter() - .map(|op| proto::serialize_operation(&language::Operation::Buffer(op))) - .collect::>(); - - db.update_channel_buffer(zed_id, a_id, &operations) - .await - .unwrap(); - - // Smoke test: Does B register as changed, A as unchanged? - assert!(db.test_has_note_changed(b_id, zed_id).await.unwrap()); - - assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); - - db.leave_channel_buffer(zed_id, connection_id_a) - .await - .unwrap(); - - // Snapshotting from leaving the channel buffer should not have a diff - assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); - - let _ = db - .join_channel_buffer(zed_id, b_id, connection_id_b) - .await - .unwrap(); - - // B has opened the channel buffer, so we shouldn't have any diff - assert!(!db.test_has_note_changed(b_id, zed_id).await.unwrap()); - - db.leave_channel_buffer(zed_id, connection_id_b) - .await - .unwrap(); - - // Since B just opened and closed the buffer without editing, neither should have a diff - assert!(!db.test_has_note_changed(a_id, zed_id).await.unwrap()); - assert!(!db.test_has_note_changed(b_id, zed_id).await.unwrap()); -} - -test_both_dbs!( - test_channel_buffers_last_operations, - test_channel_buffers_last_operations_postgres, - test_channel_buffers_last_operations_sqlite -); - -async fn test_channel_buffers_last_operations(db: &Database) { - let user_id = db - .create_user( - "user_a@example.com", - false, - NewUserParams { - github_login: "user_a".into(), - github_user_id: 101, - invite_count: 0, - }, - ) - .await - .unwrap() - .user_id; let owner_id = db.create_server("production").await.unwrap().0 as u32; let connection_id = ConnectionId { owner_id, @@ -309,6 +211,13 @@ async fn test_channel_buffers_last_operations(db: &Database) { .await .unwrap(); + db.invite_channel_member(channel, observer_id, user_id, false) + .await + .unwrap(); + db.respond_to_channel_invite(channel, observer_id, true) + .await + .unwrap(); + db.join_channel_buffer(channel, user_id, connection_id) .await .unwrap(); @@ -422,45 +331,146 @@ async fn test_channel_buffers_last_operations(db: &Database) { ], ); - async fn update_buffer( - channel_id: ChannelId, - user_id: UserId, - db: &Database, - operations: Vec, - ) { - let operations = operations + let changed_channels = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.channels_with_changed_notes( + observer_id, + &[ + buffers[0].channel_id, + buffers[1].channel_id, + buffers[2].channel_id, + ], + &*tx, + ) + .await + } + }) + .await + .unwrap(); + assert_eq!( + changed_channels, + [ + buffers[0].channel_id, + buffers[1].channel_id, + buffers[2].channel_id, + ] + .into_iter() + .collect::>() + ); + + db.observe_buffer_version( + buffers[1].id, + observer_id, + 1, + &serialize_version(&text_buffers[1].version()), + ) + .await + .unwrap(); + + let changed_channels = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.channels_with_changed_notes( + observer_id, + &[ + buffers[0].channel_id, + buffers[1].channel_id, + buffers[2].channel_id, + ], + &*tx, + ) + .await + } + }) + .await + .unwrap(); + assert_eq!( + changed_channels, + [buffers[0].channel_id, buffers[2].channel_id,] + .into_iter() + .collect::>() + ); + + // Observe an earlier version of the buffer. + db.observe_buffer_version( + buffers[1].id, + observer_id, + 1, + &[rpc::proto::VectorClockEntry { + replica_id: 0, + timestamp: 0, + }], + ) + .await + .unwrap(); + + let changed_channels = db + .transaction(|tx| { + let buffers = &buffers; + async move { + db.channels_with_changed_notes( + observer_id, + &[ + buffers[0].channel_id, + buffers[1].channel_id, + buffers[2].channel_id, + ], + &*tx, + ) + .await + } + }) + .await + .unwrap(); + assert_eq!( + changed_channels, + [buffers[0].channel_id, buffers[2].channel_id,] .into_iter() - .map(|op| proto::serialize_operation(&language::Operation::Buffer(op))) - .collect::>(); - db.update_channel_buffer(channel_id, user_id, &operations) - .await - .unwrap(); - } + .collect::>() + ); +} - fn assert_operations( - operations: &[buffer_operation::Model], - expected: &[(BufferId, i32, &text::Buffer)], - ) { - let actual = operations - .iter() - .map(|op| buffer_operation::Model { - buffer_id: op.buffer_id, - epoch: op.epoch, - lamport_timestamp: op.lamport_timestamp, - replica_id: op.replica_id, - value: vec![], - }) - .collect::>(); - let expected = expected - .iter() - .map(|(buffer_id, epoch, buffer)| buffer_operation::Model { - buffer_id: *buffer_id, - epoch: *epoch, - lamport_timestamp: buffer.lamport_clock.value as i32 - 1, - replica_id: buffer.replica_id() as i32, - value: vec![], - }) - .collect::>(); - assert_eq!(actual, expected, "unexpected operations") - } +async fn update_buffer( + channel_id: ChannelId, + user_id: UserId, + db: &Database, + operations: Vec, +) { + let operations = operations + .into_iter() + .map(|op| proto::serialize_operation(&language::Operation::Buffer(op))) + .collect::>(); + db.update_channel_buffer(channel_id, user_id, &operations) + .await + .unwrap(); +} + +fn assert_operations( + operations: &[buffer_operation::Model], + expected: &[(BufferId, i32, &text::Buffer)], +) { + let actual = operations + .iter() + .map(|op| buffer_operation::Model { + buffer_id: op.buffer_id, + epoch: op.epoch, + lamport_timestamp: op.lamport_timestamp, + replica_id: op.replica_id, + value: vec![], + }) + .collect::>(); + let expected = expected + .iter() + .map(|(buffer_id, epoch, buffer)| buffer_operation::Model { + buffer_id: *buffer_id, + epoch: *epoch, + lamport_timestamp: buffer.lamport_clock.value as i32 - 1, + replica_id: buffer.replica_id() as i32, + value: vec![], + }) + .collect::>(); + assert_eq!(actual, expected, "unexpected operations") } diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index e212c36466a89479fe512f96840e7f0a7441762b..f3d385e4a0f5180ee14a185b649dd89ceea1e2a1 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -65,9 +65,7 @@ test_both_dbs!( ); async fn test_channel_message_new_notification(db: &Arc) { - panic!("Rewriting the way this works"); - - let user_a = db + let user = db .create_user( "user_a@example.com", false, @@ -80,7 +78,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap() .user_id; - let user_b = db + let observer = db .create_user( "user_b@example.com", false, @@ -94,107 +92,132 @@ async fn test_channel_message_new_notification(db: &Arc) { .unwrap() .user_id; - let channel = db - .create_channel("channel", None, "room", user_a) + let channel_1 = db + .create_channel("channel", None, "room", user) .await .unwrap(); - db.invite_channel_member(channel, user_b, user_a, false) + let channel_2 = db + .create_channel("channel-2", None, "room", user) .await .unwrap(); - db.respond_to_channel_invite(channel, user_b, true) + db.invite_channel_member(channel_1, observer, user, false) .await .unwrap(); - let owner_id = db.create_server("test").await.unwrap().0 as u32; - - // Zero case: no messages at all - // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + db.respond_to_channel_invite(channel_1, observer, true) + .await + .unwrap(); - let a_connection_id = rpc::ConnectionId { owner_id, id: 0 }; - db.join_channel_chat(channel, a_connection_id, user_a) + db.invite_channel_member(channel_2, observer, user, false) .await .unwrap(); - let _ = db - .create_channel_message(channel, user_a, "1", OffsetDateTime::now_utc(), 1) + db.respond_to_channel_invite(channel_2, observer, true) .await .unwrap(); - let (second_message, _, _) = db - .create_channel_message(channel, user_a, "2", OffsetDateTime::now_utc(), 2) + let owner_id = db.create_server("test").await.unwrap().0 as u32; + let user_connection_id = rpc::ConnectionId { owner_id, id: 0 }; + + db.join_channel_chat(channel_1, user_connection_id, user) .await .unwrap(); let _ = db - .create_channel_message(channel, user_a, "3", OffsetDateTime::now_utc(), 3) + .create_channel_message(channel_1, user, "1_1", OffsetDateTime::now_utc(), 1) .await .unwrap(); - // Smoke test: can we detect a new message? - // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); - - let b_connection_id = rpc::ConnectionId { owner_id, id: 1 }; - db.join_channel_chat(channel, b_connection_id, user_b) + let (second_message, _, _) = db + .create_channel_message(channel_1, user, "1_2", OffsetDateTime::now_utc(), 2) .await .unwrap(); - // Joining the channel should _not_ update us to the latest message - // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); - - // Reading the earlier messages should not change that we have new messages - let _ = db - .get_channel_messages(channel, user_b, 1, Some(second_message)) + let (third_message, _, _) = db + .create_channel_message(channel_1, user, "1_3", OffsetDateTime::now_utc(), 3) .await .unwrap(); - // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); - - // This constraint is currently inexpressible, creating a message implicitly broadcasts - // it to all participants - // - // Creating new messages when we haven't read the latest one should not change the flag - // let _ = db - // .create_channel_message(channel, user_a, "4", OffsetDateTime::now_utc(), 4) - // .await - // .unwrap(); - // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + db.join_channel_chat(channel_2, user_connection_id, user) + .await + .unwrap(); - // But reading the latest message should clear the flag let _ = db - .get_channel_messages(channel, user_b, 4, None) + .create_channel_message(channel_2, user, "2_1", OffsetDateTime::now_utc(), 4) .await .unwrap(); - // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); - - // And future messages should not reset the flag - let _ = db - .create_channel_message(channel, user_a, "5", OffsetDateTime::now_utc(), 5) + // Check that observer has new messages + let channels_with_new_messages = db + .transaction(|tx| async move { + db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + .await + }) .await .unwrap(); - // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + assert_eq!( + channels_with_new_messages, + [channel_1, channel_2] + .into_iter() + .collect::>() + ); - let _ = db - .create_channel_message(channel, user_b, "6", OffsetDateTime::now_utc(), 6) + // Observe the second message + db.observe_channel_message(channel_1, observer, second_message) .await .unwrap(); - // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // Make sure the observer still has a new message + let channels_with_new_messages = db + .transaction(|tx| async move { + db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + .await + }) + .await + .unwrap(); + assert_eq!( + channels_with_new_messages, + [channel_1, channel_2] + .into_iter() + .collect::>() + ); - // And we should start seeing the flag again after we've left the channel - db.leave_channel_chat(channel, b_connection_id, user_b) + // Observe the third message, + db.observe_channel_message(channel_1, observer, third_message) .await .unwrap(); - // assert!(!db.has_new_message_tx(channel, user_b).await.unwrap()); + // Make sure the observer does not have a new method + let channels_with_new_messages = db + .transaction(|tx| async move { + db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + .await + }) + .await + .unwrap(); + assert_eq!( + channels_with_new_messages, + [channel_2].into_iter().collect::>() + ); - let _ = db - .create_channel_message(channel, user_a, "7", OffsetDateTime::now_utc(), 7) + // Observe the second message again, should not regress our observed state + db.observe_channel_message(channel_1, observer, second_message) .await .unwrap(); - // assert!(db.has_new_message_tx(channel, user_b).await.unwrap()); + // Make sure the observer does not have a new method + let channels_with_new_messages = db + .transaction(|tx| async move { + db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + .await + }) + .await + .unwrap(); + assert_eq!( + channels_with_new_messages, + [channel_2].into_iter().collect::>() + ); } From 6007c8705c38371374c57d94929fca1bb48ae275 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 12:16:53 -0700 Subject: [PATCH 11/20] Upgrade SeaORM to latest version, also upgrade sqlite bindings, rustqlite, and remove SeaQuery co-authored-by: Max --- Cargo.lock | 588 ++++++++++++++++------- Cargo.toml | 1 + crates/ai/Cargo.toml | 2 +- crates/collab/Cargo.toml | 10 +- crates/collab/src/db.rs | 10 +- crates/collab/src/db/ids.rs | 54 +-- crates/collab/src/db/queries/channels.rs | 5 +- crates/collab/src/db/queries/contacts.rs | 4 +- crates/collab/src/db/queries/users.rs | 2 +- crates/collab/src/db/tests.rs | 4 +- crates/semantic_index/Cargo.toml | 2 +- crates/sqlez/Cargo.toml | 2 +- 12 files changed, 427 insertions(+), 257 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76de671620a2539ce9e1f37b085c9cbbb1b30ad2..ffdc7368309aa09cf0101a6cf2bb1799054033bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,12 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "Inflector" -version = "0.11.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe438c63458706e03479442743baae6c88256498e6431708f6dfc520a26515d3" - [[package]] name = "activity_indicator" version = "0.1.0" @@ -73,6 +67,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" dependencies = [ "cfg-if 1.0.0", + "getrandom 0.2.10", "once_cell", "version_check", ] @@ -99,7 +94,7 @@ dependencies = [ "lazy_static", "log", "matrixmultiply", - "ordered-float", + "ordered-float 2.10.0", "parking_lot 0.11.2", "parse_duration", "postage", @@ -314,7 +309,7 @@ dependencies = [ "language", "log", "menu", - "ordered-float", + "ordered-float 2.10.0", "parking_lot 0.11.2", "project", "rand 0.8.5", @@ -587,7 +582,7 @@ dependencies = [ "futures-core", "futures-io", "rustls 0.19.1", - "webpki 0.21.4", + "webpki", "webpki-roots 0.21.1", ] @@ -618,9 +613,9 @@ dependencies = [ [[package]] name = "atoi" -version = "1.0.0" +version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7c57d12312ff59c811c0643f4d80830505833c9ffaebd193d819392b265be8e" +checksum = "f28d99ec8bfea296261ca1af174f24225171fea9664ba9003cbebee704810528" dependencies = [ "num-traits", ] @@ -778,19 +773,6 @@ dependencies = [ "rustc-demangle", ] -[[package]] -name = "bae" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33b8de67cc41132507eeece2584804efcb15f85ba516e34c944b7667f480397a" -dependencies = [ - "heck 0.3.3", - "proc-macro-error", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "base64" version = "0.13.1" @@ -809,6 +791,17 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c3c1a368f70d6cf7302d78f8f7093da241fb8e8807c05cc9e51a125895a6d5b" +[[package]] +name = "bigdecimal" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6773ddc0eafc0e509fb60e48dff7f450f8e674a0686ae8605e8d9901bd5eefa" +dependencies = [ + "num-bigint 0.4.4", + "num-integer", + "num-traits", +] + [[package]] name = "bincode" version = "1.3.3" @@ -1517,7 +1510,6 @@ dependencies = [ "rpc", "scrypt", "sea-orm", - "sea-query", "serde", "serde_derive", "serde_json", @@ -1658,6 +1650,12 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed3d0b5ff30645a68f35ece8cea4556ca14ef8a1651455f789a099a0513532a6" +[[package]] +name = "const-oid" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28c122c3980598d243d63d9a704629a2d748d101f278052ff068be5a4423ab6f" + [[package]] name = "context_menu" version = "0.1.0" @@ -2150,6 +2148,17 @@ dependencies = [ "byteorder", ] +[[package]] +name = "der" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fffa369a668c8af7dbf8b5e56c9f744fbd399949ed171606040001947de40b1c" +dependencies = [ + "const-oid", + "pem-rfc7468", + "zeroize", +] + [[package]] name = "deranged" version = "0.3.8" @@ -2159,6 +2168,17 @@ dependencies = [ "serde", ] +[[package]] +name = "derivative" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcc3dd5e9e9c0b295d6e1e4d811fb6f157d5ffd784b8d202fc62eac8035a770b" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "derive_more" version = "0.99.17" @@ -2244,6 +2264,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer 0.10.4", + "const-oid", "crypto-common", "subtle", ] @@ -2375,7 +2396,7 @@ dependencies = [ "lazy_static", "log", "lsp", - "ordered-float", + "ordered-float 2.10.0", "parking_lot 0.11.2", "postage", "project", @@ -2407,6 +2428,9 @@ name = "either" version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" +dependencies = [ + "serde", +] [[package]] name = "encoding_rs" @@ -2509,6 +2533,17 @@ dependencies = [ "svg_fmt", ] +[[package]] +name = "etcetera" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "136d1b5283a1ab77bd9257427ffd09d8667ced0570b6f938942bc7568ed5b943" +dependencies = [ + "cfg-if 1.0.0", + "home", + "windows-sys", +] + [[package]] name = "euclid" version = "0.22.9" @@ -2680,13 +2715,12 @@ checksum = "7bad48618fdb549078c333a7a8528acb57af271d0433bdecd523eb620628364e" [[package]] name = "flume" -version = "0.10.14" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1657b4441c3403d9f7b3409e47575237dac27b1b5726df654a6ecbf92f0f7577" +checksum = "55ac459de2512911e4b674ce33cf20befaba382d05b62b008afc1c8b57cbf181" dependencies = [ "futures-core", "futures-sink", - "pin-project", "spin 0.9.8", ] @@ -2914,13 +2948,13 @@ dependencies = [ [[package]] name = "futures-intrusive" -version = "0.4.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a604f7a68fbf8103337523b1fadc8ade7361ee3f112f7c680ad179651616aed5" +checksum = "1d930c203dd0b6ff06e0201a4a2fe9149b43c684fd4420555b26d21b1a02956f" dependencies = [ "futures-core", "lock_api", - "parking_lot 0.11.2", + "parking_lot 0.12.1", ] [[package]] @@ -3174,7 +3208,7 @@ dependencies = [ "metal", "num_cpus", "objc", - "ordered-float", + "ordered-float 2.10.0", "parking", "parking_lot 0.11.2", "pathfinder_color", @@ -3306,15 +3340,6 @@ dependencies = [ "allocator-api2", ] -[[package]] -name = "hashlink" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7249a3129cbc1ffccd74857f81464a323a152173cdb134e0fd81bc803b29facf" -dependencies = [ - "hashbrown 0.11.2", -] - [[package]] name = "hashlink" version = "0.8.4" @@ -3636,6 +3661,17 @@ version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bfa799dd5ed20a7e349f3b4639aa80d74549c81716d9ec4f994c9b5815598306" +[[package]] +name = "inherent" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce243b1bfa62ffc028f1cc3b6034ec63d649f3031bc8a4fbbb004e1ac17d1f68" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.37", +] + [[package]] name = "install_cli" version = "0.1.0" @@ -4087,9 +4123,9 @@ checksum = "f7012b1bbb0719e1097c47611d3898568c546d597c2e74d66f6087edd5233ff4" [[package]] name = "libsqlite3-sys" -version = "0.24.2" +version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "898745e570c7d0453cc1fbc4a701eb6c662ed54e8fec8b7d14be137ebeeb9d14" +checksum = "afc22eff61b133b115c6e8c74e818c628d6d5e7a502afea6f64dee076dd94326" dependencies = [ "cc", "pkg-config", @@ -4769,6 +4805,23 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-bigint-dig" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc84195820f291c7697304f3cbdadd1cb7199c0efc917ff5eafd71225c136151" +dependencies = [ + "byteorder", + "lazy_static", + "libm", + "num-integer", + "num-iter", + "num-traits", + "rand 0.8.5", + "smallvec", + "zeroize", +] + [[package]] name = "num-complex" version = "0.2.4" @@ -5027,6 +5080,15 @@ dependencies = [ "num-traits", ] +[[package]] +name = "ordered-float" +version = "3.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a54938017eacd63036332b4ae5c8a49fc8c0c1d6d629893057e4f13609edd06" +dependencies = [ + "num-traits", +] + [[package]] name = "os_str_bytes" version = "6.5.1" @@ -5035,25 +5097,26 @@ checksum = "4d5d9eb14b174ee9aa2ef96dc2b94637a2d4b6e7cb873c7e171f0c20c6cf3eac" [[package]] name = "ouroboros" -version = "0.15.6" +version = "0.17.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1358bd1558bd2a083fed428ffeda486fbfb323e698cdda7794259d592ca72db" +checksum = "e2ba07320d39dfea882faa70554b4bd342a5f273ed59ba7c1c6b4c840492c954" dependencies = [ "aliasable", "ouroboros_macro", + "static_assertions", ] [[package]] name = "ouroboros_macro" -version = "0.15.6" +version = "0.17.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f7d21ccd03305a674437ee1248f3ab5d4b1db095cf1caf49f1713ddf61956b7" +checksum = "ec4c6225c69b4ca778c0aea097321a64c421cf4577b331c61b229267edabb6f8" dependencies = [ - "Inflector", + "heck 0.4.1", "proc-macro-error", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.37", ] [[package]] @@ -5064,7 +5127,7 @@ dependencies = [ "fuzzy", "gpui", "language", - "ordered-float", + "ordered-float 2.10.0", "picker", "postage", "settings", @@ -5230,6 +5293,15 @@ dependencies = [ "regex", ] +[[package]] +name = "pem-rfc7468" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88b39c9bfcfc231068454382784bb460aae594343fb030d46e9f50a645418412" +dependencies = [ + "base64ct", +] + [[package]] name = "percent-encoding" version = "2.3.0" @@ -5307,6 +5379,27 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pkcs1" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" +dependencies = [ + "der", + "pkcs8", + "spki", +] + +[[package]] +name = "pkcs8" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" +dependencies = [ + "der", + "spki", +] + [[package]] name = "pkg-config" version = "0.3.27" @@ -5598,7 +5691,7 @@ dependencies = [ "gpui", "language", "lsp", - "ordered-float", + "ordered-float 2.10.0", "picker", "postage", "project", @@ -5949,7 +6042,7 @@ dependencies = [ "fuzzy", "gpui", "language", - "ordered-float", + "ordered-float 2.10.0", "picker", "postage", "settings", @@ -6262,7 +6355,7 @@ dependencies = [ "prost 0.8.0", "prost-build", "rand 0.8.5", - "rsa", + "rsa 0.4.0", "serde", "serde_derive", "smol", @@ -6275,14 +6368,14 @@ dependencies = [ [[package]] name = "rsa" -version = "0.4.1" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b0aeddcca1082112a6eeb43bf25fd7820b066aaf6eaef776e19d0a1febe38fe" +checksum = "68ef841a26fc5d040ced0417c6c6a64ee851f42489df11cdf0218e545b6f8d28" dependencies = [ "byteorder", "digest 0.9.0", "lazy_static", - "num-bigint-dig", + "num-bigint-dig 0.7.1", "num-integer", "num-iter", "num-traits", @@ -6293,18 +6386,39 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rsa" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ab43bb47d23c1a631b4b680199a45255dce26fa9ab2fa902581f624ff13e6a8" +dependencies = [ + "byteorder", + "const-oid", + "digest 0.10.7", + "num-bigint-dig 0.8.4", + "num-integer", + "num-iter", + "num-traits", + "pkcs1", + "pkcs8", + "rand_core 0.6.4", + "signature", + "spki", + "subtle", + "zeroize", +] + [[package]] name = "rusqlite" -version = "0.27.0" +version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85127183a999f7db96d1a976a309eebbfb6ea3b0b400ddd8340190129de6eb7a" +checksum = "549b9d036d571d42e6e85d1c1425e2ac83491075078ca9a15be021c56b1641f2" dependencies = [ - "bitflags 1.3.2", + "bitflags 2.4.0", "fallible-iterator", "fallible-streaming-iterator", - "hashlink 0.7.0", + "hashlink", "libsqlite3-sys", - "memchr", "smallvec", ] @@ -6433,19 +6547,18 @@ dependencies = [ "log", "ring", "sct 0.6.1", - "webpki 0.21.4", + "webpki", ] [[package]] name = "rustls" -version = "0.20.9" +version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b80e3dec595989ea8510028f30c408a4630db12c9cbb8de34203b89d6577e99" +checksum = "cd8d6c9f025a446bc4d18ad9632e69aec8f287aa84499ee335599fabd20c3fd8" dependencies = [ - "log", "ring", + "rustls-webpki", "sct 0.7.0", - "webpki 0.22.1", ] [[package]] @@ -6457,6 +6570,16 @@ dependencies = [ "base64 0.21.4", ] +[[package]] +name = "rustls-webpki" +version = "0.101.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c7d5dece342910d9ba34d259310cae3e0154b873b35408b787b59bce53d34fe" +dependencies = [ + "ring", + "untrusted", +] + [[package]] name = "rustversion" version = "1.0.14" @@ -6597,26 +6720,40 @@ dependencies = [ "untrusted", ] +[[package]] +name = "sea-bae" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3bd3534a9978d0aa7edd2808dc1f8f31c4d0ecd31ddf71d997b3c98e9f3c9114" +dependencies = [ + "heck 0.4.1", + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.37", +] + [[package]] name = "sea-orm" -version = "0.10.5" -source = "git+https://github.com/zed-industries/sea-orm?rev=18f4c691085712ad014a51792af75a9044bacee6#18f4c691085712ad014a51792af75a9044bacee6" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da5b2d70c255bc5cbe1d49f69c3c8eadae0fbbaeb18ee978edbf2f75775cb94d" dependencies = [ "async-stream", "async-trait", + "bigdecimal", "chrono", "futures 0.3.28", - "futures-util", "log", "ouroboros", "rust_decimal", "sea-orm-macros", "sea-query", "sea-query-binder", - "sea-strum", "serde", "serde_json", "sqlx", + "strum", "thiserror", "time", "tracing", @@ -6626,25 +6763,30 @@ dependencies = [ [[package]] name = "sea-orm-macros" -version = "0.10.5" -source = "git+https://github.com/zed-industries/sea-orm?rev=18f4c691085712ad014a51792af75a9044bacee6#18f4c691085712ad014a51792af75a9044bacee6" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c8d455fad40194fb9774fdc4810c0f2700ff0dc0e93bd5ce9d641cc3f5dd75" dependencies = [ - "bae", - "heck 0.3.3", + "heck 0.4.1", "proc-macro2", "quote", - "syn 1.0.109", + "sea-bae", + "syn 2.0.37", + "unicode-ident", ] [[package]] name = "sea-query" -version = "0.27.2" +version = "0.30.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4f0fc4d8e44e1d51c739a68d336252a18bc59553778075d5e32649be6ec92ed" +checksum = "fb3e6bba153bb198646c8762c48414942a38db27d142e44735a133cabddcc820" dependencies = [ + "bigdecimal", "chrono", + "derivative", + "inherent", + "ordered-float 3.9.1", "rust_decimal", - "sea-query-derive", "serde_json", "time", "uuid 1.4.1", @@ -6652,10 +6794,11 @@ dependencies = [ [[package]] name = "sea-query-binder" -version = "0.2.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c2585b89c985cfacfe0ec9fc9e7bb055b776c1a2581c4e3c6185af2b8bf8865" +checksum = "36bbb68df92e820e4d5aeb17b4acd5cc8b5d18b2c36a4dd6f4626aabfa7ab1b9" dependencies = [ + "bigdecimal", "chrono", "rust_decimal", "sea-query", @@ -6665,41 +6808,6 @@ dependencies = [ "uuid 1.4.1", ] -[[package]] -name = "sea-query-derive" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34cdc022b4f606353fe5dc85b09713a04e433323b70163e81513b141c6ae6eb5" -dependencies = [ - "heck 0.3.3", - "proc-macro2", - "quote", - "syn 1.0.109", - "thiserror", -] - -[[package]] -name = "sea-strum" -version = "0.23.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "391d06a6007842cfe79ac6f7f53911b76dfd69fc9a6769f1cf6569d12ce20e1b" -dependencies = [ - "sea-strum_macros", -] - -[[package]] -name = "sea-strum_macros" -version = "0.23.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69b4397b825df6ccf1e98bcdabef3bbcfc47ff5853983467850eeab878384f21" -dependencies = [ - "heck 0.3.3", - "proc-macro2", - "quote", - "rustversion", - "syn 1.0.109", -] - [[package]] name = "seahash" version = "4.1.0" @@ -6779,7 +6887,7 @@ dependencies = [ "log", "ndarray", "node_runtime", - "ordered-float", + "ordered-float 2.10.0", "parking_lot 0.11.2", "picker", "postage", @@ -7077,6 +7185,16 @@ dependencies = [ "libc", ] +[[package]] +name = "signature" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e1788eed21689f9cf370582dfc467ef36ed9c707f073528ddafa8d83e3b8500" +dependencies = [ + "digest 0.10.7", + "rand_core 0.6.4", +] + [[package]] name = "simdutf8" version = "0.1.4" @@ -7238,6 +7356,16 @@ dependencies = [ "lock_api", ] +[[package]] +name = "spki" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d1e996ef02c474957d681f1b05213dfb0abab947b446a62d37770b23500184a" +dependencies = [ + "base64ct", + "der", +] + [[package]] name = "spsc-buffer" version = "0.1.1" @@ -7284,104 +7412,219 @@ dependencies = [ [[package]] name = "sqlx" -version = "0.6.3" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8de3b03a925878ed54a954f621e64bf55a3c1bd29652d0d1a17830405350188" +checksum = "0e50c216e3624ec8e7ecd14c6a6a6370aad6ee5d8cfc3ab30b5162eeeef2ed33" dependencies = [ "sqlx-core", "sqlx-macros", + "sqlx-mysql", + "sqlx-postgres", + "sqlx-sqlite", ] [[package]] name = "sqlx-core" -version = "0.6.3" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa8241483a83a3f33aa5fff7e7d9def398ff9990b2752b6c6112b83c6d246029" +checksum = "8d6753e460c998bbd4cd8c6f0ed9a64346fcca0723d6e75e52fdc351c5d2169d" dependencies = [ - "ahash 0.7.6", + "ahash 0.8.3", "atoi", - "base64 0.13.1", - "bitflags 1.3.2", + "bigdecimal", "byteorder", "bytes 1.5.0", "chrono", "crc", "crossbeam-queue", - "dirs 4.0.0", "dotenvy", "either", "event-listener", - "flume", "futures-channel", "futures-core", - "futures-executor", "futures-intrusive", + "futures-io", "futures-util", - "hashlink 0.8.4", + "hashlink", "hex", - "hkdf", - "hmac 0.12.1", - "indexmap 1.9.3", - "itoa", - "libc", - "libsqlite3-sys", + "indexmap 2.0.0", "log", - "md-5", "memchr", - "num-bigint 0.4.4", "once_cell", "paste", "percent-encoding", - "rand 0.8.5", "rust_decimal", - "rustls 0.20.9", + "rustls 0.21.7", "rustls-pemfile", "serde", "serde_json", - "sha1", "sha2 0.10.7", "smallvec", "sqlformat", - "sqlx-rt", - "stringprep", "thiserror", "time", + "tokio", "tokio-stream", + "tracing", "url", "uuid 1.4.1", - "webpki-roots 0.22.6", - "whoami", + "webpki-roots 0.24.0", ] [[package]] name = "sqlx-macros" -version = "0.6.3" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a793bb3ba331ec8359c1853bd39eed32cdd7baaf22c35ccf5c92a7e8d1189ec" +dependencies = [ + "proc-macro2", + "quote", + "sqlx-core", + "sqlx-macros-core", + "syn 1.0.109", +] + +[[package]] +name = "sqlx-macros-core" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9966e64ae989e7e575b19d7265cb79d7fc3cbbdf179835cb0d716f294c2049c9" +checksum = "0a4ee1e104e00dedb6aa5ffdd1343107b0a4702e862a84320ee7cc74782d96fc" dependencies = [ "dotenvy", "either", "heck 0.4.1", + "hex", "once_cell", "proc-macro2", "quote", + "serde", "serde_json", "sha2 0.10.7", "sqlx-core", - "sqlx-rt", + "sqlx-mysql", + "sqlx-postgres", + "sqlx-sqlite", "syn 1.0.109", + "tempfile", + "tokio", "url", ] [[package]] -name = "sqlx-rt" -version = "0.6.3" +name = "sqlx-mysql" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "804d3f245f894e61b1e6263c84b23ca675d96753b5abfd5cc8597d86806e8024" +checksum = "864b869fdf56263f4c95c45483191ea0af340f9f3e3e7b4d57a61c7c87a970db" dependencies = [ + "atoi", + "base64 0.21.4", + "bigdecimal", + "bitflags 2.4.0", + "byteorder", + "bytes 1.5.0", + "chrono", + "crc", + "digest 0.10.7", + "dotenvy", + "either", + "futures-channel", + "futures-core", + "futures-io", + "futures-util", + "generic-array", + "hex", + "hkdf", + "hmac 0.12.1", + "itoa", + "log", + "md-5", + "memchr", "once_cell", - "tokio", - "tokio-rustls", + "percent-encoding", + "rand 0.8.5", + "rsa 0.9.2", + "rust_decimal", + "serde", + "sha1", + "sha2 0.10.7", + "smallvec", + "sqlx-core", + "stringprep", + "thiserror", + "time", + "tracing", + "uuid 1.4.1", + "whoami", +] + +[[package]] +name = "sqlx-postgres" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb7ae0e6a97fb3ba33b23ac2671a5ce6e3cabe003f451abd5a56e7951d975624" +dependencies = [ + "atoi", + "base64 0.21.4", + "bigdecimal", + "bitflags 2.4.0", + "byteorder", + "chrono", + "crc", + "dotenvy", + "etcetera", + "futures-channel", + "futures-core", + "futures-io", + "futures-util", + "hex", + "hkdf", + "hmac 0.12.1", + "home", + "itoa", + "log", + "md-5", + "memchr", + "num-bigint 0.4.4", + "once_cell", + "rand 0.8.5", + "rust_decimal", + "serde", + "serde_json", + "sha1", + "sha2 0.10.7", + "smallvec", + "sqlx-core", + "stringprep", + "thiserror", + "time", + "tracing", + "uuid 1.4.1", + "whoami", +] + +[[package]] +name = "sqlx-sqlite" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d59dc83cf45d89c555a577694534fcd1b55c545a816c816ce51f20bbe56a4f3f" +dependencies = [ + "atoi", + "chrono", + "flume", + "futures-channel", + "futures-core", + "futures-executor", + "futures-intrusive", + "futures-util", + "libsqlite3-sys", + "log", + "percent-encoding", + "serde", + "sqlx-core", + "time", + "tracing", + "url", + "uuid 1.4.1", ] [[package]] @@ -7710,7 +7953,7 @@ dependencies = [ "lazy_static", "libc", "mio-extras", - "ordered-float", + "ordered-float 2.10.0", "procinfo", "rand 0.8.5", "schemars", @@ -7742,7 +7985,7 @@ dependencies = [ "lazy_static", "libc", "mio-extras", - "ordered-float", + "ordered-float 2.10.0", "procinfo", "project", "rand 0.8.5", @@ -8035,17 +8278,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "tokio-rustls" -version = "0.23.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c43ee83903113e03984cb9e5cebe6c04a5116269e900e3ddba8f068a62adda59" -dependencies = [ - "rustls 0.20.9", - "tokio", - "webpki 0.22.1", -] - [[package]] name = "tokio-stream" version = "0.1.14" @@ -9363,32 +9595,22 @@ dependencies = [ "untrusted", ] -[[package]] -name = "webpki" -version = "0.22.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0e74f82d49d545ad128049b7e88f6576df2da6b02e9ce565c6f533be576957e" -dependencies = [ - "ring", - "untrusted", -] - [[package]] name = "webpki-roots" version = "0.21.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aabe153544e473b775453675851ecc86863d2a81d786d741f6b76778f2a48940" dependencies = [ - "webpki 0.21.4", + "webpki", ] [[package]] name = "webpki-roots" -version = "0.22.6" +version = "0.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6c71e40d7d2c34a5106301fb632274ca37242cd0c9d3e64dbece371a40a2d87" +checksum = "b291546d5d9d1eab74f069c77749f2cb8504a12caa20f0f2de93ddbf6f411888" dependencies = [ - "webpki 0.22.1", + "rustls-webpki", ] [[package]] @@ -9438,10 +9660,6 @@ name = "whoami" version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22fc3756b8a9133049b26c7f61ab35416c130e8c09b660f5b3958b446f52cc50" -dependencies = [ - "wasm-bindgen", - "web-sys", -] [[package]] name = "wiggle" @@ -9907,7 +10125,7 @@ dependencies = [ "recent_projects", "regex", "rpc", - "rsa", + "rsa 0.4.0", "rust-embed", "schemars", "search", @@ -9976,9 +10194,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.3.0" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" +checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" dependencies = [ "zeroize_derive", ] diff --git a/Cargo.toml b/Cargo.toml index f09d44e8da0e3c7894509ee4db09c836f935708e..37322253124d6c8e2885d6d637ea197f4cec8b3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,6 +105,7 @@ rand = { version = "0.8.5" } refineable = { path = "./crates/refineable" } regex = { version = "1.5" } rust-embed = { version = "8.0", features = ["include-exclude"] } +rusqlite = { version = "0.29.0", features = ["blob", "array", "modern_sqlite"] } schemars = { version = "0.8" } serde = { version = "1.0", features = ["derive", "rc"] } serde_derive = { version = "1.0", features = ["deserialize_in_place"] } diff --git a/crates/ai/Cargo.toml b/crates/ai/Cargo.toml index a2c70ce8c6f99b70d4992c7bb26fa3c79c7f2ad5..542d7f422fe8c1eaec7d10bf59cb5ccaa2d65ca3 100644 --- a/crates/ai/Cargo.toml +++ b/crates/ai/Cargo.toml @@ -27,7 +27,7 @@ log.workspace = true parse_duration = "2.1.1" tiktoken-rs = "0.5.0" matrixmultiply = "0.3.7" -rusqlite = { version = "0.27.0", features = ["blob", "array", "modern_sqlite"] } +rusqlite = { version = "0.29.0", features = ["blob", "array", "modern_sqlite"] } bincode = "1.3.3" [dev-dependencies] diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 6cd29b6e54e86bb067bc08d16ba6e461c76852bf..6af23223c23b1b42684f0305fdc25ce6c59e1511 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -42,14 +42,12 @@ rand.workspace = true reqwest = { version = "0.11", features = ["json"], optional = true } scrypt = "0.7" smallvec.workspace = true -# Remove fork dependency when a version with https://github.com/SeaQL/sea-orm/pull/1283 is released. -sea-orm = { git = "https://github.com/zed-industries/sea-orm", rev = "18f4c691085712ad014a51792af75a9044bacee6", features = ["sqlx-postgres", "postgres-array", "runtime-tokio-rustls"] } -sea-query = "0.27" +sea-orm = { version = "0.12.x", features = ["sqlx-postgres", "postgres-array", "runtime-tokio-rustls", "with-uuid"] } serde.workspace = true serde_derive.workspace = true serde_json.workspace = true sha-1 = "0.9" -sqlx = { version = "0.6", features = ["runtime-tokio-rustls", "postgres", "json", "time", "uuid", "any"] } +sqlx = { version = "0.7", features = ["runtime-tokio-rustls", "postgres", "json", "time", "uuid", "any"] } time.workspace = true tokio = { version = "1", features = ["full"] } tokio-tungstenite = "0.17" @@ -87,9 +85,9 @@ env_logger.workspace = true indoc.workspace = true util = { path = "../util" } lazy_static.workspace = true -sea-orm = { git = "https://github.com/zed-industries/sea-orm", rev = "18f4c691085712ad014a51792af75a9044bacee6", features = ["sqlx-sqlite"] } +sea-orm = { version = "0.12.x", features = ["sqlx-sqlite"] } serde_json.workspace = true -sqlx = { version = "0.6", features = ["sqlite"] } +sqlx = { version = "0.7", features = ["sqlite"] } unindent.workspace = true [features] diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index b0223bbf27523d384902ef13e67e5df0d9023f5d..13bb3c06e821dd49f2902a280375996715f8513b 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -19,11 +19,12 @@ use rpc::{ ConnectionId, }; use sea_orm::{ - entity::prelude::*, ActiveValue, Condition, ConnectionTrait, DatabaseConnection, - DatabaseTransaction, DbErr, FromQueryResult, IntoActiveModel, IsolationLevel, JoinType, - QueryOrder, QuerySelect, Statement, TransactionTrait, + entity::prelude::*, + sea_query::{Alias, Expr, OnConflict, Query}, + ActiveValue, Condition, ConnectionTrait, DatabaseConnection, DatabaseTransaction, DbErr, + FromQueryResult, IntoActiveModel, IsolationLevel, JoinType, QueryOrder, QuerySelect, Statement, + TransactionTrait, }; -use sea_query::{Alias, Expr, OnConflict, Query}; use serde::{Deserialize, Serialize}; use sqlx::{ migrate::{Migrate, Migration, MigrationSource}, @@ -62,6 +63,7 @@ pub struct Database { // separate files in the `queries` folder. impl Database { pub async fn new(options: ConnectOptions, executor: Executor) -> Result { + sqlx::any::install_default_drivers(); Ok(Self { options: options.clone(), pool: sea_orm::Database::connect(options).await?, diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 865a39fd7154f6aa9ac993a5c7a4b53f8a302c6a..23bb9e53bf9803ba64693f94605a8e87f904c571 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -1,6 +1,5 @@ use crate::Result; -use sea_orm::DbErr; -use sea_query::{Value, ValueTypeErr}; +use sea_orm::{entity::prelude::*, DbErr}; use serde::{Deserialize, Serialize}; macro_rules! id_type { @@ -17,6 +16,7 @@ macro_rules! id_type { Hash, Serialize, Deserialize, + DeriveValueType, )] #[serde(transparent)] pub struct $name(pub i32); @@ -42,40 +42,6 @@ macro_rules! id_type { } } - impl From<$name> for sea_query::Value { - fn from(value: $name) -> Self { - sea_query::Value::Int(Some(value.0)) - } - } - - impl sea_orm::TryGetable for $name { - fn try_get( - res: &sea_orm::QueryResult, - pre: &str, - col: &str, - ) -> Result { - Ok(Self(i32::try_get(res, pre, col)?)) - } - } - - impl sea_query::ValueType for $name { - fn try_from(v: Value) -> Result { - Ok(Self(value_to_integer(v)?)) - } - - fn type_name() -> String { - stringify!($name).into() - } - - fn array_type() -> sea_query::ArrayType { - sea_query::ArrayType::Int - } - - fn column_type() -> sea_query::ColumnType { - sea_query::ColumnType::Integer(None) - } - } - impl sea_orm::TryFromU64 for $name { fn try_from_u64(n: u64) -> Result { Ok(Self(n.try_into().map_err(|_| { @@ -88,7 +54,7 @@ macro_rules! id_type { } } - impl sea_query::Nullable for $name { + impl sea_orm::sea_query::Nullable for $name { fn null() -> Value { Value::Int(None) } @@ -96,20 +62,6 @@ macro_rules! id_type { }; } -fn value_to_integer(v: Value) -> Result { - match v { - Value::TinyInt(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::SmallInt(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::Int(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::BigInt(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::TinyUnsigned(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::SmallUnsigned(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::Unsigned(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - Value::BigUnsigned(Some(int)) => int.try_into().map_err(|_| ValueTypeErr), - _ => Err(ValueTypeErr), - } -} - id_type!(BufferId); id_type!(AccessTokenId); id_type!(ChannelChatParticipantId); diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 8292e9dbcbc06b2e19895d670c2b8066eef71d29..207cca765783c2f9df5f7824964404fefc042518 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -1,8 +1,7 @@ +use super::*; use rpc::proto::ChannelEdge; use smallvec::SmallVec; -use super::*; - type ChannelDescendants = HashMap>; impl Database { @@ -659,7 +658,7 @@ impl Database { ) -> Result> { let paths = channel_path::Entity::find() .filter(channel_path::Column::ChannelId.eq(channel_id)) - .order_by(channel_path::Column::IdPath, sea_query::Order::Desc) + .order_by(channel_path::Column::IdPath, sea_orm::Order::Desc) .all(tx) .await?; let mut channel_ids = Vec::new(); diff --git a/crates/collab/src/db/queries/contacts.rs b/crates/collab/src/db/queries/contacts.rs index a18958f035515450be2f9448b65c5f00dc6368fd..2171f1a6bf87354b8017fd35b47d77bba1e25af0 100644 --- a/crates/collab/src/db/queries/contacts.rs +++ b/crates/collab/src/db/queries/contacts.rs @@ -18,12 +18,12 @@ impl Database { let user_b_participant = Alias::new("user_b_participant"); let mut db_contacts = contact::Entity::find() .column_as( - Expr::tbl(user_a_participant.clone(), room_participant::Column::Id) + Expr::col((user_a_participant.clone(), room_participant::Column::Id)) .is_not_null(), "user_a_busy", ) .column_as( - Expr::tbl(user_b_participant.clone(), room_participant::Column::Id) + Expr::col((user_b_participant.clone(), room_participant::Column::Id)) .is_not_null(), "user_b_busy", ) diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index db968ba8958dd3685e629f32bf8d5076dfd0eef1..27e64e25981ecdbd31e6aa337875e1ff81852b9c 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -184,7 +184,7 @@ impl Database { Ok(user::Entity::find() .from_raw_sql(Statement::from_sql_and_values( self.pool.get_database_backend(), - query.into(), + query, vec![like_string.into(), name_query.into(), limit.into()], )) .all(&*tx) diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index cf12be9b8d3beafb7332f20ee10f81ad65ad6a98..75584ff90b68cf4fea0b4151a835c77e970daae5 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -39,7 +39,7 @@ impl TestDb { db.pool .execute(sea_orm::Statement::from_string( db.pool.get_database_backend(), - sql.into(), + sql, )) .await .unwrap(); @@ -134,7 +134,7 @@ impl Drop for TestDb { db.pool .execute(sea_orm::Statement::from_string( db.pool.get_database_backend(), - query.into(), + query, )) .await .log_err(); diff --git a/crates/semantic_index/Cargo.toml b/crates/semantic_index/Cargo.toml index efda311633cad4b416adcea4b4f77c7a3e71c72d..34850f7035c15aba1e7a1ba1c54bc6b1431a5c9d 100644 --- a/crates/semantic_index/Cargo.toml +++ b/crates/semantic_index/Cargo.toml @@ -26,7 +26,7 @@ postage.workspace = true futures.workspace = true ordered-float.workspace = true smol.workspace = true -rusqlite = { version = "0.27.0", features = ["blob", "array", "modern_sqlite"] } +rusqlite.workspace = true log.workspace = true tree-sitter.workspace = true lazy_static.workspace = true diff --git a/crates/sqlez/Cargo.toml b/crates/sqlez/Cargo.toml index 01d17d48123f181b5913029102aa3215f75fafbb..6811ee054c1fc90e9aebf03a07d29e894bec9875 100644 --- a/crates/sqlez/Cargo.toml +++ b/crates/sqlez/Cargo.toml @@ -7,7 +7,7 @@ publish = false [dependencies] anyhow.workspace = true indoc.workspace = true -libsqlite3-sys = { version = "0.24", features = ["bundled"] } +libsqlite3-sys = { version = "0.26", features = ["bundled"] } smol.workspace = true thread_local = "1.1.4" lazy_static.workspace = true From af09861f5ccd59956e66d21452d23629fbf9c075 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 3 Oct 2023 17:39:24 -0700 Subject: [PATCH 12/20] Specify uuid crate in the root Cargo.toml Co-authored-by: Mikayla --- Cargo.toml | 1 + crates/assistant/Cargo.toml | 2 +- crates/channel/Cargo.toml | 3 ++- crates/client/Cargo.toml | 2 +- crates/collab/Cargo.toml | 1 + crates/gpui/Cargo.toml | 2 +- crates/sqlez/Cargo.toml | 2 +- crates/workspace/Cargo.toml | 2 +- crates/zed/Cargo.toml | 2 +- 9 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 37322253124d6c8e2885d6d637ea197f4cec8b3f..a634b7f67b4ab22b79353a39d2c7d8fcbed55678 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -120,6 +120,7 @@ tree-sitter = "0.20" unindent = { version = "0.1.7" } pretty_assertions = "1.3.0" git2 = { version = "0.15", default-features = false} +uuid = { version = "1.1.2", features = ["v4"] } tree-sitter-bash = { git = "https://github.com/tree-sitter/tree-sitter-bash", rev = "1b0321ee85701d5036c334a6f04761cdc672e64c" } tree-sitter-c = "0.20.1" diff --git a/crates/assistant/Cargo.toml b/crates/assistant/Cargo.toml index 5d141b32d5a448c43a9db96f0879461709963697..f1daf47bab05c96acc0876b1aff9a2871d5c416a 100644 --- a/crates/assistant/Cargo.toml +++ b/crates/assistant/Cargo.toml @@ -21,8 +21,8 @@ search = { path = "../search" } settings = { path = "../settings" } theme = { path = "../theme" } util = { path = "../util" } -uuid = { version = "1.1.2", features = ["v4"] } workspace = { path = "../workspace" } +uuid.workspace = true anyhow.workspace = true chrono = { version = "0.4", features = ["serde"] } diff --git a/crates/channel/Cargo.toml b/crates/channel/Cargo.toml index 16a1d418d5c17750db582252118c3ddcc1cad2d3..6bd177bed573bc8a034dbb50588c77b111422242 100644 --- a/crates/channel/Cargo.toml +++ b/crates/channel/Cargo.toml @@ -23,6 +23,7 @@ language = { path = "../language" } settings = { path = "../settings" } feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } +clock = { path = "../clock" } anyhow.workspace = true futures.workspace = true @@ -38,7 +39,7 @@ smol.workspace = true thiserror.workspace = true time.workspace = true tiny_http = "0.8" -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true url = "2.2" serde.workspace = true serde_derive.workspace = true diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index e3038e5bcc49bd41b756062b676e00f4f355867a..d3229170133bc63886b912b21219e123483dc9a7 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -37,7 +37,7 @@ smol.workspace = true thiserror.workspace = true time.workspace = true tiny_http = "0.8" -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true url = "2.2" serde.workspace = true serde_derive.workspace = true diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 6af23223c23b1b42684f0305fdc25ce6c59e1511..ecc4b57b120f2bbd1b0e0ab8da8284034595d5ae 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -57,6 +57,7 @@ toml.workspace = true tracing = "0.1.34" tracing-log = "0.1.3" tracing-subscriber = { version = "0.3.11", features = ["env-filter", "json"] } +uuid.workspace = true [dev-dependencies] audio = { path = "../audio" } diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index 95b7ccb559980e746eeaaa3a5eb07b73166997b4..cdd5a0a5c88d42d3defa9cc1e5c27f223f257a52 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -53,7 +53,7 @@ thiserror.workspace = true time.workspace = true tiny-skia = "0.5" usvg = { version = "0.14", features = [] } -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true waker-fn = "1.1.0" [build-dependencies] diff --git a/crates/sqlez/Cargo.toml b/crates/sqlez/Cargo.toml index 6811ee054c1fc90e9aebf03a07d29e894bec9875..1d1d052e93f1dd436da02e8a149c47413a4295d8 100644 --- a/crates/sqlez/Cargo.toml +++ b/crates/sqlez/Cargo.toml @@ -13,4 +13,4 @@ thread_local = "1.1.4" lazy_static.workspace = true parking_lot.workspace = true futures.workspace = true -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index e2dae07b8c9dc9c75ac60c32499a7884834b75f4..41c86e538de466825115359d8bffcec816c06d8c 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -51,7 +51,7 @@ serde.workspace = true serde_derive.workspace = true serde_json.workspace = true smallvec.workspace = true -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true [dev-dependencies] call = { path = "../call", features = ["test-support"] } diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index fde598a735024c599626ff31c2c6d2a8397f61d8..bd6a85e3aaedc9ae1cc6fc8380a13227058427dd 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -138,7 +138,7 @@ tree-sitter-nu.workspace = true url = "2.2" urlencoding = "2.1.2" -uuid = { version = "1.1.2", features = ["v4"] } +uuid.workspace = true [dev-dependencies] call = { path = "../call", features = ["test-support"] } From 61e0289014732bef59e306c5f0e027c47c1d967a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 3 Oct 2023 17:40:10 -0700 Subject: [PATCH 13/20] Acknowledge channel notes and chat changes when views are active Co-authored-by: Mikayla --- Cargo.lock | 2 + crates/channel/src/channel_buffer.rs | 57 +++++++++--- crates/channel/src/channel_chat.rs | 27 +++++- crates/channel/src/channel_store.rs | 93 ++++++++++++------- .../src/channel_store/channel_index.rs | 62 ++++++++++--- crates/client/src/client.rs | 28 +++--- crates/collab/src/db.rs | 4 +- crates/collab/src/db/queries/buffers.rs | 75 +++++++++------ crates/collab/src/db/queries/channels.rs | 12 +-- crates/collab/src/db/queries/messages.rs | 13 ++- crates/collab/src/db/tests/buffer_tests.rs | 79 +++++++++++----- crates/collab/src/db/tests/message_tests.rs | 65 ++++++++----- crates/collab/src/rpc.rs | 58 ++++++------ .../collab/src/tests/channel_buffer_tests.rs | 4 +- crates/collab/src/tests/test_server.rs | 4 +- crates/collab_ui/src/channel_view.rs | 52 ++++++++++- crates/collab_ui/src/chat_panel.rs | 30 ++++-- crates/collab_ui/src/collab_panel.rs | 4 +- crates/rpc/proto/zed.proto | 18 +++- 19 files changed, 478 insertions(+), 209 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ffdc7368309aa09cf0101a6cf2bb1799054033bb..18950cdb82acac495dcc5c4b97d1fcd028de4050 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,6 +1217,7 @@ version = "0.1.0" dependencies = [ "anyhow", "client", + "clock", "collections", "db", "feature_flags", @@ -1530,6 +1531,7 @@ dependencies = [ "tracing-subscriber", "unindent", "util", + "uuid 1.4.1", "workspace", ] diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index a03eb1f1b549772e9efe50c0ca160b916453eefc..a097cc5467303e2f47f58dc892345c9d24fd5d20 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -2,14 +2,17 @@ use crate::Channel; use anyhow::Result; use client::{Client, Collaborator, UserStore}; use collections::HashMap; -use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle}; +use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task}; +use language::proto::serialize_version; use rpc::{ proto::{self, PeerId}, TypedEnvelope, }; -use std::sync::Arc; +use std::{sync::Arc, time::Duration}; use util::ResultExt; +const ACKNOWLEDGE_DEBOUNCE_INTERVAL: Duration = Duration::from_millis(250); + pub(crate) fn init(client: &Arc) { client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer); client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer_collaborators); @@ -24,11 +27,13 @@ pub struct ChannelBuffer { buffer_epoch: u64, client: Arc, subscription: Option, + acknowledge_task: Option>>, } pub enum ChannelBufferEvent { CollaboratorsChanged, Disconnected, + BufferEdited, } impl Entity for ChannelBuffer { @@ -36,6 +41,9 @@ impl Entity for ChannelBuffer { fn release(&mut self, _: &mut AppContext) { if self.connected { + if let Some(task) = self.acknowledge_task.take() { + task.detach(); + } self.client .send(proto::LeaveChannelBuffer { channel_id: self.channel.id, @@ -81,6 +89,7 @@ impl ChannelBuffer { client, connected: true, collaborators: Default::default(), + acknowledge_task: None, channel, subscription: Some(subscription.set_model(&cx.handle(), &mut cx.to_async())), user_store, @@ -159,19 +168,45 @@ impl ChannelBuffer { &mut self, _: ModelHandle, event: &language::Event, - _: &mut ModelContext, + cx: &mut ModelContext, ) { - if let language::Event::Operation(operation) = event { - let operation = language::proto::serialize_operation(operation); - self.client - .send(proto::UpdateChannelBuffer { - channel_id: self.channel.id, - operations: vec![operation], - }) - .log_err(); + match event { + language::Event::Operation(operation) => { + let operation = language::proto::serialize_operation(operation); + self.client + .send(proto::UpdateChannelBuffer { + channel_id: self.channel.id, + operations: vec![operation], + }) + .log_err(); + } + language::Event::Edited => { + cx.emit(ChannelBufferEvent::BufferEdited); + } + _ => {} } } + pub fn acknowledge_buffer_version(&mut self, cx: &mut ModelContext<'_, ChannelBuffer>) { + let buffer = self.buffer.read(cx); + let version = buffer.version(); + let buffer_id = buffer.remote_id(); + let client = self.client.clone(); + let epoch = self.epoch(); + + self.acknowledge_task = Some(cx.spawn_weak(|_, cx| async move { + cx.background().timer(ACKNOWLEDGE_DEBOUNCE_INTERVAL).await; + client + .send(proto::AckBufferOperation { + buffer_id, + epoch, + version: serialize_version(&version), + }) + .ok(); + Ok(()) + })); + } + pub fn epoch(&self) -> u64 { self.buffer_epoch } diff --git a/crates/channel/src/channel_chat.rs b/crates/channel/src/channel_chat.rs index 8e03a3b6fd0f1d2d56ccb648525f268766a9eaf1..5b32d67f63b0bb12450a75856b5d7f57f931a9a9 100644 --- a/crates/channel/src/channel_chat.rs +++ b/crates/channel/src/channel_chat.rs @@ -1,4 +1,4 @@ -use crate::Channel; +use crate::{Channel, ChannelStore}; use anyhow::{anyhow, Result}; use client::{ proto, @@ -16,7 +16,9 @@ use util::{post_inc, ResultExt as _, TryFutureExt}; pub struct ChannelChat { channel: Arc, messages: SumTree, + channel_store: ModelHandle, loaded_all_messages: bool, + last_acknowledged_id: Option, next_pending_message_id: usize, user_store: ModelHandle, rpc: Arc, @@ -77,6 +79,7 @@ impl Entity for ChannelChat { impl ChannelChat { pub async fn new( channel: Arc, + channel_store: ModelHandle, user_store: ModelHandle, client: Arc, mut cx: AsyncAppContext, @@ -94,11 +97,13 @@ impl ChannelChat { let mut this = Self { channel, user_store, + channel_store, rpc: client, outgoing_messages_lock: Default::default(), messages: Default::default(), loaded_all_messages, next_pending_message_id: 0, + last_acknowledged_id: None, rng: StdRng::from_entropy(), _subscription: subscription.set_model(&cx.handle(), &mut cx.to_async()), }; @@ -219,6 +224,26 @@ impl ChannelChat { false } + pub fn acknowledge_last_message(&mut self, cx: &mut ModelContext) { + if let ChannelMessageId::Saved(latest_message_id) = self.messages.summary().max_id { + if self + .last_acknowledged_id + .map_or(true, |acknowledged_id| acknowledged_id < latest_message_id) + { + self.rpc + .send(proto::AckChannelMessage { + channel_id: self.channel.id, + message_id: latest_message_id, + }) + .ok(); + self.last_acknowledged_id = Some(latest_message_id); + self.channel_store.update(cx, |store, cx| { + store.acknowledge_message_id(self.channel.id, latest_message_id, cx); + }); + } + } + } + pub fn rejoin(&mut self, cx: &mut ModelContext) { let user_store = self.user_store.clone(); let rpc = self.rpc.clone(); diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index f0f66f4839ea6ec19d042c7cce3caaf0994c28bb..db6d5f42c5309e01c33469aa581faa142377f2bb 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -43,8 +43,8 @@ pub type ChannelData = (Channel, ChannelPath); pub struct Channel { pub id: ChannelId, pub name: String, - pub has_note_changed: bool, - pub has_new_messages: bool, + pub unseen_note_version: Option<(u64, clock::Global)>, + pub unseen_message_id: Option, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] @@ -201,34 +201,60 @@ impl ChannelStore { ) -> Task>> { let client = self.client.clone(); let user_store = self.user_store.clone(); - let open_channel_buffer = self.open_channel_resource( + self.open_channel_resource( channel_id, |this| &mut this.opened_buffers, |channel, cx| ChannelBuffer::new(channel, client, user_store, cx), cx, - ); - cx.spawn(|this, mut cx| async move { - let buffer = open_channel_buffer.await?; - this.update(&mut cx, |this, cx| { - this.channel_index.clear_note_changed(channel_id); - cx.notify(); - }); - Ok(buffer) - }) + ) } pub fn has_channel_buffer_changed(&self, channel_id: ChannelId) -> Option { self.channel_index .by_id() .get(&channel_id) - .map(|channel| channel.has_note_changed) + .map(|channel| channel.unseen_note_version.is_some()) } pub fn has_new_messages(&self, channel_id: ChannelId) -> Option { self.channel_index .by_id() .get(&channel_id) - .map(|channel| channel.has_new_messages) + .map(|channel| channel.unseen_message_id.is_some()) + } + + pub fn notes_changed( + &mut self, + channel_id: ChannelId, + epoch: u64, + version: &clock::Global, + cx: &mut ModelContext, + ) { + self.channel_index.note_changed(channel_id, epoch, version); + cx.notify(); + } + + pub fn acknowledge_message_id( + &mut self, + channel_id: ChannelId, + message_id: u64, + cx: &mut ModelContext, + ) { + self.channel_index + .acknowledge_message_id(channel_id, message_id); + cx.notify(); + } + + pub fn acknowledge_notes_version( + &mut self, + channel_id: ChannelId, + epoch: u64, + version: &clock::Global, + cx: &mut ModelContext, + ) { + self.channel_index + .acknowledge_note_version(channel_id, epoch, version); + cx.notify(); } pub fn open_channel_chat( @@ -238,20 +264,13 @@ impl ChannelStore { ) -> Task>> { let client = self.client.clone(); let user_store = self.user_store.clone(); - let open_channel_chat = self.open_channel_resource( + let this = cx.handle(); + self.open_channel_resource( channel_id, |this| &mut this.opened_chats, - |channel, cx| ChannelChat::new(channel, user_store, client, cx), + |channel, cx| ChannelChat::new(channel, this, user_store, client, cx), cx, - ); - cx.spawn(|this, mut cx| async move { - let chat = open_channel_chat.await?; - this.update(&mut cx, |this, cx| { - this.channel_index.clear_message_changed(channel_id); - cx.notify(); - }); - Ok(chat) - }) + ) } /// Asynchronously open a given resource associated with a channel. @@ -811,8 +830,8 @@ impl ChannelStore { Arc::new(Channel { id: channel.id, name: channel.name, - has_note_changed: false, - has_new_messages: false, + unseen_note_version: None, + unseen_message_id: None, }), ), } @@ -822,8 +841,8 @@ impl ChannelStore { || !payload.delete_channels.is_empty() || !payload.insert_edge.is_empty() || !payload.delete_edge.is_empty() - || !payload.notes_changed.is_empty() - || !payload.new_messages.is_empty(); + || !payload.unseen_channel_messages.is_empty() + || !payload.unseen_channel_buffer_changes.is_empty(); if channels_changed { if !payload.delete_channels.is_empty() { @@ -850,12 +869,20 @@ impl ChannelStore { index.insert(channel) } - for id_changed in payload.notes_changed { - index.note_changed(id_changed); + for unseen_buffer_change in payload.unseen_channel_buffer_changes { + let version = language::proto::deserialize_version(&unseen_buffer_change.version); + index.note_changed( + unseen_buffer_change.channel_id, + unseen_buffer_change.epoch, + &version, + ); } - for id_changed in payload.new_messages { - index.new_messages(id_changed); + for unseen_channel_message in payload.unseen_channel_messages { + index.new_messages( + unseen_channel_message.channel_id, + unseen_channel_message.message_id, + ); } for edge in payload.insert_edge { diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 513e20e3a771185f0b8a517b6bc3c6186c9e132c..2a93ca6573cc4aa999f4d43d023aa9df47edb663 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -39,17 +39,38 @@ impl ChannelIndex { } } - pub fn clear_note_changed(&mut self, channel_id: ChannelId) { + pub fn acknowledge_note_version( + &mut self, + channel_id: ChannelId, + epoch: u64, + version: &clock::Global, + ) { if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - Arc::make_mut(channel).has_note_changed = false; + let channel = Arc::make_mut(channel); + if let Some((unseen_epoch, unseen_version)) = &channel.unseen_note_version { + if epoch > *unseen_epoch + || epoch == *unseen_epoch && version.observed_all(unseen_version) + { + channel.unseen_note_version = None; + } + } } } - pub fn clear_message_changed(&mut self, channel_id: ChannelId) { + pub fn acknowledge_message_id(&mut self, channel_id: ChannelId, message_id: u64) { if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - Arc::make_mut(channel).has_new_messages = false; + let channel = Arc::make_mut(channel); + if let Some(unseen_message_id) = channel.unseen_message_id { + if message_id >= unseen_message_id { + channel.unseen_message_id = None; + } + } } } + + pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) { + insert_note_changed(&mut self.channels_by_id, channel_id, epoch, version); + } } impl Deref for ChannelIndex { @@ -88,15 +109,14 @@ impl<'a> ChannelPathsInsertGuard<'a> { } } - pub fn note_changed(&mut self, channel_id: ChannelId) { - if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - Arc::make_mut(channel).has_note_changed = true; - } + pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) { + insert_note_changed(&mut self.channels_by_id, channel_id, epoch, &version); } - pub fn new_messages(&mut self, channel_id: ChannelId) { + pub fn new_messages(&mut self, channel_id: ChannelId, message_id: u64) { if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - Arc::make_mut(channel).has_new_messages = true; + let unseen_message_id = Arc::make_mut(channel).unseen_message_id.get_or_insert(0); + *unseen_message_id = message_id.max(*unseen_message_id); } } @@ -109,8 +129,8 @@ impl<'a> ChannelPathsInsertGuard<'a> { Arc::new(Channel { id: channel_proto.id, name: channel_proto.name, - has_note_changed: false, - has_new_messages: false, + unseen_note_version: None, + unseen_message_id: None, }), ); self.insert_root(channel_proto.id); @@ -186,3 +206,21 @@ fn channel_path_sorting_key<'a>( path.iter() .map(|id| Some(channels_by_id.get(id)?.name.as_str())) } + +fn insert_note_changed( + channels_by_id: &mut BTreeMap>, + channel_id: u64, + epoch: u64, + version: &clock::Global, +) { + if let Some(channel) = channels_by_id.get_mut(&channel_id) { + let unseen_version = Arc::make_mut(channel) + .unseen_note_version + .get_or_insert((0, clock::Global::new())); + if epoch > unseen_version.0 { + *unseen_version = (epoch, version.clone()); + } else { + unseen_version.1.join(&version); + } + } +} diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 5eae700404c316cdf55d44cccb24470ded092c93..5767ac54b7893f7425dfd56202b7512d17314f0f 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -34,7 +34,7 @@ use std::{ future::Future, marker::PhantomData, path::PathBuf, - sync::{Arc, Weak}, + sync::{atomic::AtomicU64, Arc, Weak}, time::{Duration, Instant}, }; use telemetry::Telemetry; @@ -105,7 +105,7 @@ pub fn init(client: &Arc, cx: &mut AppContext) { } pub struct Client { - id: usize, + id: AtomicU64, peer: Arc, http: Arc, telemetry: Arc, @@ -374,7 +374,7 @@ impl settings::Setting for TelemetrySettings { impl Client { pub fn new(http: Arc, cx: &AppContext) -> Arc { Arc::new(Self { - id: 0, + id: AtomicU64::new(0), peer: Peer::new(0), telemetry: Telemetry::new(http.clone(), cx), http, @@ -387,17 +387,16 @@ impl Client { }) } - pub fn id(&self) -> usize { - self.id + pub fn id(&self) -> u64 { + self.id.load(std::sync::atomic::Ordering::SeqCst) } pub fn http_client(&self) -> Arc { self.http.clone() } - #[cfg(any(test, feature = "test-support"))] - pub fn set_id(&mut self, id: usize) -> &Self { - self.id = id; + pub fn set_id(&self, id: u64) -> &Self { + self.id.store(id, std::sync::atomic::Ordering::SeqCst); self } @@ -454,7 +453,7 @@ impl Client { } fn set_status(self: &Arc, status: Status, cx: &AsyncAppContext) { - log::info!("set status on client {}: {:?}", self.id, status); + log::info!("set status on client {}: {:?}", self.id(), status); let mut state = self.state.write(); *state.status.0.borrow_mut() = status; @@ -805,6 +804,7 @@ impl Client { } } let credentials = credentials.unwrap(); + self.set_id(credentials.user_id); if was_disconnected { self.set_status(Status::Connecting, cx); @@ -1221,7 +1221,7 @@ impl Client { } pub fn send(&self, message: T) -> Result<()> { - log::debug!("rpc send. client_id:{}, name:{}", self.id, T::NAME); + log::debug!("rpc send. client_id:{}, name:{}", self.id(), T::NAME); self.peer.send(self.connection_id()?, message) } @@ -1237,7 +1237,7 @@ impl Client { &self, request: T, ) -> impl Future>> { - let client_id = self.id; + let client_id = self.id(); log::debug!( "rpc request start. client_id:{}. name:{}", client_id, @@ -1258,7 +1258,7 @@ impl Client { } fn respond(&self, receipt: Receipt, response: T::Response) -> Result<()> { - log::debug!("rpc respond. client_id:{}. name:{}", self.id, T::NAME); + log::debug!("rpc respond. client_id:{}. name:{}", self.id(), T::NAME); self.peer.respond(receipt, response) } @@ -1267,7 +1267,7 @@ impl Client { receipt: Receipt, error: proto::Error, ) -> Result<()> { - log::debug!("rpc respond. client_id:{}. name:{}", self.id, T::NAME); + log::debug!("rpc respond. client_id:{}. name:{}", self.id(), T::NAME); self.peer.respond_with_error(receipt, error) } @@ -1336,7 +1336,7 @@ impl Client { if let Some(handler) = handler { let future = handler(subscriber, message, &self, cx.clone()); - let client_id = self.id; + let client_id = self.id(); log::debug!( "rpc message received. client_id:{}, sender_id:{:?}, type:{}", client_id, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 13bb3c06e821dd49f2902a280375996715f8513b..e60b7cc33dfb22f030c5600cfa0d8d0794438c1d 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -439,8 +439,8 @@ pub struct ChannelsForUser { pub channels: ChannelGraph, pub channel_participants: HashMap>, pub channels_with_admin_privileges: HashSet, - pub channels_with_changed_notes: HashSet, - pub channels_with_new_messages: HashSet, + pub unseen_buffer_changes: Vec, + pub channel_messages: Vec, } #[derive(Debug)] diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 78ccd9e54af5d8c2f9266054e20e5f3872404803..c85432f2bba1b62a1e346626171fb59bfb8ef7ac 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -432,7 +432,12 @@ impl Database { channel_id: ChannelId, user: UserId, operations: &[proto::Operation], - ) -> Result<(Vec, Vec)> { + ) -> Result<( + Vec, + Vec, + i32, + Vec, + )> { self.transaction(move |tx| async move { self.check_user_is_channel_member(channel_id, user, &*tx) .await?; @@ -453,6 +458,7 @@ impl Database { .collect::>(); let mut channel_members; + let max_version; if !operations.is_empty() { let max_operation = operations @@ -460,6 +466,11 @@ impl Database { .max_by_key(|op| (op.lamport_timestamp.as_ref(), op.replica_id.as_ref())) .unwrap(); + max_version = vec![proto::VectorClockEntry { + replica_id: *max_operation.replica_id.as_ref() as u32, + timestamp: *max_operation.lamport_timestamp.as_ref() as u32, + }]; + // get current channel participants and save the max operation above self.save_max_operation( user, @@ -492,6 +503,7 @@ impl Database { .await?; } else { channel_members = Vec::new(); + max_version = Vec::new(); } let mut connections = Vec::new(); @@ -510,7 +522,7 @@ impl Database { }); } - Ok((connections, channel_members)) + Ok((connections, channel_members, buffer.epoch, max_version)) }) .await } @@ -712,12 +724,12 @@ impl Database { .await } - pub async fn channels_with_changed_notes( + pub async fn unseen_channel_buffer_changes( &self, user_id: UserId, channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result> { + ) -> Result> { #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] enum QueryIds { ChannelId, @@ -750,37 +762,45 @@ impl Database { } drop(rows); - let last_operations = self - .get_last_operations_for_buffers(channel_ids_by_buffer_id.keys().copied(), &*tx) + let latest_operations = self + .get_latest_operations_for_buffers(channel_ids_by_buffer_id.keys().copied(), &*tx) .await?; - let mut channels_with_new_changes = HashSet::default(); - for last_operation in last_operations { - if let Some(observed_edit) = observed_edits_by_buffer_id.get(&last_operation.buffer_id) - { - if observed_edit.epoch == last_operation.epoch - && observed_edit.lamport_timestamp == last_operation.lamport_timestamp - && observed_edit.replica_id == last_operation.replica_id + let mut changes = Vec::default(); + for latest in latest_operations { + if let Some(observed) = observed_edits_by_buffer_id.get(&latest.buffer_id) { + if ( + observed.epoch, + observed.lamport_timestamp, + observed.replica_id, + ) >= (latest.epoch, latest.lamport_timestamp, latest.replica_id) { continue; } } - if let Some(channel_id) = channel_ids_by_buffer_id.get(&last_operation.buffer_id) { - channels_with_new_changes.insert(*channel_id); + if let Some(channel_id) = channel_ids_by_buffer_id.get(&latest.buffer_id) { + changes.push(proto::UnseenChannelBufferChange { + channel_id: channel_id.to_proto(), + epoch: latest.epoch as u64, + version: vec![proto::VectorClockEntry { + replica_id: latest.replica_id as u32, + timestamp: latest.lamport_timestamp as u32, + }], + }); } } - Ok(channels_with_new_changes) + Ok(changes) } - pub async fn get_last_operations_for_buffers( + pub async fn get_latest_operations_for_buffers( &self, - channel_ids: impl IntoIterator, + buffer_ids: impl IntoIterator, tx: &DatabaseTransaction, ) -> Result> { let mut values = String::new(); - for id in channel_ids { + for id in buffer_ids { if !values.is_empty() { values.push_str(", "); } @@ -795,13 +815,10 @@ impl Database { r#" SELECT * - FROM ( + FROM + ( SELECT - buffer_id, - epoch, - lamport_timestamp, - replica_id, - value, + *, row_number() OVER ( PARTITION BY buffer_id ORDER BY @@ -812,17 +829,17 @@ impl Database { FROM buffer_operations WHERE buffer_id in ({values}) - ) AS operations + ) AS last_operations WHERE row_number = 1 "#, ); let stmt = Statement::from_string(self.pool.get_database_backend(), sql); - let operations = buffer_operation::Model::find_by_statement(stmt) + Ok(buffer_operation::Entity::find() + .from_raw_sql(stmt) .all(&*tx) - .await?; - Ok(operations) + .await?) } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 207cca765783c2f9df5f7824964404fefc042518..ab31f59541887ac0c7a70db7ab60ade2bce79dbf 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -463,20 +463,20 @@ impl Database { } let channel_ids = graph.channels.iter().map(|c| c.id).collect::>(); - let channels_with_changed_notes = self - .channels_with_changed_notes(user_id, &channel_ids, &*tx) + let channel_buffer_changes = self + .unseen_channel_buffer_changes(user_id, &channel_ids, &*tx) .await?; - let channels_with_new_messages = self - .channels_with_new_messages(user_id, &channel_ids, &*tx) + let unseen_messages = self + .unseen_channel_messages(user_id, &channel_ids, &*tx) .await?; Ok(ChannelsForUser { channels: graph, channel_participants, channels_with_admin_privileges, - channels_with_changed_notes, - channels_with_new_messages, + unseen_buffer_changes: channel_buffer_changes, + channel_messages: unseen_messages, }) } diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 893c1726da3b44ee27e83c1d1b5e1ac35fad9697..db1252230ec0ed7e153922fbad5094d105371cc2 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -279,12 +279,12 @@ impl Database { Ok(()) } - pub async fn channels_with_new_messages( + pub async fn unseen_channel_messages( &self, user_id: UserId, channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result> { + ) -> Result> { let mut observed_messages_by_channel_id = HashMap::default(); let mut rows = observed_channel_messages::Entity::find() .filter(observed_channel_messages::Column::UserId.eq(user_id)) @@ -334,7 +334,7 @@ impl Database { .all(&*tx) .await?; - let mut channels_with_new_changes = HashSet::default(); + let mut changes = Vec::new(); for last_message in last_messages { if let Some(observed_message) = observed_messages_by_channel_id.get(&last_message.channel_id) @@ -343,10 +343,13 @@ impl Database { continue; } } - channels_with_new_changes.insert(last_message.channel_id); + changes.push(proto::UnseenChannelMessage { + channel_id: last_message.channel_id.to_proto(), + message_id: last_message.id.to_proto(), + }); } - Ok(channels_with_new_changes) + Ok(changes) } pub async fn remove_channel_message( diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 407cc2210830ff89e04f38ec0efc430a0b8df01e..e5d8ab8cf9a2a3d7cee5459c28b516e5ad24371d 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -235,7 +235,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .transaction(|tx| { let buffers = &buffers; async move { - db.get_last_operations_for_buffers([buffers[0].id, buffers[2].id], &*tx) + db.get_latest_operations_for_buffers([buffers[0].id, buffers[2].id], &*tx) .await } }) @@ -299,7 +299,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .transaction(|tx| { let buffers = &buffers; async move { - db.get_last_operations_for_buffers([buffers[1].id, buffers[2].id], &*tx) + db.get_latest_operations_for_buffers([buffers[1].id, buffers[2].id], &*tx) .await } }) @@ -317,7 +317,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .transaction(|tx| { let buffers = &buffers; async move { - db.get_last_operations_for_buffers([buffers[0].id, buffers[1].id], &*tx) + db.get_latest_operations_for_buffers([buffers[0].id, buffers[1].id], &*tx) .await } }) @@ -331,11 +331,11 @@ async fn test_channel_buffers_last_operations(db: &Database) { ], ); - let changed_channels = db + let buffer_changes = db .transaction(|tx| { let buffers = &buffers; async move { - db.channels_with_changed_notes( + db.unseen_channel_buffer_changes( observer_id, &[ buffers[0].channel_id, @@ -349,31 +349,42 @@ async fn test_channel_buffers_last_operations(db: &Database) { }) .await .unwrap(); + assert_eq!( - changed_channels, + buffer_changes, [ - buffers[0].channel_id, - buffers[1].channel_id, - buffers[2].channel_id, + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[0].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[0].version()), + }, + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[1].channel_id.to_proto(), + epoch: 1, + version: serialize_version(&text_buffers[1].version()), + }, + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[2].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[2].version()), + }, ] - .into_iter() - .collect::>() ); db.observe_buffer_version( buffers[1].id, observer_id, 1, - &serialize_version(&text_buffers[1].version()), + serialize_version(&text_buffers[1].version()).as_slice(), ) .await .unwrap(); - let changed_channels = db + let buffer_changes = db .transaction(|tx| { let buffers = &buffers; async move { - db.channels_with_changed_notes( + db.unseen_channel_buffer_changes( observer_id, &[ buffers[0].channel_id, @@ -387,11 +398,21 @@ async fn test_channel_buffers_last_operations(db: &Database) { }) .await .unwrap(); + assert_eq!( - changed_channels, - [buffers[0].channel_id, buffers[2].channel_id,] - .into_iter() - .collect::>() + buffer_changes, + [ + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[0].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[0].version()), + }, + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[2].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[2].version()), + }, + ] ); // Observe an earlier version of the buffer. @@ -407,11 +428,11 @@ async fn test_channel_buffers_last_operations(db: &Database) { .await .unwrap(); - let changed_channels = db + let buffer_changes = db .transaction(|tx| { let buffers = &buffers; async move { - db.channels_with_changed_notes( + db.unseen_channel_buffer_changes( observer_id, &[ buffers[0].channel_id, @@ -425,11 +446,21 @@ async fn test_channel_buffers_last_operations(db: &Database) { }) .await .unwrap(); + assert_eq!( - changed_channels, - [buffers[0].channel_id, buffers[2].channel_id,] - .into_iter() - .collect::>() + buffer_changes, + [ + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[0].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[0].version()), + }, + rpc::proto::UnseenChannelBufferChange { + channel_id: buffers[2].channel_id.to_proto(), + epoch: 0, + version: serialize_version(&text_buffers[2].version()), + }, + ] ); } diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index f3d385e4a0f5180ee14a185b649dd89ceea1e2a1..4966ef1bda4fc99647edf03daa8f9a09ff47474b 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -144,25 +144,32 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - let _ = db + let (fourth_message, _, _) = db .create_channel_message(channel_2, user, "2_1", OffsetDateTime::now_utc(), 4) .await .unwrap(); // Check that observer has new messages - let channels_with_new_messages = db + let unseen_messages = db .transaction(|tx| async move { - db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) .await }) .await .unwrap(); assert_eq!( - channels_with_new_messages, - [channel_1, channel_2] - .into_iter() - .collect::>() + unseen_messages, + [ + rpc::proto::UnseenChannelMessage { + channel_id: channel_1.to_proto(), + message_id: third_message.to_proto(), + }, + rpc::proto::UnseenChannelMessage { + channel_id: channel_2.to_proto(), + message_id: fourth_message.to_proto(), + }, + ] ); // Observe the second message @@ -171,18 +178,25 @@ async fn test_channel_message_new_notification(db: &Arc) { .unwrap(); // Make sure the observer still has a new message - let channels_with_new_messages = db + let unseen_messages = db .transaction(|tx| async move { - db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) .await }) .await .unwrap(); assert_eq!( - channels_with_new_messages, - [channel_1, channel_2] - .into_iter() - .collect::>() + unseen_messages, + [ + rpc::proto::UnseenChannelMessage { + channel_id: channel_1.to_proto(), + message_id: third_message.to_proto(), + }, + rpc::proto::UnseenChannelMessage { + channel_id: channel_2.to_proto(), + message_id: fourth_message.to_proto(), + }, + ] ); // Observe the third message, @@ -191,16 +205,20 @@ async fn test_channel_message_new_notification(db: &Arc) { .unwrap(); // Make sure the observer does not have a new method - let channels_with_new_messages = db + let unseen_messages = db .transaction(|tx| async move { - db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) .await }) .await .unwrap(); + assert_eq!( - channels_with_new_messages, - [channel_2].into_iter().collect::>() + unseen_messages, + [rpc::proto::UnseenChannelMessage { + channel_id: channel_2.to_proto(), + message_id: fourth_message.to_proto(), + }] ); // Observe the second message again, should not regress our observed state @@ -208,16 +226,19 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - // Make sure the observer does not have a new method - let channels_with_new_messages = db + // Make sure the observer does not have a new message + let unseen_messages = db .transaction(|tx| async move { - db.channels_with_new_messages(observer, &[channel_1, channel_2], &*tx) + db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) .await }) .await .unwrap(); assert_eq!( - channels_with_new_messages, - [channel_2].into_iter().collect::>() + unseen_messages, + [rpc::proto::UnseenChannelMessage { + channel_id: channel_2.to_proto(), + message_id: fourth_message.to_proto(), + }] ); } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 371d0466c1fc956a64c3fa9142fe7fea6ad0dd18..1f0ecce2bf0d8e37a1af834d0ab607bf61cb016c 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -274,7 +274,8 @@ impl Server { .add_message_handler(unfollow) .add_message_handler(update_followers) .add_message_handler(update_diff_base) - .add_request_handler(get_private_user_info); + .add_request_handler(get_private_user_info) + .add_message_handler(acknowledge_channel_message); Arc::new(server) } @@ -2568,16 +2569,8 @@ async fn respond_to_channel_invite( name: channel.name, }), ); - update.notes_changed = result - .channels_with_changed_notes - .iter() - .map(|id| id.to_proto()) - .collect(); - update.new_messages = result - .channels_with_new_messages - .iter() - .map(|id| id.to_proto()) - .collect(); + update.unseen_channel_messages = result.channel_messages; + update.unseen_channel_buffer_changes = result.unseen_buffer_changes; update.insert_edge = result.channels.edges; update .channel_participants @@ -2701,7 +2694,7 @@ async fn update_channel_buffer( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let (collaborators, non_collaborators) = db + let (collaborators, non_collaborators, epoch, version) = db .update_channel_buffer(channel_id, session.user_id, &request.operations) .await?; @@ -2726,7 +2719,11 @@ async fn update_channel_buffer( session.peer.send( peer_id.into(), proto::UpdateChannels { - notes_changed: vec![channel_id.to_proto()], + unseen_channel_buffer_changes: vec![proto::UnseenChannelBufferChange { + channel_id: channel_id.to_proto(), + epoch: epoch as u64, + version: version.clone(), + }], ..Default::default() }, ) @@ -2859,9 +2856,7 @@ async fn send_channel_message( message: Some(message), })?; - dbg!(&non_participants); let pool = &*session.connection_pool().await; - broadcast( None, non_participants @@ -2871,7 +2866,10 @@ async fn send_channel_message( session.peer.send( peer_id.into(), proto::UpdateChannels { - new_messages: vec![channel_id.to_proto()], + unseen_channel_messages: vec![proto::UnseenChannelMessage { + channel_id: channel_id.to_proto(), + message_id: message_id.to_proto(), + }], ..Default::default() }, ) @@ -2900,6 +2898,20 @@ async fn remove_channel_message( Ok(()) } +async fn acknowledge_channel_message( + request: proto::AckChannelMessage, + session: Session, +) -> Result<()> { + let channel_id = ChannelId::from_proto(request.channel_id); + let message_id = MessageId::from_proto(request.message_id); + session + .db() + .await + .observe_channel_message(channel_id, session.user_id, message_id) + .await?; + Ok(()) +} + async fn join_channel_chat( request: proto::JoinChannelChat, response: Response, @@ -3035,18 +3047,8 @@ fn build_initial_channels_update( }); } - update.notes_changed = channels - .channels_with_changed_notes - .iter() - .map(|channel_id| channel_id.to_proto()) - .collect(); - - update.new_messages = channels - .channels_with_new_messages - .iter() - .map(|channel_id| channel_id.to_proto()) - .collect(); - + update.unseen_channel_buffer_changes = channels.unseen_buffer_changes; + update.unseen_channel_messages = channels.channel_messages; update.insert_edge = channels.channels.edges; for (channel_id, participants) in channels.channel_participants { diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 68acffacf8fc0271a92a796b5b7603643b771896..d9d12d485b3ea290bfab9382ef4e6e91721169b0 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -445,8 +445,8 @@ fn channel(id: u64, name: &'static str) -> Channel { Channel { id, name: name.to_string(), - has_note_changed: false, - has_new_messages: false, + unseen_note_version: None, + unseen_message_id: None, } } diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index a56df311bd7d4c79184c788b1eb4e3bca941cdcd..cf5b58703cb95dbf255fb1a6d4ea2d163ad5b6d1 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -151,12 +151,12 @@ impl TestServer { Arc::get_mut(&mut client) .unwrap() - .set_id(user_id.0 as usize) + .set_id(user_id.to_proto()) .override_authenticate(move |cx| { cx.spawn(|_| async move { let access_token = "the-token".to_string(); Ok(Credentials { - user_id: user_id.0 as u64, + user_id: user_id.to_proto(), access_token, }) }) diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index 1d9e409748e826a524fdb1fbd636ff8383015705..a95576805074d63a5a74de8d1d107899cb956714 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Result}; use call::report_call_event_for_channel; -use channel::{Channel, ChannelBuffer, ChannelBufferEvent, ChannelId}; +use channel::{Channel, ChannelBuffer, ChannelBufferEvent, ChannelId, ChannelStore}; use client::{ proto::{self, PeerId}, Collaborator, ParticipantIndex, @@ -36,6 +36,7 @@ pub fn init(cx: &mut AppContext) { pub struct ChannelView { pub editor: ViewHandle, project: ModelHandle, + channel_store: ModelHandle, channel_buffer: ModelHandle, remote_id: Option, _editor_event_subscription: Subscription, @@ -94,7 +95,13 @@ impl ChannelView { pane.update(&mut cx, |pane, cx| { pane.items_of_type::() .find(|channel_view| channel_view.read(cx).channel_buffer == channel_buffer) - .unwrap_or_else(|| cx.add_view(|cx| Self::new(project, channel_buffer, cx))) + .unwrap_or_else(|| { + cx.add_view(|cx| { + let mut this = Self::new(project, channel_store, channel_buffer, cx); + this.acknowledge_buffer_version(cx); + this + }) + }) }) .ok_or_else(|| anyhow!("pane was dropped")) }) @@ -102,6 +109,7 @@ impl ChannelView { pub fn new( project: ModelHandle, + channel_store: ModelHandle, channel_buffer: ModelHandle, cx: &mut ViewContext, ) -> Self { @@ -121,6 +129,7 @@ impl ChannelView { Self { editor, project, + channel_store, channel_buffer, remote_id: None, _editor_event_subscription, @@ -137,13 +146,44 @@ impl ChannelView { event: &ChannelBufferEvent, cx: &mut ViewContext, ) { - if let ChannelBufferEvent::Disconnected = event { - self.editor.update(cx, |editor, cx| { + match event { + ChannelBufferEvent::Disconnected => self.editor.update(cx, |editor, cx| { editor.set_read_only(true); cx.notify(); - }) + }), + ChannelBufferEvent::BufferEdited => { + if cx.is_self_focused() || self.editor.is_focused(cx) { + self.acknowledge_buffer_version(cx); + } else { + self.channel_store.update(cx, |store, cx| { + let channel_buffer = self.channel_buffer.read(cx); + store.notes_changed( + channel_buffer.channel().id, + channel_buffer.epoch(), + &channel_buffer.buffer().read(cx).version(), + cx, + ) + }); + } + } + _ => {} } } + + fn acknowledge_buffer_version(&mut self, cx: &mut ViewContext<'_, '_, ChannelView>) { + self.channel_store.update(cx, |store, cx| { + let channel_buffer = self.channel_buffer.read(cx); + store.acknowledge_notes_version( + channel_buffer.channel().id, + channel_buffer.epoch(), + &channel_buffer.buffer().read(cx).version(), + cx, + ) + }); + self.channel_buffer.update(cx, |buffer, cx| { + buffer.acknowledge_buffer_version(cx); + }); + } } impl Entity for ChannelView { @@ -161,6 +201,7 @@ impl View for ChannelView { fn focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { if cx.is_self_focused() { + self.acknowledge_buffer_version(cx); cx.focus(self.editor.as_any()) } } @@ -200,6 +241,7 @@ impl Item for ChannelView { fn clone_on_split(&self, _: WorkspaceId, cx: &mut ViewContext) -> Option { Some(Self::new( self.project.clone(), + self.channel_store.clone(), self.channel_buffer.clone(), cx, )) diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 81a421e8d904272fe56de27d20d20a8a8f036f8e..626b3004d78e1c94b5f770a2421e130979090384 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -42,6 +42,7 @@ pub struct ChatPanel { local_timezone: UtcOffset, fs: Arc, width: Option, + active: bool, pending_serialization: Task>, subscriptions: Vec, workspace: WeakViewHandle, @@ -138,6 +139,7 @@ impl ChatPanel { has_focus: false, subscriptions: Vec::new(), workspace: workspace_handle, + active: false, width: None, }; @@ -154,9 +156,9 @@ impl ChatPanel { }), ); - this.init_active_channel(cx); + this.update_channel_count(cx); cx.observe(&this.channel_store, |this, _, cx| { - this.init_active_channel(cx); + this.update_channel_count(cx) }) .detach(); @@ -225,10 +227,8 @@ impl ChatPanel { ); } - fn init_active_channel(&mut self, cx: &mut ViewContext) { + fn update_channel_count(&mut self, cx: &mut ViewContext) { let channel_count = self.channel_store.read(cx).channel_count(); - self.message_list.reset(0); - self.active_chat = None; self.channel_select.update(cx, |select, cx| { select.set_item_count(channel_count, cx); }); @@ -247,6 +247,7 @@ impl ChatPanel { } let subscription = cx.subscribe(&chat, Self::channel_did_change); self.active_chat = Some((chat, subscription)); + self.acknowledge_last_message(cx); self.channel_select.update(cx, |select, cx| { if let Some(ix) = self.channel_store.read(cx).index_of_channel(id) { select.set_selected_index(ix, cx); @@ -268,11 +269,22 @@ impl ChatPanel { new_count, } => { self.message_list.splice(old_range.clone(), *new_count); + self.acknowledge_last_message(cx); } } cx.notify(); } + fn acknowledge_last_message(&mut self, cx: &mut ViewContext<'_, '_, ChatPanel>) { + if self.active { + if let Some((chat, _)) = &self.active_chat { + chat.update(cx, |chat, cx| { + chat.acknowledge_last_message(cx); + }); + } + } + } + fn render_channel(&self, cx: &mut ViewContext) -> AnyElement { let theme = theme::current(cx); Flex::column() @@ -627,8 +639,12 @@ impl Panel for ChatPanel { } fn set_active(&mut self, active: bool, cx: &mut ViewContext) { - if active && !is_chat_feature_enabled(cx) { - cx.emit(Event::Dismissed); + self.active = active; + if active { + self.acknowledge_last_message(cx); + if !is_chat_feature_enabled(cx) { + cx.emit(Event::Dismissed); + } } } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 53d5140a12a43172a25a64cf554a9efd68fd2bbb..5e90d438fc175335f52a06e025f470fcd898016d 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1821,7 +1821,7 @@ impl CollabPanel { channel.name.clone(), theme .channel_name - .in_state(channel.has_new_messages) + .in_state(channel.unseen_message_id.is_some()) .text .clone(), ) @@ -1880,7 +1880,7 @@ impl CollabPanel { let participants = self.channel_store.read(cx).channel_participants(channel_id); if participants.is_empty() { - if channel.has_note_changed { + if channel.unseen_note_version.is_some() { Svg::new("icons/terminal.svg") .with_color(theme.channel_note_active_color) .constrained() diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 0f289edcf8bd91b79405d3df8c7ef7e265702c3f..3501e70e6ac4191c5fe3141620e18cd8f7b8c32d 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -957,8 +957,19 @@ message UpdateChannels { repeated uint64 remove_channel_invitations = 6; repeated ChannelParticipants channel_participants = 7; repeated ChannelPermission channel_permissions = 8; - repeated uint64 notes_changed = 9; - repeated uint64 new_messages = 10; + repeated UnseenChannelMessage unseen_channel_messages = 9; + repeated UnseenChannelBufferChange unseen_channel_buffer_changes = 10; +} + +message UnseenChannelMessage { + uint64 channel_id = 1; + uint64 message_id = 2; +} + +message UnseenChannelBufferChange { + uint64 channel_id = 1; + uint64 epoch = 2; + repeated VectorClockEntry version = 3; } message ChannelEdge { @@ -1127,8 +1138,7 @@ message RejoinChannelBuffersResponse { message AckBufferOperation { uint64 buffer_id = 1; uint64 epoch = 2; - uint64 lamport_timestamp = 3; - uint64 replica_id = 4; + repeated VectorClockEntry version = 3; } message JoinChannelBufferResponse { From 23ee8211c7f44dcab15c2f3b39a5260547fcdea9 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 19:30:05 -0700 Subject: [PATCH 14/20] Lower frequency of popup warning when leaving a call co-authored-by: conrad --- crates/call/src/room.rs | 4 ++++ crates/collab_ui/src/collab_panel.rs | 11 +++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index f24a8e9a9cf84743aa5a588f5be8e03dcc739aa0..ce010995763b5e9ae30cde5b8078f02d58789fdc 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -104,6 +104,10 @@ impl Room { self.channel_id } + pub fn is_sharing_project(&self) -> bool { + !self.shared_projects.is_empty() + } + #[cfg(any(test, feature = "test-support"))] pub fn is_connected(&self) -> bool { if let Some(live_kit) = self.live_kit.as_ref() { diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 4a0b37b26206bbb59f1775dbbe3c114534f7a3e9..7a81b49868d009b56ca276ac3b9390f5e2835869 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -3165,10 +3165,17 @@ impl CollabPanel { let window = cx.window(); let active_call = ActiveCall::global(cx); cx.spawn(|_, mut cx| async move { - if active_call.read_with(&mut cx, |active_call, _| active_call.room().is_some()) { + if active_call.read_with(&mut cx, |active_call, cx| { + if let Some(room) = active_call.room() { + let room = room.read(cx); + room.is_sharing_project() && room.remote_participants().len() > 0 + } else { + false + } + }) { let answer = window.prompt( PromptLevel::Warning, - "Do you want to leave the current call?", + "Leaving this call will unshare your current project.\nDo you want to switch channels?", &["Yes, Join Channel", "Cancel"], &mut cx, ); From 4ff80a707407716cea9e9f664a3421403f304f56 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 19:31:55 -0700 Subject: [PATCH 15/20] Fix a few mouse event id bugs and move facepile to the left co-authored-by: conrad --- crates/collab_ui/src/collab_panel.rs | 185 +++++++++++++++------------ 1 file changed, 102 insertions(+), 83 deletions(-) diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 7a81b49868d009b56ca276ac3b9390f5e2835869..5b2b7f001862a51d85d48e49a4df65c49dbed484 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -592,6 +592,7 @@ impl CollabPanel { *channel_id, &theme.collab_panel, is_selected, + ix, cx, ), ListEntry::ChannelInvite(channel) => Self::render_channel_invite( @@ -1918,7 +1919,8 @@ impl CollabPanel { enum ChannelCall {} enum ChannelNote {} - enum IconTooltip {} + enum NotesTooltip {} + enum ChatTooltip {} enum ChannelTooltip {} let mut is_dragged_over = false; @@ -1965,72 +1967,111 @@ impl CollabPanel { let style = collab_theme .channel_name .in_state(channel.unseen_note_version.is_some()); - Label::new(channel.name.clone(), style.text.clone()) - .contained() - .with_style(style.container) - .aligned() - .left() - .with_tooltip::( - channel_id as usize, - if is_active { - "Open channel notes" - } else { - "Join channel" - }, - None, - theme.tooltip.clone(), - cx, - ) - .flex(1., true) - }) - .with_child( - MouseEventHandler::new::(ix, cx, move |_, cx| { - let participants = - self.channel_store.read(cx).channel_participants(channel_id); - if !participants.is_empty() { - let extra_count = participants.len().saturating_sub(FACEPILE_LIMIT); - - FacePile::new(collab_theme.face_overlap) - .with_children( - participants - .iter() - .filter_map(|user| { - Some( - Image::from_data(user.avatar.clone()?) - .with_style(collab_theme.channel_avatar), - ) - }) - .take(FACEPILE_LIMIT), - ) - .with_children((extra_count > 0).then(|| { - Label::new( - format!("+{}", extra_count), - collab_theme.extra_participant_label.text.clone(), - ) - .contained() - .with_style(collab_theme.extra_participant_label.container) - })) - .with_tooltip::( - channel_id as usize, + Flex::row() + .with_child( + Label::new(channel.name.clone(), style.text.clone()) + .contained() + .with_style(style.container) + .aligned() + .left() + .with_tooltip::( + ix, if is_active { - "Open Channel Notes" + "Open channel notes" } else { "Join channel" }, None, theme.tooltip.clone(), cx, - ) + ), + ) + .with_children({ + let participants = + self.channel_store.read(cx).channel_participants(channel_id); + + if !participants.is_empty() { + let extra_count = participants.len().saturating_sub(FACEPILE_LIMIT); + + let result = FacePile::new(collab_theme.face_overlap) + .with_children( + participants + .iter() + .filter_map(|user| { + Some( + Image::from_data(user.avatar.clone()?) + .with_style(collab_theme.channel_avatar), + ) + }) + .take(FACEPILE_LIMIT), + ) + .with_children((extra_count > 0).then(|| { + Label::new( + format!("+{}", extra_count), + collab_theme.extra_participant_label.text.clone(), + ) + .contained() + .with_style(collab_theme.extra_participant_label.container) + })); + + Some(result) + } else { + None + } + }) + .with_spacing(8.) + .align_children_center() + .flex(1., true) + }) + .with_child( + MouseEventHandler::new::(ix, cx, move |_, _| { + if channel.unseen_message_id.is_some() { + Svg::new("icons/conversations.svg") + .with_color(collab_theme.channel_note_active_color) + .constrained() + .with_width(collab_theme.channel_hash.width) .into_any() } else if row_hovered { - Svg::new("icons/file.svg") + Svg::new("icons/conversations.svg") .with_color(collab_theme.channel_hash.color) .constrained() .with_width(collab_theme.channel_hash.width) + .into_any() + } else { + Empty::new() + .constrained() + .with_width(collab_theme.channel_hash.width) + .into_any() + } + }) + .on_click(MouseButton::Left, move |_, this, cx| { + this.join_channel_chat(&JoinChannelChat { channel_id }, cx); + }) + .with_tooltip::( + ix, + "Open channel chat", + None, + theme.tooltip.clone(), + cx, + ) + .contained() + .with_margin_right(4.), + ) + .with_child( + MouseEventHandler::new::(ix, cx, move |_, cx| { + if row_hovered || channel.unseen_note_version.is_some() { + Svg::new("icons/file.svg") + .with_color(if channel.unseen_note_version.is_some() { + collab_theme.channel_note_active_color + } else { + collab_theme.channel_hash.color + }) + .constrained() + .with_width(collab_theme.channel_hash.width) .contained() .with_margin_right(collab_theme.channel_hash.container.margin.left) - .with_tooltip::( - channel_id as usize, + .with_tooltip::( + ix as usize, "Open channel notes", None, theme.tooltip.clone(), @@ -2038,7 +2079,12 @@ impl CollabPanel { ) .into_any() } else { - Empty::new().into_any() + Empty::new() + .constrained() + .with_width(collab_theme.channel_hash.width) + .contained() + .with_margin_right(collab_theme.channel_hash.container.margin.left) + .into_any() } }) .on_click(MouseButton::Left, move |_, this, cx| { @@ -2051,34 +2097,6 @@ impl CollabPanel { }; }), ) - .with_child( - MouseEventHandler::new::(ix, cx, move |_, cx| { - let participants = - self.channel_store.read(cx).channel_participants(channel_id); - if participants.is_empty() { - if channel.unseen_message_id.is_some() { - Svg::new("icons/conversations.svg") - .with_color(collab_theme.channel_note_active_color) - .constrained() - .with_width(collab_theme.channel_hash.width) - .into_any() - } else if row_hovered { - Svg::new("icons/conversations.svg") - .with_color(collab_theme.channel_hash.color) - .constrained() - .with_width(collab_theme.channel_hash.width) - .into_any() - } else { - Empty::new().into_any() - } - } else { - Empty::new().into_any() - } - }) - .on_click(MouseButton::Left, move |_, this, cx| { - this.join_channel_chat(&JoinChannelChat { channel_id }, cx); - }), - ) .align_children_center() .styleable_component() .disclosable( @@ -2223,6 +2241,7 @@ impl CollabPanel { channel_id: ChannelId, theme: &theme::CollabPanel, is_selected: bool, + ix: usize, cx: &mut ViewContext, ) -> AnyElement { enum ChannelNotes {} @@ -2232,7 +2251,7 @@ impl CollabPanel { .or(theme.contact_avatar.height) .unwrap_or(0.); - MouseEventHandler::new::(channel_id as usize, cx, |state, cx| { + MouseEventHandler::new::(ix as usize, cx, |state, cx| { let tree_branch = *theme.tree_branch.in_state(is_selected).style_for(state); let row = theme.project_row.in_state(is_selected).style_for(state); From 3bc7024f8b2a568ec219c413399ea1faba280844 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 20:01:10 -0700 Subject: [PATCH 16/20] Fix unit test co-authored-by: Conrad --- crates/collab/src/tests/channel_buffer_tests.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 41662f10c4ac9192a11aae4bcff158322e9498f3..82bd7e6afee7c39a758d1424885b6c746a7abddf 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -804,11 +804,13 @@ async fn test_channel_buffer_changes( assert!(has_buffer_changed); // Opening the buffer should clear the changed flag. - let channel_buffer_b = client_b - .channel_store() - .update(cx_b, |store, cx| store.open_channel_buffer(channel_id, cx)) + let project_b = client_b.build_empty_local_project(cx_b); + let workspace_b = client_b.build_workspace(&project_b, cx_b).root(cx_b); + let channel_view_b = cx_b + .update(|cx| ChannelView::open(channel_id, workspace_b.clone(), cx)) .await .unwrap(); + deterministic.run_until_parked(); let has_buffer_changed = cx_b.read(|cx| { @@ -840,8 +842,12 @@ async fn test_channel_buffer_changes( assert!(!has_buffer_changed); // Closing the buffer should re-enable change tracking - cx_b.update(|_| { - drop(channel_buffer_b); + cx_b.update(|cx| { + workspace_b.update(cx, |workspace, cx| { + workspace.close_all_items_and_panes(&Default::default(), cx) + }); + + drop(channel_view_b) }); deterministic.run_until_parked(); From db8096ccdc4d34321df95aa70e53e90b0fe4fe61 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 3 Oct 2023 20:47:54 -0700 Subject: [PATCH 17/20] Fix most tests for new chat changes --- crates/collab/src/db/tests/buffer_tests.rs | 8 +++-- .../collab/src/tests/channel_message_tests.rs | 32 +++++++++++++++---- crates/collab_ui/src/chat_panel.rs | 4 +++ crates/collab_ui/src/collab_panel.rs | 4 +-- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index e5d8ab8cf9a2a3d7cee5459c28b516e5ad24371d..f6e91b91f011046d6a20431ac10099b21e543774 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -350,7 +350,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .await .unwrap(); - assert_eq!( + pretty_assertions::assert_eq!( buffer_changes, [ rpc::proto::UnseenChannelBufferChange { @@ -361,7 +361,11 @@ async fn test_channel_buffers_last_operations(db: &Database) { rpc::proto::UnseenChannelBufferChange { channel_id: buffers[1].channel_id.to_proto(), epoch: 1, - version: serialize_version(&text_buffers[1].version()), + version: serialize_version(&text_buffers[1].version()) + .into_iter() + .filter(|vector| vector.replica_id + == buffer_changes[1].version.first().unwrap().replica_id) + .collect::>(), }, rpc::proto::UnseenChannelBufferChange { channel_id: buffers[2].channel_id.to_proto(), diff --git a/crates/collab/src/tests/channel_message_tests.rs b/crates/collab/src/tests/channel_message_tests.rs index 1b3a54dc428dfdcae10b3ebc5f4dcb451bf6a514..0ee17cec599de358b79b951af6682b332b5909e3 100644 --- a/crates/collab/src/tests/channel_message_tests.rs +++ b/crates/collab/src/tests/channel_message_tests.rs @@ -1,7 +1,9 @@ use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer}; use channel::{ChannelChat, ChannelMessageId}; +use collab_ui::chat_panel::ChatPanel; use gpui::{executor::Deterministic, BorrowAppContext, ModelHandle, TestAppContext}; use std::sync::Arc; +use workspace::dock::Panel; #[gpui::test] async fn test_basic_channel_messages( @@ -244,6 +246,15 @@ async fn test_channel_message_changes( ) .await; + let other_channel_id = server + .make_channel( + "other-channel", + None, + (&client_a, cx_a), + &mut [(&client_b, cx_b)], + ) + .await; + // Client A sends a message, client B should see that there is a new message. let channel_chat_a = client_a .channel_store() @@ -269,12 +280,22 @@ async fn test_channel_message_changes( assert!(b_has_messages); // Opening the chat should clear the changed flag. - let channel_chat_b = client_b - .channel_store() - .update(cx_b, |store, cx| store.open_channel_chat(channel_id, cx)) + cx_b.update(|cx| { + collab_ui::init(&client_b.app_state, cx); + }); + let project_b = client_b.build_empty_local_project(cx_b); + let workspace_b = client_b.build_workspace(&project_b, cx_b).root(cx_b); + let chat_panel_b = workspace_b.update(cx_b, |workspace, cx| ChatPanel::new(workspace, cx)); + chat_panel_b + .update(cx_b, |chat_panel, cx| { + chat_panel.set_active(true, cx); + chat_panel.select_channel(channel_id, cx) + }) .await .unwrap(); + deterministic.run_until_parked(); + let b_has_messages = cx_b.read_with(|cx| { client_b .channel_store() @@ -304,10 +325,7 @@ async fn test_channel_message_changes( assert!(!b_has_messages); // Closing the chat should re-enable change tracking - - cx_b.update(|_| { - drop(channel_chat_b); - }); + todo!(); deterministic.run_until_parked(); diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 626b3004d78e1c94b5f770a2421e130979090384..2050ee2f69e974e4e1c25de8175deca65520366d 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -181,6 +181,10 @@ impl ChatPanel { }) } + pub fn active_chat(&self) -> Option> { + self.active_chat.as_ref().map(|(chat, _)| chat.clone()) + } + pub fn load( workspace: WeakViewHandle, cx: AsyncAppContext, diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 5b2b7f001862a51d85d48e49a4df65c49dbed484..6214eecac2adf119de13de5a00292b4167954973 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1964,9 +1964,7 @@ impl CollabPanel { .left(), ) .with_child({ - let style = collab_theme - .channel_name - .in_state(channel.unseen_note_version.is_some()); + let style = collab_theme.channel_name.inactive_state(); Flex::row() .with_child( Label::new(channel.name.clone(), style.text.clone()) From e548572f121e373e5368bb071b3580fd22f60c67 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Wed, 4 Oct 2023 10:13:02 -0700 Subject: [PATCH 18/20] Fix channel messages test --- crates/channel/src/channel_chat.rs | 14 ++++++- crates/channel/src/channel_store.rs | 10 +++++ .../src/channel_store/channel_index.rs | 20 ++++++++-- .../collab/src/tests/channel_message_tests.rs | 38 +++++++++++++------ crates/collab_ui/src/chat_panel.rs | 14 ++++++- 5 files changed, 77 insertions(+), 19 deletions(-) diff --git a/crates/channel/src/channel_chat.rs b/crates/channel/src/channel_chat.rs index 5b32d67f63b0bb12450a75856b5d7f57f931a9a9..29a260ea7e58a68d590ff2cfc6827a3e01ecfa54 100644 --- a/crates/channel/src/channel_chat.rs +++ b/crates/channel/src/channel_chat.rs @@ -1,4 +1,4 @@ -use crate::{Channel, ChannelStore}; +use crate::{Channel, ChannelId, ChannelStore}; use anyhow::{anyhow, Result}; use client::{ proto, @@ -57,6 +57,10 @@ pub enum ChannelChatEvent { old_range: Range, new_count: usize, }, + NewMessage { + channel_id: ChannelId, + message_id: u64, + }, } pub fn init(client: &Arc) { @@ -338,10 +342,15 @@ impl ChannelChat { .payload .message .ok_or_else(|| anyhow!("empty message"))?; + let message_id = message.id; let message = ChannelMessage::from_proto(message, &user_store, &mut cx).await?; this.update(&mut cx, |this, cx| { - this.insert_messages(SumTree::from_item(message, &()), cx) + this.insert_messages(SumTree::from_item(message, &()), cx); + cx.emit(ChannelChatEvent::NewMessage { + channel_id: this.channel.id, + message_id, + }) }); Ok(()) @@ -413,6 +422,7 @@ impl ChannelChat { old_range: start_ix..end_ix, new_count, }); + cx.notify(); } } diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index db6d5f42c5309e01c33469aa581faa142377f2bb..bd72c92c7db768c558f7bc1b39a371f01f5dfd6c 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -234,6 +234,16 @@ impl ChannelStore { cx.notify(); } + pub fn new_message( + &mut self, + channel_id: ChannelId, + message_id: u64, + cx: &mut ModelContext, + ) { + self.channel_index.new_message(channel_id, message_id); + cx.notify(); + } + pub fn acknowledge_message_id( &mut self, channel_id: ChannelId, diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 2a93ca6573cc4aa999f4d43d023aa9df47edb663..bf0de1b644cee0df8df3948e504e406a8f9cd3e7 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -71,6 +71,10 @@ impl ChannelIndex { pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) { insert_note_changed(&mut self.channels_by_id, channel_id, epoch, version); } + + pub fn new_message(&mut self, channel_id: ChannelId, message_id: u64) { + insert_new_message(&mut self.channels_by_id, channel_id, message_id) + } } impl Deref for ChannelIndex { @@ -114,10 +118,7 @@ impl<'a> ChannelPathsInsertGuard<'a> { } pub fn new_messages(&mut self, channel_id: ChannelId, message_id: u64) { - if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - let unseen_message_id = Arc::make_mut(channel).unseen_message_id.get_or_insert(0); - *unseen_message_id = message_id.max(*unseen_message_id); - } + insert_new_message(&mut self.channels_by_id, channel_id, message_id) } pub fn insert(&mut self, channel_proto: proto::Channel) { @@ -224,3 +225,14 @@ fn insert_note_changed( } } } + +fn insert_new_message( + channels_by_id: &mut BTreeMap>, + channel_id: u64, + message_id: u64, +) { + if let Some(channel) = channels_by_id.get_mut(&channel_id) { + let unseen_message_id = Arc::make_mut(channel).unseen_message_id.get_or_insert(0); + *unseen_message_id = message_id.max(*unseen_message_id); + } +} diff --git a/crates/collab/src/tests/channel_message_tests.rs b/crates/collab/src/tests/channel_message_tests.rs index 0ee17cec599de358b79b951af6682b332b5909e3..0fc3b085edde00cbfe552352fe5590753f686134 100644 --- a/crates/collab/src/tests/channel_message_tests.rs +++ b/crates/collab/src/tests/channel_message_tests.rs @@ -246,15 +246,6 @@ async fn test_channel_message_changes( ) .await; - let other_channel_id = server - .make_channel( - "other-channel", - None, - (&client_a, cx_a), - &mut [(&client_b, cx_b)], - ) - .await; - // Client A sends a message, client B should see that there is a new message. let channel_chat_a = client_a .channel_store() @@ -324,16 +315,39 @@ async fn test_channel_message_changes( assert!(!b_has_messages); - // Closing the chat should re-enable change tracking - todo!(); + // Sending a message while the chat is closed should change the flag. + chat_panel_b.update(cx_b, |chat_panel, cx| { + chat_panel.set_active(false, cx); + }); + + // Sending a message while the chat is open should not change the flag. + channel_chat_a + .update(cx_a, |c, cx| c.send_message("three".into(), cx).unwrap()) + .await + .unwrap(); deterministic.run_until_parked(); + let b_has_messages = cx_b.read_with(|cx| { + client_b + .channel_store() + .read(cx) + .has_new_messages(channel_id) + .unwrap() + }); + + assert!(b_has_messages); + + // Closing the chat should re-enable change tracking + cx_b.update(|_| drop(chat_panel_b)); + channel_chat_a - .update(cx_a, |c, cx| c.send_message("three".into(), cx).unwrap()) + .update(cx_a, |c, cx| c.send_message("four".into(), cx).unwrap()) .await .unwrap(); + deterministic.run_until_parked(); + let b_has_messages = cx_b.read_with(|cx| { client_b .channel_store() diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 2050ee2f69e974e4e1c25de8175deca65520366d..f0874f544eab5adc7bda8dad976a71dd969bffa0 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -273,7 +273,19 @@ impl ChatPanel { new_count, } => { self.message_list.splice(old_range.clone(), *new_count); - self.acknowledge_last_message(cx); + if self.active { + self.acknowledge_last_message(cx); + } + } + ChannelChatEvent::NewMessage { + channel_id, + message_id, + } => { + if !self.active { + self.channel_store.update(cx, |store, cx| { + store.new_message(*channel_id, *message_id, cx) + }) + } } } cx.notify(); From dd0edcd20336ce83bb01728b13e324a958bd07a1 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Wed, 4 Oct 2023 11:46:08 -0700 Subject: [PATCH 19/20] Changed the on-click behavior of joining a channel to not open the chat, and only open 1 project instead of all projects Co-authored-by: conrad Co-authored-by: max --- crates/call/src/room.rs | 12 +++++------- crates/collab_ui/src/collab_panel.rs | 25 +++++++------------------ 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index ce010995763b5e9ae30cde5b8078f02d58789fdc..72db174d7256b0a5686bee0210bb5c37464bc97d 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -598,9 +598,8 @@ impl Room { .map_or(&[], |v| v.as_slice()) } - /// projects_to_join returns a list of shared projects sorted such - /// that the most 'active' projects appear last. - pub fn projects_to_join(&self) -> Vec<(u64, u64)> { + /// Returns the most 'active' projects, defined as most people in the project + pub fn most_active_project(&self) -> Option<(u64, u64)> { let mut projects = HashMap::default(); let mut hosts = HashMap::default(); for participant in self.remote_participants.values() { @@ -617,12 +616,11 @@ impl Room { } let mut pairs: Vec<(u64, usize)> = projects.into_iter().collect(); - pairs.sort_by_key(|(_, count)| 0 - *count as i32); + pairs.sort_by_key(|(_, count)| *count as i32); pairs - .into_iter() - .map(|(project_id, _)| (project_id, hosts[&project_id])) - .collect() + .first() + .map(|(project_id, _)| (*project_id, hosts[&project_id])) } async fn handle_room_updated( diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 6214eecac2adf119de13de5a00292b4167954973..893306710952dba0fd9bbd5561003043737f333f 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -3208,27 +3208,16 @@ impl CollabPanel { .update(&mut cx, |call, cx| call.join_channel(channel_id, cx)) .await?; - let tasks = room.update(&mut cx, |room, cx| { - let Some(workspace) = workspace.upgrade(cx) else { - return vec![]; - }; - let projects = room.projects_to_join(); - - if projects.is_empty() { - ChannelView::open(channel_id, workspace, cx).detach(); - return vec![]; - } - room.projects_to_join() - .into_iter() - .map(|(project_id, user_id)| { - let app_state = workspace.read(cx).app_state().clone(); - workspace::join_remote_project(project_id, user_id, app_state, cx) - }) - .collect() + let task = room.update(&mut cx, |room, cx| { + let workspace = workspace.upgrade(cx)?; + let (project, host) = room.most_active_project()?; + let app_state = workspace.read(cx).app_state().clone(); + Some(workspace::join_remote_project(project, host, app_state, cx)) }); - for task in tasks { + if let Some(task) = task { task.await?; } + anyhow::Ok(()) }) .detach_and_log_err(cx); From 4d61d01943f9c1e406d86359dbdf2aca3f0ebe86 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Wed, 4 Oct 2023 11:46:41 -0700 Subject: [PATCH 20/20] Add an RPC handler for channel buffer acks co-authored-by: max --- crates/channel/src/channel.rs | 2 +- crates/channel/src/channel_buffer.rs | 2 +- crates/collab/src/rpc.rs | 25 ++++++++++++++++--- .../collab/src/tests/channel_buffer_tests.rs | 21 +++++++++++----- crates/collab/src/tests/test_server.rs | 16 +++++++++++- crates/collab_ui/src/collab_panel.rs | 2 -- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/crates/channel/src/channel.rs b/crates/channel/src/channel.rs index 724ff75d60f01d75371b1c1286a4e46cca1b298a..160b8441ffd74f1ca835c70234fcbb166c7fa477 100644 --- a/crates/channel/src/channel.rs +++ b/crates/channel/src/channel.rs @@ -2,7 +2,7 @@ mod channel_buffer; mod channel_chat; mod channel_store; -pub use channel_buffer::{ChannelBuffer, ChannelBufferEvent}; +pub use channel_buffer::{ChannelBuffer, ChannelBufferEvent, ACKNOWLEDGE_DEBOUNCE_INTERVAL}; pub use channel_chat::{ChannelChat, ChannelChatEvent, ChannelMessage, ChannelMessageId}; pub use channel_store::{ Channel, ChannelData, ChannelEvent, ChannelId, ChannelMembership, ChannelPath, ChannelStore, diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index a097cc5467303e2f47f58dc892345c9d24fd5d20..7de8b956f1e897b4293274441689570f63780dcb 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -11,7 +11,7 @@ use rpc::{ use std::{sync::Arc, time::Duration}; use util::ResultExt; -const ACKNOWLEDGE_DEBOUNCE_INTERVAL: Duration = Duration::from_millis(250); +pub const ACKNOWLEDGE_DEBOUNCE_INTERVAL: Duration = Duration::from_millis(250); pub(crate) fn init(client: &Arc) { client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 1f0ecce2bf0d8e37a1af834d0ab607bf61cb016c..6171803341b2e3bb47d890a0882f786a0b28aa6a 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -3,8 +3,8 @@ mod connection_pool; use crate::{ auth, db::{ - self, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId, ServerId, User, - UserId, + self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId, + ServerId, User, UserId, }, executor::Executor, AppState, Result, @@ -275,7 +275,8 @@ impl Server { .add_message_handler(update_followers) .add_message_handler(update_diff_base) .add_request_handler(get_private_user_info) - .add_message_handler(acknowledge_channel_message); + .add_message_handler(acknowledge_channel_message) + .add_message_handler(acknowledge_buffer_version); Arc::new(server) } @@ -2912,6 +2913,24 @@ async fn acknowledge_channel_message( Ok(()) } +async fn acknowledge_buffer_version( + request: proto::AckBufferOperation, + session: Session, +) -> Result<()> { + let buffer_id = BufferId::from_proto(request.buffer_id); + session + .db() + .await + .observe_buffer_version( + buffer_id, + session.user_id, + request.epoch as i32, + &request.version, + ) + .await?; + Ok(()) +} + async fn join_channel_chat( request: proto::JoinChannelChat, response: Response, diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 82bd7e6afee7c39a758d1424885b6c746a7abddf..a0b9b524841f19e4eb6537348317ae38d23627c2 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -3,7 +3,7 @@ use crate::{ tests::TestServer, }; use call::ActiveCall; -use channel::Channel; +use channel::{Channel, ACKNOWLEDGE_DEBOUNCE_INTERVAL}; use client::ParticipantIndex; use client::{Collaborator, UserId}; use collab_ui::channel_view::ChannelView; @@ -800,7 +800,6 @@ async fn test_channel_buffer_changes( .has_channel_buffer_changed(channel_id) .unwrap() }); - assert!(has_buffer_changed); // Opening the buffer should clear the changed flag. @@ -810,7 +809,6 @@ async fn test_channel_buffer_changes( .update(|cx| ChannelView::open(channel_id, workspace_b.clone(), cx)) .await .unwrap(); - deterministic.run_until_parked(); let has_buffer_changed = cx_b.read(|cx| { @@ -820,10 +818,9 @@ async fn test_channel_buffer_changes( .has_channel_buffer_changed(channel_id) .unwrap() }); - assert!(!has_buffer_changed); - // Editing the channel while the buffer is open shuold not show that the buffer has changed. + // Editing the channel while the buffer is open should not show that the buffer has changed. channel_buffer_a.update(cx_a, |buffer, cx| { buffer.buffer().update(cx, |buffer, cx| { buffer.edit([(0..0, "2")], None, cx); @@ -838,7 +835,20 @@ async fn test_channel_buffer_changes( .has_channel_buffer_changed(channel_id) .unwrap() }); + assert!(!has_buffer_changed); + deterministic.advance_clock(ACKNOWLEDGE_DEBOUNCE_INTERVAL); + + // Test that the server is tracking things correctly, and we retain our 'not changed' + // state across a disconnect + server.simulate_long_connection_interruption(client_b.peer_id().unwrap(), &deterministic); + let has_buffer_changed = cx_b.read(|cx| { + client_b + .channel_store() + .read(cx) + .has_channel_buffer_changed(channel_id) + .unwrap() + }); assert!(!has_buffer_changed); // Closing the buffer should re-enable change tracking @@ -866,7 +876,6 @@ async fn test_channel_buffer_changes( .has_channel_buffer_changed(channel_id) .unwrap() }); - assert!(has_buffer_changed); } diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index cf5b58703cb95dbf255fb1a6d4ea2d163ad5b6d1..e10ded7d953f3872a3056a29940a7610db73de41 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -1,7 +1,7 @@ use crate::{ db::{tests::TestDb, NewUserParams, UserId}, executor::Executor, - rpc::{Server, CLEANUP_TIMEOUT}, + rpc::{Server, CLEANUP_TIMEOUT, RECONNECT_TIMEOUT}, AppState, }; use anyhow::anyhow; @@ -17,6 +17,7 @@ use gpui::{executor::Deterministic, ModelHandle, Task, TestAppContext, WindowHan use language::LanguageRegistry; use parking_lot::Mutex; use project::{Project, WorktreeId}; +use rpc::RECEIVE_TIMEOUT; use settings::SettingsStore; use std::{ cell::{Ref, RefCell, RefMut}, @@ -255,6 +256,19 @@ impl TestServer { .store(true, SeqCst); } + pub fn simulate_long_connection_interruption( + &self, + peer_id: PeerId, + deterministic: &Arc, + ) { + self.forbid_connections(); + self.disconnect_client(peer_id); + deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT); + self.allow_connections(); + deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT); + deterministic.run_until_parked(); + } + pub fn forbid_connections(&self) { self.forbid_connections.store(true, SeqCst); } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 893306710952dba0fd9bbd5561003043737f333f..39543c8defebfa99aa62067fd11479ef06c72da1 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -2760,11 +2760,9 @@ impl CollabPanel { .read(cx) .channel_id()?; - dbg!(call_channel, channel.id); Some(call_channel == channel.id) }) .unwrap_or(false); - dbg!(is_active); if is_active { self.open_channel_notes( &OpenChannelNotes {