From e9c1ad6acd9b1ae6750ef958be237e6d05b1f6d7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 14:21:44 -0700 Subject: [PATCH] Undo making project optional on stored follower states Following works without a project, but following in unshared projects does not need to be replicated to other participants. --- crates/call/src/call.rs | 2 +- crates/call/src/room.rs | 4 +- .../20221109000000_test_schema.sql | 2 +- ...142700_allow_following_without_project.sql | 1 - crates/collab/src/db/queries/projects.rs | 93 +++++-------------- crates/collab/src/db/queries/rooms.rs | 2 +- crates/collab/src/db/tables/follower.rs | 2 +- crates/collab/src/rpc.rs | 32 ++++--- crates/rpc/proto/zed.proto | 2 +- 9 files changed, 47 insertions(+), 93 deletions(-) delete mode 100644 crates/collab/migrations/20230918142700_allow_following_without_project.sql diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index 8c570c7165..6756c2aa53 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -58,7 +58,7 @@ pub struct ActiveCall { _subscriptions: Vec, } -#[derive(PartialEq, Eq, PartialOrd, Ord)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] struct Follower { project_id: Option, peer_id: PeerId, diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index bf30e31a98..26a531cc31 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -62,7 +62,7 @@ pub struct Room { leave_when_empty: bool, client: Arc, user_store: ModelHandle, - follows_by_leader_id_project_id: HashMap<(PeerId, Option), Vec>, + follows_by_leader_id_project_id: HashMap<(PeerId, u64), Vec>, subscriptions: Vec, pending_room_update: Option>, maintain_connection: Option>>, @@ -584,7 +584,7 @@ impl Room { pub fn followers_for(&self, leader_id: PeerId, project_id: u64) -> &[PeerId] { self.follows_by_leader_id_project_id - .get(&(leader_id, Some(project_id))) + .get(&(leader_id, project_id)) .map_or(&[], |v| v.as_slice()) } diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 8a153949c2..d8325755f8 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -176,7 +176,7 @@ CREATE TABLE "servers" ( CREATE TABLE "followers" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, "room_id" INTEGER NOT NULL REFERENCES rooms (id) ON DELETE CASCADE, - "project_id" INTEGER REFERENCES projects (id) ON DELETE CASCADE, + "project_id" INTEGER NOT NULL REFERENCES projects (id) ON DELETE CASCADE, "leader_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, "leader_connection_id" INTEGER NOT NULL, "follower_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, diff --git a/crates/collab/migrations/20230918142700_allow_following_without_project.sql b/crates/collab/migrations/20230918142700_allow_following_without_project.sql deleted file mode 100644 index e0cc0141ec..0000000000 --- a/crates/collab/migrations/20230918142700_allow_following_without_project.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE followers ALTER COLUMN project_id DROP NOT NULL; diff --git a/crates/collab/src/db/queries/projects.rs b/crates/collab/src/db/queries/projects.rs index 80e71eb1eb..3e2c003378 100644 --- a/crates/collab/src/db/queries/projects.rs +++ b/crates/collab/src/db/queries/projects.rs @@ -862,83 +862,34 @@ impl Database { .await } - pub async fn check_can_follow( + pub async fn check_room_participants( &self, room_id: RoomId, - project_id: Option, - leader_id: ConnectionId, - follower_id: ConnectionId, - ) -> Result<()> { - let mut found_leader = false; - let mut found_follower = false; - self.transaction(|tx| async move { - if let Some(project_id) = project_id { - let mut rows = project_collaborator::Entity::find() - .filter(project_collaborator::Column::ProjectId.eq(project_id)) - .stream(&*tx) - .await?; - while let Some(row) = rows.next().await { - let row = row?; - let connection = row.connection(); - if connection == leader_id { - found_leader = true; - } else if connection == follower_id { - found_follower = true; - } - } - } else { - let mut rows = room_participant::Entity::find() - .filter(room_participant::Column::RoomId.eq(room_id)) - .stream(&*tx) - .await?; - while let Some(row) = rows.next().await { - let row = row?; - if let Some(connection) = row.answering_connection() { - if connection == leader_id { - found_leader = true; - } else if connection == follower_id { - found_follower = true; - } - } - } - } - - if !found_leader || !found_follower { - Err(anyhow!("not a room participant"))?; - } - - Ok(()) - }) - .await - } - - pub async fn check_can_unfollow( - &self, - room_id: RoomId, - project_id: Option, leader_id: ConnectionId, follower_id: ConnectionId, ) -> Result<()> { self.transaction(|tx| async move { - follower::Entity::find() + use room_participant::Column; + + let count = room_participant::Entity::find() .filter( - Condition::all() - .add(follower::Column::RoomId.eq(room_id)) - .add(follower::Column::ProjectId.eq(project_id)) - .add(follower::Column::LeaderConnectionId.eq(leader_id.id as i32)) - .add(follower::Column::FollowerConnectionId.eq(follower_id.id as i32)) - .add( - follower::Column::LeaderConnectionServerId - .eq(leader_id.owner_id as i32), - ) - .add( - follower::Column::FollowerConnectionServerId - .eq(follower_id.owner_id as i32), - ), + Condition::all().add(Column::RoomId.eq(room_id)).add( + Condition::any() + .add(Column::AnsweringConnectionId.eq(leader_id.id as i32).and( + Column::AnsweringConnectionServerId.eq(leader_id.owner_id as i32), + )) + .add(Column::AnsweringConnectionId.eq(follower_id.id as i32).and( + Column::AnsweringConnectionServerId.eq(follower_id.owner_id as i32), + )), + ), ) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("not a follower"))?; + .count(&*tx) + .await?; + + if count < 2 { + Err(anyhow!("not room participants"))?; + } + Ok(()) }) .await @@ -947,7 +898,7 @@ impl Database { pub async fn follow( &self, room_id: RoomId, - project_id: Option, + project_id: ProjectId, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { @@ -977,7 +928,7 @@ impl Database { pub async fn unfollow( &self, room_id: RoomId, - project_id: Option, + project_id: ProjectId, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index de1e8a1ce7..41f9755872 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -1154,7 +1154,7 @@ impl Database { followers.push(proto::Follower { leader_id: Some(db_follower.leader_connection().into()), follower_id: Some(db_follower.follower_connection().into()), - project_id: db_follower.project_id.map(|id| id.to_proto()), + project_id: db_follower.project_id.to_proto(), }); } diff --git a/crates/collab/src/db/tables/follower.rs b/crates/collab/src/db/tables/follower.rs index b5bc163b21..ffd45434e9 100644 --- a/crates/collab/src/db/tables/follower.rs +++ b/crates/collab/src/db/tables/follower.rs @@ -8,7 +8,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: FollowerId, pub room_id: RoomId, - pub project_id: Option, + pub project_id: ProjectId, pub leader_connection_server_id: ServerId, pub leader_connection_id: i32, pub follower_connection_server_id: ServerId, diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 56cecb2e74..9e14c48473 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1901,7 +1901,7 @@ async fn follow( session .db() .await - .check_can_follow(room_id, project_id, leader_id, session.connection_id) + .check_room_participants(room_id, leader_id, session.connection_id) .await?; let mut response_payload = session @@ -1913,12 +1913,14 @@ async fn follow( .retain(|view| view.leader_id != Some(follower_id.into())); response.send(response_payload)?; - let room = session - .db() - .await - .follow(room_id, project_id, leader_id, follower_id) - .await?; - room_updated(&room, &session.peer); + if let Some(project_id) = project_id { + let room = session + .db() + .await + .follow(room_id, project_id, leader_id, follower_id) + .await?; + room_updated(&room, &session.peer); + } Ok(()) } @@ -1935,19 +1937,21 @@ async fn unfollow(request: proto::Unfollow, session: Session) -> Result<()> { session .db() .await - .check_can_unfollow(room_id, project_id, leader_id, session.connection_id) + .check_room_participants(room_id, leader_id, session.connection_id) .await?; session .peer .forward_send(session.connection_id, leader_id, request)?; - let room = session - .db() - .await - .unfollow(room_id, project_id, leader_id, follower_id) - .await?; - room_updated(&room, &session.peer); + if let Some(project_id) = project_id { + let room = session + .db() + .await + .unfollow(room_id, project_id, leader_id, follower_id) + .await?; + room_updated(&room, &session.peer); + } Ok(()) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 523ef1d763..a62a9f06c3 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -273,7 +273,7 @@ message ParticipantProject { message Follower { PeerId leader_id = 1; PeerId follower_id = 2; - optional uint64 project_id = 3; + uint64 project_id = 3; } message ParticipantLocation {