improve design for apis with transaction

[darkskygit]

Currently all workspace/block apis require transaction, we need call the api with transaction like block.xxx(&trx, ..)

Consider providing the Workspace/Block type encapsulated by with_trx, which includes trx:

struct WrappedBlock {
  block: Block, // original block type
  trx: Transact,// transaction
}

struct WrappedWorkspace {
  workspace: Workspace, // original workspace type
  trx: Transact,// transaction
}

workspace.with_trx(|ws: WrappedWorkspace| {
  let block: WrappedBlock = ws.get(&block_id);
  block.set("key", value);
});

[fundon]

We can add two structs: Transaction<'doc, T> and TransactionMut<'doc, T>

pub struct Transaction<'doc, T> {
    inner: &'doc T,
    txn: yrs::Transaction<'doc>,
}

impl<'doc, T> Transaction<'doc, T> {
    pub fn new(inner: &'doc T, txn: yrs::Transaction<'doc>) -> Self {
        Self { inner, txn }
    }
}

pub struct TransactionMut<'doc, T> {
    inner: &'doc T,
    txn: yrs::TransactionMut<'doc>,
}

impl<'doc, T> TransactionMut<'doc, T> {
    pub fn new(inner: &'doc T, txn: yrs::TransactionMut<'doc>) -> Self {
        Self { inner, txn }
    }

    pub fn encode_update_v1(&self) -> Vec<u8> {
        self.txn.encode_update_v1()
    }
}

impl<'doc> TransactionMut<'doc, Workspace> {
    pub fn create<B, F>(&mut self, block_id: B, flavor: F) -> Block
    where
        B: AsRef<str>,
        F: AsRef<str>,
    {
        info!("create block: {}", block_id.as_ref());
        Block::new(
            &mut self.txn,
            self.inner,
            block_id,
            flavor,
            self.inner.client_id(),
        )
    }
}

impl<'doc> TransactionMut<'doc, Block> {
    pub fn set<K, T>(&mut self, key: K, value: T)
    where
        K: AsRef<str>,
        T: Into<Any>,
    {
        self.inner.set(&mut self.txn, key.as_ref(), value)
    }
}

And add tow methods to Workspace and Block

    pub fn with_transact<T>(&self, f: impl FnOnce(crate::Transaction<'_, Self>) -> T) -> T {
        f(crate::Transaction::new(self, self.doc.transact()))
    }

    pub fn with_transact_mut<T>(&self, f: impl FnOnce(crate::TransactionMut<'_, Self>) -> T) -> T {
        f(crate::TransactionMut::new(self, self.doc.transact_mut()))
    }

If we want to add Block to Workspace, we can do this:

                workspace.with_transact_mut(|mut t| {
                    let block = t.create(&block_id, "text");

                    let count = block.with_transact_mut(|mut t| {
                        payload
                            .into_iter()
                            .filter_map(|(key, value)| {
                                serde_json::from_value::<Any>(value)
                                    .map(|value| t.set(&key, value))
                                    .ok()
                            })
                            .count()
                    });

                    Some(((count > 0).then(|| t.encode_update_v1()), block))
                })

[fundon]

And add tow methods to Workspace and Block

    pub fn with_transact<T>(&self, f: impl FnOnce(crate::Transaction<'_, Self>) -> T) -> T {
        f(crate::Transaction::new(self, self.doc.transact()))
    }

    pub fn with_transact_mut<T>(&self, f: impl FnOnce(crate::TransactionMut<'_, Self>) -> T) -> T {
        f(crate::TransactionMut::new(self, self.doc.transact_mut()))
    }

It can be abstracted into a Trait.

If we want to add Block to Workspace, we can do this:

                workspace.with_transact_mut(|mut t| {
                    let block = t.create(&block_id, "text");

                    let count = block.with_transact_mut(|mut t| {
                        payload
                            .into_iter()
                            .filter_map(|(key, value)| {
                                serde_json::from_value::<Any>(value)
                                    .map(|value| t.set(&key, value))
                                    .ok()
                            })
                            .count()
                    });

                    Some(((count > 0).then(|| t.encode_update_v1()), block))
                })

Or

block.transaction(|mut t| {}, &mut txn);

block.transaction(|mut t| {});

let transaction = block.transaction();

[darkskygit]

i prefer to abstract into traits

Another thing that can be improved is that we need to do some error handling for transform/transact_mut -- i means switch to try_transact/try_transact_mut

The transact function call try_transact in internal and unwrap it directly, this will cause thread panic in a multi-threaded environment.

It may be a good choice to rewrite with_trx as an async function:

// try to get transaction internally with try_transact and automatically retry
workspace.with_transact(|t| {
    // do something with transaction
}).await;

[fundon]

https://github.com/toeverything/OctoBase/blob/edb3f2ce0aa697374b92c87af7b3df3703a96b7d/libs/jwst/src/workspaces/workspace.rs#L247-L249

Direct unwrap is definitely wrong.

Using try_read/try_write for lock. This also corresponds to the try_transact/try_transact_mut.

For error handling, Result<Response> is a bit better.