Skip to content

Commit

Permalink
[uniffi] Remove GroupState type from storage trait (#128)
Browse files Browse the repository at this point in the history
This removes the `GroupState` type from the `GroupStateStorage` trait
in mls-rs-uniffi. This is done for two reasons:

- It makes all methods on the trait consistent in the way that they
  take a `group_id` as the first argument.

- Fewer types means less boiler-plate generated by the UniFFI bindings
  generator (`Vec<u8>` is a built-in type in UniFFI).

The `EpochRecord` type stays: I don’t think we have a different way of
passing in a vector of epochs IDs with their data.

I could apply this change to mls-rs too, but it doesn’t seem useful
there since it’s cheap and easy to create a new type in Rust.
  • Loading branch information
mgeisler authored Mar 21, 2024
1 parent e4009f9 commit 5b83cac
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 36 deletions.
3 changes: 2 additions & 1 deletion mls-rs-uniffi/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ impl mls_rs_core::group::GroupStateStorage for ClientGroupStorage {
updates: Vec<mls_rs_core::group::EpochRecord>,
) -> Result<(), Self::Error> {
self.0.write(
state.into(),
state.id,
state.data,
inserts.into_iter().map(Into::into).collect(),
updates.into_iter().map(Into::into).collect(),
)
Expand Down
36 changes: 10 additions & 26 deletions mls-rs-uniffi/src/config/group_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,18 @@ use std::sync::Mutex;

use crate::Error;

// TODO(mulmarta): we'd like to use GroupState and EpochRecord from mls-rs-core
// but this breaks python tests because using 2 crates makes uniffi generate
// a python module which must be in a subdirectory of the directory with test scripts
// which is not supported by the script we use.
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, uniffi::Record)]
pub struct GroupState {
/// A unique group identifier.
pub id: Vec<u8>,
pub data: Vec<u8>,
}

// TODO(mulmarta): we'd like to use EpochRecord from mls-rs-core but
// this breaks the Python tests because using two crates makes UniFFI
// generate a Python module which must be in a subdirectory of the
// directory with test scripts which is not supported by the script we
// use.
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, uniffi::Record)]
pub struct EpochRecord {
/// A unique epoch identifier within a particular group.
pub id: u64,
pub data: Vec<u8>,
}

impl From<mls_rs_core::group::GroupState> for GroupState {
fn from(mls_rs_core::group::GroupState { id, data }: mls_rs_core::group::GroupState) -> Self {
Self { id, data }
}
}

impl From<GroupState> for mls_rs_core::group::GroupState {
fn from(GroupState { id, data }: GroupState) -> Self {
Self { id, data }
}
}

impl From<mls_rs_core::group::EpochRecord> for EpochRecord {
fn from(mls_rs_core::group::EpochRecord { id, data }: mls_rs_core::group::EpochRecord) -> Self {
Self { id, data }
Expand All @@ -55,7 +37,8 @@ pub trait GroupStateStorage: Send + Sync + Debug {

async fn write(
&self,
state: GroupState,
group_id: Vec<u8>,
group_state: Vec<u8>,
epoch_inserts: Vec<EpochRecord>,
epoch_updates: Vec<EpochRecord>,
) -> Result<(), Error>;
Expand Down Expand Up @@ -102,13 +85,14 @@ where

async fn write(
&self,
state: GroupState,
id: Vec<u8>,
data: Vec<u8>,
epoch_inserts: Vec<EpochRecord>,
epoch_updates: Vec<EpochRecord>,
) -> Result<(), Error> {
self.inner()
.write(
state.into(),
mls_rs_core::group::GroupState { id, data },
epoch_inserts.into_iter().map(Into::into).collect(),
epoch_updates.into_iter().map(Into::into).collect(),
)
Expand Down
9 changes: 5 additions & 4 deletions mls-rs-uniffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ impl Group {
#[cfg(test)]
mod tests {
use super::*;
use crate::config::group_state::{EpochRecord, GroupState, GroupStateStorage};
use crate::config::group_state::{EpochRecord, GroupStateStorage};
use std::collections::HashMap;

#[test]
Expand Down Expand Up @@ -754,14 +754,15 @@ mod tests {

fn write(
&self,
state: GroupState,
group_id: Vec<u8>,
group_state: Vec<u8>,
epoch_inserts: Vec<EpochRecord>,
epoch_updates: Vec<EpochRecord>,
) -> Result<(), Error> {
let mut groups = self.lock();

let group = groups.entry(state.id).or_default();
group.state = state.data;
let group = groups.entry(group_id).or_default();
group.state = group_state;
for insert in epoch_inserts {
group.epoch_data.push(insert);
}
Expand Down
10 changes: 5 additions & 5 deletions mls-rs-uniffi/tests/simple_scenario_sync.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from dataclasses import dataclass, field

from mls_rs_uniffi import CipherSuite, generate_signature_keypair, Client, \
GroupStateStorage, GroupState, EpochRecord, ClientConfig, ProtocolVersion
GroupStateStorage, EpochRecord, ClientConfig, ProtocolVersion

@dataclass
class GroupStateData:
Expand Down Expand Up @@ -32,11 +32,11 @@ def epoch(self, group_id: bytes, epoch_id: int):

return None

def write(self, state: GroupState, epoch_inserts: list[EpochRecord], epoch_updates: list[EpochRecord]):
if self.groups.get(state.id.hex()) == None:
self.groups[state.id.hex()] = GroupStateData(state.data)
def write(self, group_id: bytes, group_state: bytes, epoch_inserts: list[EpochRecord], epoch_updates: list[EpochRecord]):
if self.groups.get(group_id.hex()) == None:
self.groups[group_id.hex()] = GroupStateData(group_state)

group = self.groups[state.id.hex()]
group = self.groups[group_id.hex()]

for insert in epoch_inserts:
group.epoch_data.append(insert)
Expand Down

0 comments on commit 5b83cac

Please sign in to comment.