Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve database and RPC API for moving and linking channels, improve test legibility

Browse source
This commit is contained in:
Mikayla 2023-09-09 18:20:14 -07:00
parent bf296ebbd7
commit 3a1f13645a
No known key found for this signature in database
8 changed files with 523 additions and 530 deletions
Download patch file Download diff file Expand all files Collapse all files

40
crates/channel/src/channel_store.rs
View file

@ -289,11 +289,43 @@ impl ChannelStore {
})
}
pub fn link_channel(
&mut self,
channel_id: ChannelId,
to: ChannelId,
cx: &mut ModelContext<Self>,
) -> Task<Result<()>> {
let client = self.client.clone();
cx.spawn(|_, _| async move {
let _ = client
.request(proto::LinkChannel { channel_id, to })
.await?;
Ok(())
})
}
pub fn unlink_channel(
&mut self,
channel_id: ChannelId,
from: Option<ChannelId>,
cx: &mut ModelContext<Self>,
) -> Task<Result<()>> {
let client = self.client.clone();
cx.spawn(|_, _| async move {
let _ = client
.request(proto::UnlinkChannel { channel_id, from })
.await?;
Ok(())
})
}
pub fn move_channel(
&mut self,
channel_id: ChannelId,
from_parent: Option<ChannelId>,
to: Option<ChannelId>,
from: Option<ChannelId>,
to: ChannelId,
cx: &mut ModelContext<Self>,
) -> Task<Result<()>> {
let client = self.client.clone();
@ -301,7 +333,7 @@ impl ChannelStore {
let _ = client
.request(proto::MoveChannel {
channel_id,
from_parent,
from,
to,
})
.await?;
@ -754,6 +786,4 @@ impl ChannelStore {
anyhow::Ok(())
}))
}
}

184
crates/collab/src/db/queries/channels.rs
View file

@ -761,20 +761,41 @@ impl Database {
.await
}
async fn link_channel(
// Insert an edge from the given channel to the given other channel.
pub async fn link_channel(
&self,
from: ChannelId,
user: UserId,
channel: ChannelId,
to: ChannelId,
) -> Result<Vec<Channel>> {
self.transaction(|tx| async move {
// Note that even with these maxed permissions, this linking operation
// is still insecure because you can't remove someone's permissions to a
// channel if they've linked the channel to one where they're an admin.
self.check_user_is_channel_admin(channel, user, &*tx)
.await?;
self.link_channel_internal(user, channel, to, &*tx).await
})
.await
}
pub async fn link_channel_internal(
&self,
user: UserId,
channel: ChannelId,
to: ChannelId,
tx: &DatabaseTransaction,
) -> Result<ChannelDescendants> {
) -> Result<Vec<Channel>> {
self.check_user_is_channel_admin(to, user, &*tx).await?;
let to_ancestors = self.get_channel_ancestors(to, &*tx).await?;
let from_descendants = self.get_channel_descendants([from], &*tx).await?;
let mut from_descendants = self.get_channel_descendants([channel], &*tx).await?;
for ancestor in to_ancestors {
if from_descendants.contains_key(&ancestor) {
return Err(anyhow!("Cannot create a channel cycle").into());
}
}
let sql = r#"
INSERT INTO channel_paths
(id_path, channel_id)
@ -790,14 +811,13 @@ impl Database {
self.pool.get_database_backend(),
sql,
[
from.to_proto().into(),
from.to_proto().into(),
channel.to_proto().into(),
channel.to_proto().into(),
to.to_proto().into(),
],
);
tx.execute(channel_paths_stmt).await?;
for (from_id, to_ids) in from_descendants.iter().filter(|(id, _)| id != &&from) {
for (from_id, to_ids) in from_descendants.iter().filter(|(id, _)| id != &&channel) {
for to_id in to_ids {
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
@ -812,94 +832,116 @@ impl Database {
}
}
Ok(from_descendants)
if let Some(channel) = from_descendants.get_mut(&channel) {
// Remove the other parents
channel.clear();
channel.insert(to);
}
let channels = self.get_all_channels(from_descendants, &*tx).await?;
Ok(channels)
}
async fn remove_channel_from_parent(
/// Unlink a channel from a given parent. This will add in a root edge if
/// the channel has no other parents after this operation.
pub async fn unlink_channel(
&self,
from: ChannelId,
parent: ChannelId,
user: UserId,
channel: ChannelId,
from: Option<ChannelId>,
) -> Result<()> {
self.transaction(|tx| async move {
// Note that even with these maxed permissions, this linking operation
// is still insecure because you can't remove someone's permissions to a
// channel if they've linked the channel to one where they're an admin.
self.check_user_is_channel_admin(channel, user, &*tx)
.await?;
self.unlink_channel_internal(user, channel, from, &*tx)
.await?;
Ok(())
})
.await
}
pub async fn unlink_channel_internal(
&self,
user: UserId,
channel: ChannelId,
from: Option<ChannelId>,
tx: &DatabaseTransaction,
) -> Result<()> {
let sql = r#"
if let Some(from) = from {
self.check_user_is_channel_admin(from, user, &*tx).await?;
let sql = r#"
DELETE FROM channel_paths
WHERE
id_path LIKE '%' || $1 || '/' || $2 || '%'
"#;
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
sql,
[from.to_proto().into(), channel.to_proto().into()],
);
tx.execute(channel_paths_stmt).await?;
} else {
let sql = r#"
DELETE FROM channel_paths
WHERE
id_path = '/' || $1 || '/'
"#;
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
sql,
[channel.to_proto().into()],
);
tx.execute(channel_paths_stmt).await?;
}
// Make sure that there is always at least one path to the channel
let sql = r#"
INSERT INTO channel_paths
(id_path, channel_id)
SELECT
'/' || $1 || '/', $2
WHERE NOT EXISTS
(SELECT *
FROM channel_paths
WHERE channel_id = $2)
"#;
let channel_paths_stmt = Statement::from_sql_and_values(
self.pool.get_database_backend(),
sql,
[parent.to_proto().into(), from.to_proto().into()],
[channel.to_proto().into(), channel.to_proto().into()],
);
tx.execute(channel_paths_stmt).await?;
Ok(())
}
/// Move a channel from one parent to another.
/// Note that this requires a valid parent_id in the 'from_parent' field.
/// As channels are a DAG, we need to know which parent to remove the channel from.
/// Here's a list of the parameters to this function and their behavior:
///
/// - (`None`, `None`) No op
/// - (`None`, `Some(id)`) Link the channel without removing it from any of it's parents
/// - (`Some(id)`, `None`) Remove a channel from a given parent, and leave other parents
/// - (`Some(id)`, `Some(id)`) Move channel from one parent to another, leaving other parents
///
/// Returns the channel that was moved + it's sub channels for use
/// by the members for `to`
/// Move a channel from one parent to another, returns the
/// Channels that were moved for notifying clients
pub async fn move_channel(
&self,
user: UserId,
from: ChannelId,
from_parent: Option<ChannelId>,
to: Option<ChannelId>,
channel: ChannelId,
from: Option<ChannelId>,
to: ChannelId,
) -> Result<Vec<Channel>> {
self.transaction(|tx| async move {
// Note that even with these maxed permissions, this linking operation
// is still insecure because you can't remove someone's permissions to a
// channel if they've linked the channel to one where they're an admin.
self.check_user_is_channel_admin(from, user, &*tx).await?;
self.check_user_is_channel_admin(channel, user, &*tx)
.await?;
let mut channel_descendants = None;
let moved_channels = self.link_channel_internal(user, channel, to, &*tx).await?;
// Note that we have to do the linking before the removal, so that we
// can leave the channel_path table in a consistent state.
if let Some(to) = to {
self.check_user_is_channel_admin(to, user, &*tx).await?;
self.unlink_channel_internal(user, channel, from, &*tx)
.await?;
channel_descendants = Some(self.link_channel(from, to, &*tx).await?);
}
let mut channel_descendants = match channel_descendants {
Some(channel_descendants) => channel_descendants,
None => self.get_channel_descendants([from], &*tx).await?,
};
if let Some(from_parent) = from_parent {
self.check_user_is_channel_admin(from_parent, user, &*tx)
.await?;
self.remove_channel_from_parent(from, from_parent, &*tx)
.await?;
}
let channels;
if let Some(to) = to {
if let Some(channel) = channel_descendants.get_mut(&from) {
// Remove the other parents
channel.clear();
channel.insert(to);
}
channels = self
.get_all_channels(channel_descendants, &*tx)
.await?;
} else {
channels = vec![];
}
Ok(channels)
Ok(moved_channels)
})
.await
}

595
crates/collab/src/db/tests/channel_tests.rs
View file

@ -1,7 +1,7 @@
use rpc::{proto, ConnectionId};
use crate::{
db::{Channel, Database, NewUserParams},
db::{Channel, ChannelId, Database, NewUserParams},
test_both_dbs,
};
use std::sync::Arc;
@ -501,50 +501,32 @@ async fn test_channels_moving(db: &Arc<Database>) {
.await
.unwrap();
// ========================================================================
// sanity check
// Initial DAG:
// /- gpui2
// zed -- crdb - livestreaming - livestreaming_dag
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
assert_dag(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
]
&[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
],
);
// Attempt to make a cycle
assert!(db
.move_channel(a_id, zed_id, None, Some(livestreaming_id))
.link_channel(a_id, zed_id, livestreaming_id)
.await
.is_err());
// ========================================================================
// Make a link
db.move_channel(a_id, livestreaming_id, None, Some(zed_id))
db.link_channel(a_id, livestreaming_id, zed_id)
.await
.unwrap();
@ -553,42 +535,16 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed -- crdb - livestreaming - livestreaming_dag
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
]);
// ========================================================================
// Create a new channel below a channel with multiple parents
let livestreaming_dag_sub_id = db
.create_channel(
@ -605,50 +561,20 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// Make a link
let channels = db
.move_channel(a_id, livestreaming_dag_sub_id, None, Some(livestreaming_id))
// ========================================================================
// Test a complex DAG by making another link
let returned_channels = db
.link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
.await
.unwrap();
@ -658,66 +584,32 @@ async fn test_channels_moving(db: &Arc<Database>) {
// \--------/
// make sure we're getting just the new link
// Not using the assert_dag helper because we want to make sure we're returning the full data
pretty_assertions::assert_eq!(
channels,
vec![
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_id),
}
]
returned_channels,
vec![Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_id),
}]
);
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// Make another link
let channels = db.move_channel(a_id, livestreaming_id, None, Some(gpui2_id))
// ========================================================================
// Test a complex DAG by making another link
let returned_channels = db
.link_channel(a_id, livestreaming_id, gpui2_id)
.await
.unwrap();
@ -727,62 +619,14 @@ async fn test_channels_moving(db: &Arc<Database>) {
// \---------/
// Make sure that we're correctly getting the full sub-dag
pretty_assertions::assert_eq!(channels,
vec![Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(gpui2_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
}]);
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
returned_channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(gpui2_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
@ -797,12 +641,31 @@ async fn test_channels_moving(db: &Arc<Database>) {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
}
]
);
// Remove that inner link
let channels = db.move_channel(a_id, livestreaming_dag_sub_id, Some(livestreaming_id), None)
let result = db.get_channels_for_user(a_id).await.unwrap();
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(gpui2_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// Test unlinking in a complex DAG by removing the inner link
db
.unlink_channel(
a_id,
livestreaming_dag_sub_id,
Some(livestreaming_id),
)
.await
.unwrap();
@ -811,62 +674,21 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
// \---------/
// Since we're not moving it to anywhere, there's nothing to notify anyone about
pretty_assertions::assert_eq!(
channels,
vec![]
);
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(gpui2_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(gpui2_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// Remove that outer link
db.move_channel(a_id, livestreaming_id, Some(gpui2_id), None)
// ========================================================================
// Test unlinking in a complex DAG by removing the inner link
db.unlink_channel(a_id, livestreaming_id, Some(gpui2_id))
.await
.unwrap();
@ -875,49 +697,19 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(crdb_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// Move livestreaming to be below gpui2
db.move_channel(a_id, livestreaming_id, Some(crdb_id), Some(gpui2_id))
// ========================================================================
// Test moving DAG nodes by moving livestreaming to be below gpui2
db.move_channel(a_id, livestreaming_id, Some(crdb_id), gpui2_id)
.await
.unwrap();
@ -926,47 +718,17 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb /
// \---------/
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: gpui2_id,
name: "gpui2".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(gpui2_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(gpui2_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(gpui2_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// Deleting a channel should not delete children that still have other parents
db.delete_channel(gpui2_id, a_id).await.unwrap();
@ -974,46 +736,109 @@ async fn test_channels_moving(db: &Arc<Database>) {
// zed - crdb
// \- livestreaming - livestreaming_dag - livestreaming_dag_sub
let result = db.get_channels_for_user(a_id).await.unwrap();
pretty_assertions::assert_eq!(
result.channels,
vec![
Channel {
id: zed_id,
name: "zed".to_string(),
parent_id: None,
},
Channel {
id: crdb_id,
name: "crdb".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_id,
name: "livestreaming".to_string(),
parent_id: Some(zed_id),
},
Channel {
id: livestreaming_dag_id,
name: "livestreaming_dag".to_string(),
parent_id: Some(livestreaming_id),
},
Channel {
id: livestreaming_dag_sub_id,
name: "livestreaming_dag_sub".to_string(),
parent_id: Some(livestreaming_dag_id),
},
]
);
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// But deleting a parent of a DAG should delete the whole DAG:
db.move_channel(a_id, livestreaming_id, None, Some(crdb_id))
// ========================================================================
// Unlinking a channel from it's parent should automatically promote it to a root channel
db.unlink_channel(a_id, crdb_id, Some(zed_id))
.await
.unwrap();
// DAG is now:
// crdb
// zed
// \- livestreaming - livestreaming_dag - livestreaming_dag_sub
let result = db.get_channels_for_user(a_id).await.unwrap();
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, None),
(livestreaming_id, Some(zed_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// Unlinking a root channel should not have any effect
db.unlink_channel(a_id, crdb_id, None)
.await
.unwrap();
// DAG is now:
// crdb
// zed
// \- livestreaming - livestreaming_dag - livestreaming_dag_sub
//
let result = db.get_channels_for_user(a_id).await.unwrap();
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, None),
(livestreaming_id, Some(zed_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// You should be able to move a root channel into a non-root channel
db.move_channel(a_id, crdb_id, None, zed_id)
.await
.unwrap();
// DAG is now:
// zed - crdb
// \- livestreaming - livestreaming_dag - livestreaming_dag_sub
let result = db.get_channels_for_user(a_id).await.unwrap();
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// Moving a non-root channel without a parent id should be the equivalent of a link operation
db.move_channel(a_id, livestreaming_id, None, crdb_id)
.await
.unwrap();
// DAG is now:
// zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub
// \--------/
let result = db.get_channels_for_user(a_id).await.unwrap();
assert_dag(result.channels, &[
(zed_id, None),
(crdb_id, Some(zed_id)),
(livestreaming_id, Some(zed_id)),
(livestreaming_id, Some(crdb_id)),
(livestreaming_dag_id, Some(livestreaming_id)),
(livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
]);
// ========================================================================
// Deleting a parent of a DAG should delete the whole DAG:
db.delete_channel(zed_id, a_id).await.unwrap();
let result = db.get_channels_for_user(a_id).await.unwrap();
assert!(result.channels.is_empty())
assert!(
result.channels.is_empty()
)
}
#[track_caller]
fn assert_dag(actual: Vec<Channel>, expected: &[(ChannelId, Option<ChannelId>)]) {
let actual = actual
.iter()
.map(|channel| (channel.id, channel.parent_id))
.collect::<Vec<_>>();
pretty_assertions::assert_eq!(actual, expected)
}

113
crates/collab/src/rpc.rs
View file

@ -2,7 +2,10 @@ mod connection_pool;
use crate::{
auth,
db::{self, ChannelId, ChannelsForUser, Database, ProjectId, RoomId, ServerId, User, UserId},
db::{
self, Channel, ChannelId, ChannelsForUser, Database, ProjectId, RoomId, ServerId, User,
UserId,
},
executor::Executor,
AppState, Result,
};
@ -255,6 +258,8 @@ impl Server {
.add_request_handler(get_channel_members)
.add_request_handler(respond_to_channel_invite)
.add_request_handler(join_channel)
.add_request_handler(link_channel)
.add_request_handler(unlink_channel)
.add_request_handler(move_channel)
.add_request_handler(follow)
.add_message_handler(unfollow)
@ -2388,6 +2393,72 @@ async fn rename_channel(
Ok(())
}
async fn link_channel(
request: proto::LinkChannel,
response: Response<proto::LinkChannel>,
session: Session,
) -> Result<()> {
let db = session.db().await;
let channel_id = ChannelId::from_proto(request.channel_id);
let to = ChannelId::from_proto(request.to);
let channels_to_send = db.link_channel(session.user_id, channel_id, to).await?;
let members = db.get_channel_members(to).await?;
let connection_pool = session.connection_pool().await;
let update = proto::UpdateChannels {
channels: channels_to_send
.into_iter()
.map(|channel| proto::Channel {
id: channel.id.to_proto(),
name: channel.name,
parent_id: channel.parent_id.map(ChannelId::to_proto),
})
.collect(),
..Default::default()
};
for member_id in members {
for connection_id in connection_pool.user_connection_ids(member_id) {
session.peer.send(connection_id, update.clone())?;
}
}
response.send(Ack {})?;
Ok(())
}
async fn unlink_channel(
request: proto::UnlinkChannel,
response: Response<proto::UnlinkChannel>,
session: Session,
) -> Result<()> {
let db = session.db().await;
let channel_id = ChannelId::from_proto(request.channel_id);
let from = request.from.map(ChannelId::from_proto);
db.unlink_channel(session.user_id, channel_id, from).await?;
if let Some(from_parent) = from {
let members = db.get_channel_members(from_parent).await?;
let update = proto::UpdateChannels {
delete_channel_edge: vec![proto::ChannelEdge {
channel_id: channel_id.to_proto(),
parent_id: from_parent.to_proto(),
}],
..Default::default()
};
let connection_pool = session.connection_pool().await;
for member_id in members {
for connection_id in connection_pool.user_connection_ids(member_id) {
session.peer.send(connection_id, update.clone())?;
}
}
}
response.send(Ack {})?;
Ok(())
}
async fn move_channel(
request: proto::MoveChannel,
response: Response<proto::MoveChannel>,
@ -2395,18 +2466,12 @@ async fn move_channel(
) -> Result<()> {
let db = session.db().await;
let channel_id = ChannelId::from_proto(request.channel_id);
let from_parent = request.from_parent.map(ChannelId::from_proto);
let to = request.to.map(ChannelId::from_proto);
let channels_to_send = db
.move_channel(
session.user_id,
channel_id,
from_parent,
to,
)
let from_parent = request.from.map(ChannelId::from_proto);
let to = ChannelId::from_proto(request.to);
let channels_to_send: Vec<Channel> = db
.move_channel(session.user_id, channel_id, from_parent, to)
.await?;
if let Some(from_parent) = from_parent {
let members = db.get_channel_members(from_parent).await?;
let update = proto::UpdateChannels {
@ -2422,24 +2487,24 @@ async fn move_channel(
session.peer.send(connection_id, update.clone())?;
}
}
}
if let Some(to) = to {
let members = db.get_channel_members(to).await?;
let connection_pool = session.connection_pool().await;
let update = proto::UpdateChannels {
channels: channels_to_send.into_iter().map(|channel| proto::Channel {
let members = db.get_channel_members(to).await?;
let connection_pool = session.connection_pool().await;
let update = proto::UpdateChannels {
channels: channels_to_send
.into_iter()
.map(|channel| proto::Channel {
id: channel.id.to_proto(),
name: channel.name,
parent_id: channel.parent_id.map(ChannelId::to_proto),
}).collect(),
..Default::default()
};
for member_id in members {
for connection_id in connection_pool.user_connection_ids(member_id) {
session.peer.send(connection_id, update.clone())?;
}
})
.collect(),
..Default::default()
};
for member_id in members {
for connection_id in connection_pool.user_connection_ids(member_id) {
session.peer.send(connection_id, update.clone())?;
}
}

4
crates/collab/src/tests/channel_tests.rs
View file

@ -933,7 +933,7 @@ async fn test_channel_moving(deterministic: Arc<Deterministic>, cx_a: &mut TestA
client_a
.channel_store()
.update(cx_a, |channel_store, cx| {
channel_store.move_channel(channel_c_id, Some(channel_b_id), Some(channel_a_id), cx)
channel_store.move_channel(channel_c_id, Some(channel_b_id), channel_a_id, cx)
})
.await
.unwrap();
@ -970,7 +970,7 @@ async fn test_channel_moving(deterministic: Arc<Deterministic>, cx_a: &mut TestA
client_a
.channel_store()
.update(cx_a, |channel_store, cx| {
channel_store.move_channel(channel_c_id, None, Some(channel_b_id), cx)
channel_store.link_channel(channel_c_id, channel_b_id, cx)
})
.await
.unwrap();

95
crates/collab_ui/src/collab_panel.rs
View file

@ -106,7 +106,7 @@ struct PutChannel {
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
struct UnlinkChannel {
channel_id: ChannelId,
parent_id: ChannelId,
parent_id: Option<ChannelId>,
}
actions!(
@ -188,13 +188,13 @@ pub fn init(_client: Arc<Client>, cx: &mut AppContext) {
cx.add_action(
|panel: &mut CollabPanel, action: &LinkChannel, _: &mut ViewContext<CollabPanel>| {
panel.copy = Some(ChannelCopy::Link(action.channel_id));
panel.link_or_move = Some(ChannelCopy::Link(action.channel_id));
},
);
cx.add_action(
|panel: &mut CollabPanel, action: &MoveChannel, _: &mut ViewContext<CollabPanel>| {
panel.copy = Some(ChannelCopy::Move {
panel.link_or_move = Some(ChannelCopy::Move {
channel_id: action.channel_id,
parent_id: action.parent_id,
});
@ -203,20 +203,20 @@ pub fn init(_client: Arc<Client>, cx: &mut AppContext) {
cx.add_action(
|panel: &mut CollabPanel, action: &PutChannel, cx: &mut ViewContext<CollabPanel>| {
if let Some(copy) = panel.copy.take() {
if let Some(copy) = panel.link_or_move.take() {
match copy {
ChannelCopy::Move {
channel_id,
parent_id,
} => panel.channel_store.update(cx, |channel_store, cx| {
channel_store
.move_channel(channel_id, parent_id, Some(action.to), cx)
.move_channel(channel_id, parent_id, action.to, cx)
.detach_and_log_err(cx)
}),
ChannelCopy::Link(channel) => {
panel.channel_store.update(cx, |channel_store, cx| {
channel_store
.move_channel(channel, None, Some(action.to), cx)
.link_channel(channel, action.to, cx)
.detach_and_log_err(cx)
})
}
@ -229,7 +229,7 @@ pub fn init(_client: Arc<Client>, cx: &mut AppContext) {
|panel: &mut CollabPanel, action: &UnlinkChannel, cx: &mut ViewContext<CollabPanel>| {
panel.channel_store.update(cx, |channel_store, cx| {
channel_store
.move_channel(action.channel_id, Some(action.parent_id), None, cx)
.unlink_channel(action.channel_id, action.parent_id, cx)
.detach_and_log_err(cx)
})
},
@ -273,13 +273,20 @@ impl ChannelCopy {
ChannelCopy::Link(channel_id) => *channel_id,
}
}
fn is_move(&self) -> bool {
match self {
ChannelCopy::Move { .. } => true,
ChannelCopy::Link(_) => false,
}
}
}
pub struct CollabPanel {
width: Option<f32>,
fs: Arc<dyn Fs>,
has_focus: bool,
copy: Option<ChannelCopy>,
link_or_move: Option<ChannelCopy>,
pending_serialization: Task<Option<()>>,
context_menu: ViewHandle<ContextMenu>,
filter_editor: ViewHandle<Editor>,
@ -549,7 +556,7 @@ impl CollabPanel {
let mut this = Self {
width: None,
has_focus: false,
copy: None,
link_or_move: None,
fs: workspace.app_state().fs.clone(),
pending_serialization: Task::ready(None),
context_menu: cx.add_view(|cx| ContextMenu::new(view_id, cx)),
@ -2034,15 +2041,14 @@ impl CollabPanel {
) {
self.context_menu_on_selected = position.is_none();
let copy_channel = self
.copy
.as_ref()
.and_then(|copy| {
self.channel_store
.read(cx)
.channel_for_id(copy.channel_id())
})
.map(|channel| channel.name.clone());
let operation_details = self.link_or_move.as_ref().and_then(|link_or_move| {
let channel_name = self
.channel_store
.read(cx)
.channel_for_id(link_or_move.channel_id())
.map(|channel| channel.name.clone())?;
Some((channel_name, link_or_move.is_move()))
});
self.context_menu.update(cx, |context_menu, cx| {
context_menu.set_position_mode(if self.context_menu_on_selected {
@ -2051,13 +2057,29 @@ impl CollabPanel {
OverlayPositionMode::Window
});
let mut items = Vec::new();
if let Some((channel_name, is_move)) = operation_details {
items.push(ContextMenuItem::action(
format!(
"{} '#{}' here",
if is_move { "Move" } else { "Link" },
channel_name
),
PutChannel {
to: location.channel,
},
));
items.push(ContextMenuItem::Separator)
}
let expand_action_name = if self.is_channel_collapsed(&location) {
"Expand Subchannels"
} else {
"Collapse Subchannels"
};
let mut items = vec![
items.extend([
ContextMenuItem::action(
expand_action_name,
ToggleCollapse {
@ -2070,7 +2092,7 @@ impl CollabPanel {
channel_id: location.channel,
},
),
];
]);
if self.channel_store.read(cx).is_user_admin(location.channel) {
let parent_id = location.path.parent_id();
@ -2092,25 +2114,27 @@ impl CollabPanel {
ContextMenuItem::Separator,
]);
if let Some(parent) = parent_id {
items.push(ContextMenuItem::action(
"Unlink from parent",
UnlinkChannel {
channel_id: location.channel,
parent_id: parent,
},
))
}
items.push(ContextMenuItem::action(
if parent_id.is_some() {
"Unlink from parent"
} else {
"Unlink from root"
},
UnlinkChannel {
channel_id: location.channel,
parent_id,
},
));
items.extend([
ContextMenuItem::action(
"Link to new parent",
"Link this channel",
LinkChannel {
channel_id: location.channel,
},
),
ContextMenuItem::action(
"Move",
"Move this channel",
MoveChannel {
channel_id: location.channel,
parent_id,
@ -2118,15 +2142,6 @@ impl CollabPanel {
),
]);
if let Some(copy_channel) = copy_channel {
items.push(ContextMenuItem::action(
format!("Put '#{}'", copy_channel),
PutChannel {
to: location.channel,
},
));
}
items.extend([
ContextMenuItem::Separator,
ContextMenuItem::action(

18
crates/rpc/proto/zed.proto
View file

@ -156,7 +156,9 @@ message Envelope {
UpdateChannelBufferCollaborator update_channel_buffer_collaborator = 139;
RejoinChannelBuffers rejoin_channel_buffers = 140;
RejoinChannelBuffersResponse rejoin_channel_buffers_response = 141;
MoveChannel move_channel = 142; // Current max
LinkChannel link_channel = 142;
UnlinkChannel unlink_channel = 143;
MoveChannel move_channel = 144; // Current max
}
}
@ -1028,10 +1030,20 @@ message RenameChannel {
string name = 2;
}
message LinkChannel {
uint64 channel_id = 1;
uint64 to = 2;
}
message UnlinkChannel {
uint64 channel_id = 1;
optional uint64 from = 2;
}
message MoveChannel {
uint64 channel_id = 1;
optional uint64 from_parent = 2;
optional uint64 to = 3;
optional uint64 from = 2;
uint64 to = 3;
}
message JoinChannelBuffer {

4
crates/rpc/src/proto.rs
View file

@ -239,6 +239,8 @@ messages!(
(UpdateContacts, Foreground),
(DeleteChannel, Foreground),
(MoveChannel, Foreground),
(LinkChannel, Foreground),
(UnlinkChannel, Foreground),
(UpdateChannels, Foreground),
(UpdateDiagnosticSummary, Foreground),
(UpdateFollowers, Foreground),
@ -319,6 +321,8 @@ request_messages!(
(DeleteChannel, Ack),
(RenameProjectEntry, ProjectEntryResponse),
(RenameChannel, ChannelResponse),
(LinkChannel, Ack),
(UnlinkChannel, Ack),
(MoveChannel, Ack),
(SaveBuffer, BufferSaved),
(SearchProject, SearchProjectResponse),