Skip to content

Commit

Permalink
Guard against accessing doc store during transaction
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanw committed Oct 3, 2023
1 parent 6daea64 commit cf7383b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 18 deletions.
62 changes: 44 additions & 18 deletions src/y_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ pub struct YDocInner {
}

impl YDocInner {
pub fn has_transaction(&self) -> bool {
if let Some(weak_txn) = &self.txn {
if let Some(txn) = weak_txn.upgrade() {
return !txn.borrow().committed
}
}
false
}

pub fn begin_transaction(&mut self) -> Rc<RefCell<YTransactionInner>> {
// Check if we think we still have a transaction
if let Some(weak_txn) = &self.txn {
Expand Down Expand Up @@ -109,6 +118,17 @@ pub struct YDoc {
pub inner: Rc<RefCell<YDocInner>>,
}

impl YDoc {
pub fn guard_store(&self) -> PyResult<()> {
if self.inner.borrow().has_transaction() {
return Err(pyo3::exceptions::PyAssertionError::new_err(
"Transaction already started!",
));
}
Ok(())
}
}

#[pymethods]
impl YDoc {
/// Creates a new Ypy document. If `client_id` parameter was passed it will be used as this
Expand Down Expand Up @@ -197,8 +217,9 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YMap` instance.
pub fn get_map(&mut self, name: &str) -> YMap {
self.inner.borrow().doc.get_or_insert_map(name).with_doc(self.inner.clone())
pub fn get_map(&mut self, name: &str) -> PyResult<YMap> {
self.guard_store()?;
Ok(self.inner.borrow().doc.get_or_insert_map(name).with_doc(self.inner.clone()))
}

/// Returns a `YXmlElement` shared data type, that's accessible for subsequent accesses using
Expand All @@ -208,12 +229,13 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YXmlElement` instance.
pub fn get_xml_element(&mut self, name: &str) -> YXmlElement {
self.inner
pub fn get_xml_element(&mut self, name: &str) -> PyResult<YXmlElement> {
self.guard_store()?;
Ok(self.inner
.borrow()
.doc
.get_or_insert_xml_element(name)
.with_doc(self.inner.clone())
.with_doc(self.inner.clone()))
}

/// Returns a `YXmlText` shared data type, that's accessible for subsequent accesses using given
Expand All @@ -223,8 +245,9 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YXmlText` instance.
pub fn get_xml_text(&mut self, name: &str) -> YXmlText {
self.inner.borrow().doc.get_or_insert_xml_text(name).with_doc(self.inner.clone())
pub fn get_xml_text(&mut self, name: &str) -> PyResult<YXmlText> {
self.guard_store()?;
Ok(self.inner.borrow().doc.get_or_insert_xml_text(name).with_doc(self.inner.clone()))
}

/// Returns a `YXmlFragment` shared data type, that's accessible for subsequent accesses using
Expand All @@ -234,12 +257,13 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YXmlFragment` instance.
pub fn get_xml_fragment(&mut self, name: &str) -> YXmlFragment {
self.inner
pub fn get_xml_fragment(&mut self, name: &str) -> PyResult<YXmlFragment> {
self.guard_store()?;
Ok(self.inner
.borrow()
.doc
.get_or_insert_xml_fragment(name)
.with_doc(self.inner.clone())
.with_doc(self.inner.clone()))
}

/// Returns a `YArray` shared data type, that's accessible for subsequent accesses using given
Expand All @@ -249,12 +273,13 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YArray` instance.
pub fn get_array(&mut self, name: &str) -> YArray {
self.inner
pub fn get_array(&mut self, name: &str) -> PyResult<YArray> {
self.guard_store()?;
Ok(self.inner
.borrow()
.doc
.get_or_insert_array(&name)
.with_doc(self.inner.clone())
.get_or_insert_array(name)
.with_doc(self.inner.clone()))
}

/// Returns a `YText` shared data type, that's accessible for subsequent accesses using given
Expand All @@ -264,12 +289,13 @@ impl YDoc {
///
/// If there was an instance with this name, but it was of different type, it will be projected
/// onto `YText` instance.
pub fn get_text(&mut self, name: &str) -> YText {
self.inner
pub fn get_text(&mut self, name: &str) -> PyResult<YText> {
self.guard_store()?;
Ok(self.inner
.borrow()
.doc
.get_or_insert_text(&name)
.with_doc(self.inner.clone())
.get_or_insert_text(name)
.with_doc(self.inner.clone()))
}

/// Subscribes a callback to a `YDoc` lifecycle event.
Expand Down
10 changes: 10 additions & 0 deletions tests/test_y_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,13 @@ def test_transaction_already_committed():
txn.commit()
assert str(excinfo.value) == "Transaction already committed!"


def test_document_modification_during_transaction():
doc = Y.YDoc()
text = doc.get_text("test")
with doc.begin_transaction() as txn:
with pytest.raises(AssertionError) as excinfo:
text_2 = doc.get_text("test2")
assert str(excinfo.value) == "Transaction already started!"

doc.get_text("test2")

0 comments on commit cf7383b

Please sign in to comment.