Add groups storage #88
Conversation
|
Awesome addition, it was on my todo list forever 😄 |
examples/cli.rs
Outdated
| println!("{:?} {:?}",key, title); | ||
| } | ||
| Err(e) => { | ||
| println!("Error: {:?}", e); |
There was a problem hiding this comment.
Maybe use the error! macro here instead? And avoid using the Debug representation of the error as well.
There was a problem hiding this comment.
Actually i would love to use log::Error and log::Info or error! but they don't print anything to my console, so I ended up with println! statements. Regarding the {:?}, I still have to read some basics about the representations.
There was a problem hiding this comment.
the macro error! isn't defined and i changed it to an error representation.
examples/cli.rs
Outdated
| let manager = Manager::load_registered(config_store)?; | ||
| let group = manager.get_group_v2(group_master_key).await?; | ||
| let group = manager.get_group_v2(master_key).await?; |
There was a problem hiding this comment.
I'm not super sure why you'd have to do it like this? Is it because you want to use the master key as a string key in the storage backend?
There was a problem hiding this comment.
The proto::group has no field for the master_key, so how should we store the masterkey? Either by adding a new field or using it as key in the tree.
There was a problem hiding this comment.
I am unsure, it was just a shot in the dark. First i looked into the groups_v2::Group but this doesn't serialize into json or a protobuf, so I tried it this way.
examples/cli.rs
Outdated
| println!("{:#?}", DebugGroup(&group)); | ||
| for member in &group.members { | ||
| let profile_key = base64::encode(&member.profile_key.bytes); | ||
| println!("{member:#?} => profile_key = {profile_key}",); | ||
| } | ||
| manager.save_group(group, key.as_bytes().to_vec())?; |
There was a problem hiding this comment.
I'm pretty sure you can get the bytes representation from the GroupMasterKey object directly as well, also without allocating a new Vec.
There was a problem hiding this comment.
503 | manager.save_group(group, master_key.into())?;
| ^^^^ the trait From<GroupMasterKey> is not implemented for Vec<u8>
src/manager.rs
Outdated
| @@ -811,6 +817,26 @@ impl<C: Store> Manager<C, Registered> { | |||
| Ok(group_changes) | |||
| } | |||
|
|
|||
| pub fn save_group(&self, group:Group, key:Vec<u8>) -> Result<(), Error> { | |||
There was a problem hiding this comment.
| pub fn save_group(&self, group:Group, key:Vec<u8>) -> Result<(), Error> { | |
| pub fn save_group(&self, key: impl AsRef<[u8]>, group: Group) -> Result<(), Error> { |
There was a problem hiding this comment.
here we have to rethink, it a little. I set the public_key: key which needs an vector. I misuse that field to pass the group id to self.config_store.save_group(proto_group)?; We have to either make a struct à la {key, group} or we need to pass the key as second argument.
src/proto.rs
Outdated
| @@ -110,3 +111,26 @@ impl TryInto<Content> for ContentProto { | |||
| Content::from_proto(self.content, self.metadata.into()).map_err(Error::from) | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Clone, PartialEq, ::prost::Message)] | |||
There was a problem hiding this comment.
Maybe I'm confused but I feel like we don't need this wrapper to serialize the group?
|
Some minor comments from my side:
Otherwise this |
|
Actually should we add the group sync here or in another pr? |
I would take care of this separately, this is already working fine fetching groups when you receive messages. |
This implements a group store and saves the groups as protobuf messages in the store.