From 541cc3be607dce17ef940ab2b9dbe6f55978c59d Mon Sep 17 00:00:00 2001 From: Alexander Parrill Date: Fri, 18 Oct 2024 15:01:15 -0400 Subject: [PATCH] Add XML related tests and fixes for found bugs --- .gitignore | 1 + python/pycrdt/_xml.py | 64 +++++++-------- src/text.rs | 2 +- src/xml.rs | 18 ++++- tests/test_xml.py | 177 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 227 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index 82f31ba..1edda7e 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ Cargo.lock .coverage /site /dist +_pycrdt.*.pyd diff --git a/python/pycrdt/_xml.py b/python/pycrdt/_xml.py index 3af0362..c3b60c0 100644 --- a/python/pycrdt/_xml.py +++ b/python/pycrdt/_xml.py @@ -135,41 +135,40 @@ def __init__( _doc: Doc | None = None, _integrated: _XmlElement | None = None, ) -> None: - if tag is None and attributes is None and contents is None: - init = None - elif (attributes is not None or contents is not None) and tag is None: - raise ValueError("Tag is required if specifying attributes or contents") + """ + Creates a new preliminary element. + + `tag` is required. + """ + if _integrated is not None: + super().__init__(init=None, _doc=_doc, _integrated=_integrated) + return + + if tag is None: + raise ValueError("XmlElement: tag is required") + + if isinstance(attributes, dict): + init_attrs = list(attributes.items()) + elif attributes is not None: + init_attrs = list(attributes) else: - if isinstance(attributes, dict): - init_attrs = list(attributes.items()) - elif attributes is not None: - init_attrs = list(attributes) - else: - init_attrs = [] + init_attrs = [] - init = ( + super().__init__( + init=( tag, init_attrs, list(contents) if contents is not None else [], ) - - super().__init__( - init=init, - _doc=_doc, - _integrated=_integrated, ) def to_py(self) -> None: raise ValueError("XmlElement has no Python equivalent") - def _get_or_insert(self, _name: str, _doc: Doc) -> Any: + def _get_or_insert(self, name: str, doc: Doc) -> Any: raise ValueError("Cannot get an XmlElement from a doc - get an XmlFragment instead.") - def _init( - self, value: tuple[str, list[tuple[str, str]], list[str | XmlElement | XmlText]] | None - ): - if value is None: - return + def _init(self, value: tuple[str, list[tuple[str, str]], list[str | XmlElement | XmlText]]): _, attrs, contents = value with self.doc.transaction(): for k, v in attrs: @@ -179,6 +178,9 @@ def _init( @property def tag(self) -> str | None: + """ + Gets the element's tag. + """ return self.integrated.tag() @@ -190,12 +192,12 @@ class XmlText(_XmlTraitMixin): of an `XmlElement` or `XmlFragment`. """ - _prelim: str | None + _prelim: str _integrated: _XmlText | None def __init__( self, - init: str | None = None, + init: str = "", *, _doc: Doc | None = None, _integrated: _XmlText | None = None, @@ -209,14 +211,12 @@ def __init__( def _get_or_insert(self, _name: str, _doc: Doc) -> Any: raise ValueError("Cannot get an XmlText from a doc - get an XmlFragment instead.") - def to_py(self) -> str | None: + def to_py(self) -> str: if self._integrated is None: return self._prelim return str(self) - def _init(self, value: str | None) -> None: - if value is None: - return + def _init(self, value: str) -> None: # pragma: no cover with self.doc.transaction() as txn: self.integrated.insert(txn._txn, 0, value) @@ -236,7 +236,7 @@ def insert(self, index: int, value: str, attrs: Mapping[str, Any] | None = None) with self.doc.transaction() as txn: self._forbid_read_transaction(txn) self.integrated.insert( - txn._txn, index, value, attrs.items() if attrs is not None else iter([]) + txn._txn, index, value, iter(attrs.items()) if attrs is not None else iter([]) ) def insert_embed(self, index: int, value: Any, attrs: dict[str, Any] | None = None) -> None: @@ -283,7 +283,7 @@ def __delitem__(self, key: int | slice) -> None: if length > 0: self.integrated.remove_range(txn._txn, start, length) else: - raise RuntimeError(f"Index not supported: {key}") + raise TypeError(f"Index not supported: {key}") def clear(self) -> None: """Remove the entire range of characters.""" @@ -407,7 +407,7 @@ def __delitem__(self, key: int | slice) -> None: if length > 0: self.inner.integrated.remove_range(txn._txn, start, length) else: - raise RuntimeError(f"Index not supported: {key}") + raise TypeError(f"Index not supported: {key}") def __setitem__(self, key: int, value: str | XmlText | XmlElement): """ @@ -459,7 +459,7 @@ def insert(self, index: int, element: str | XmlText | XmlElement) -> XmlText | X element._init(prelim) return element else: - raise ValueError("Cannot add value to XML: " + repr(element)) + raise TypeError("Cannot add value to XML: " + repr(element)) @overload def append(self, element: str | XmlText) -> XmlText: ... diff --git a/src/text.rs b/src/text.rs index e18c3f5..5bc3f1b 100644 --- a/src/text.rs +++ b/src/text.rs @@ -51,9 +51,9 @@ impl Text { #[pyo3(signature = (txn, index, embed, attrs=None))] fn insert_embed(&self, txn: &mut Transaction, index: u32, embed: Bound<'_, PyAny>, attrs: Option>) -> PyResult<()> { + let embed = py_to_any(&embed); let mut _t = txn.transaction(); let mut t = _t.as_mut().unwrap().as_mut(); - let embed = py_to_any(&embed); if let Some(attrs) = attrs { let attrs = py_to_attrs(attrs)?; self.text.insert_embed_with_attributes(&mut t, index, embed, attrs); diff --git a/src/xml.rs b/src/xml.rs index dfe7913..b01753e 100644 --- a/src/xml.rs +++ b/src/xml.rs @@ -1,6 +1,6 @@ use pyo3::types::{PyAnyMethods, PyDict, PyIterator, PyList, PyString, PyTuple}; -use pyo3::{pyclass, pymethods, Bound, IntoPy as _, PyObject, PyResult, Python}; +use pyo3::{pyclass, pymethods, Bound, IntoPy as _, PyAny, PyObject, PyResult, Python}; use yrs::types::text::YChange; use yrs::types::xml::{XmlEvent as _XmlEvent, XmlTextEvent as _XmlTextEvent}; use yrs::{ @@ -8,7 +8,7 @@ use yrs::{ }; use crate::subscription::Subscription; -use crate::type_conversions::{events_into_py, py_to_attrs, EntryChangeWrapper}; +use crate::type_conversions::{events_into_py, py_to_any, py_to_attrs, EntryChangeWrapper}; use crate::{transaction::Transaction, type_conversions::ToPython}; /// Implements methods common to `XmlFragment`, `XmlElement`, and `XmlText`. @@ -221,6 +221,20 @@ impl_xml_methods!(XmlText[text, xml: text] { Ok(()) } + #[pyo3(signature = (txn, index, embed, attrs=None))] + fn insert_embed<'py>(&self, txn: &mut Transaction, index: u32, embed: Bound<'py, PyAny>, attrs: Option>) -> PyResult<()> { + let embed = py_to_any(&embed); + let mut _t = txn.transaction(); + let mut t = _t.as_mut().unwrap().as_mut(); + if let Some(attrs) = attrs { + let attrs = py_to_attrs(attrs)?; + self.text.insert_embed_with_attributes(&mut t, index, embed, attrs); + } else { + self.text.insert_embed(&mut t, index, embed); + } + Ok(()) + } + fn remove_range(&self, txn: &mut Transaction, index: u32, len: u32) { let mut _t = txn.transaction(); let mut t = _t.as_mut().unwrap().as_mut(); diff --git a/tests/test_xml.py b/tests/test_xml.py index 2539796..77392c5 100644 --- a/tests/test_xml.py +++ b/tests/test_xml.py @@ -33,13 +33,20 @@ def test_api(): frag.doc assert str(excinfo.value) == "Not integrated in a document yet" + with pytest.raises(ValueError): + frag.to_py() + doc["test"] = frag + assert frag.parent is None assert str(frag) == 'Hello World!' assert len(frag.children) == 3 assert str(frag.children[0]) == "Hello " assert str(frag.children[1]) == 'World' assert str(frag.children[2]) == "!" assert list(frag.children) == [frag.children[0], frag.children[1], frag.children[2]] + assert frag.children[0].parent == frag + assert hash(frag.children[0].parent) == hash(frag) + assert frag != object() frag.children.insert(1, XmlElement("strong", None, ["wonderful"])) frag.children.insert(2, " ") @@ -63,6 +70,176 @@ def test_api(): assert str(frag) == 'Hello World!' +def test_text(): + text = XmlText("Hello") + assert text.to_py() == "Hello" + + doc = Doc() + + with pytest.raises(ValueError): + doc["test"] = XmlText("test") + + doc["test"] = XmlFragment([text]) + + assert str(text) == "Hello" + assert text.to_py() == "Hello" + assert len(text) == len("Hello") + + text.clear() + assert str(text) == "" + + text += "Goodbye" + assert str(text) == "Goodbye" + + text.insert(1, " ") + assert str(text) == "G oodbye" + del text[1] + assert str(text) == "Goodbye" + + text.insert(1, " ") + del text[1:3] + assert str(text) == "Goodbye" + + assert text.diff() == [("Goodbye", None)] + text.format(1, 3, {"bold": True}) + assert text.diff() == [ + ("G", None), + ("oo", {"bold": True}), + ("dbye", None), + ] + + text.insert_embed(0, b"PNG!", {"type": "image"}) + assert text.diff() == [ + (b"PNG!", {"type": "image"}), + ("G", None), + ("oo", {"bold": True}), + ("dbye", None), + ] + + text.insert(len(text), " World!", {"href": "some-url"}) + assert text.diff() == [ + (b"PNG!", {"type": "image"}), + ("G", None), + ("oo", {"bold": True}), + ("dbye", None), + (" World!", {"href": "some-url"}), + ] + + del text[0] + assert text.diff() == [ + ("G", None), + ("oo", {"bold": True}), + ("dbye", None), + (" World!", {"href": "some-url"}), + ] + + del text[0:3] + assert text.diff() == [ + ("dbye", None), + (" World!", {"href": "some-url"}), + ] + + with pytest.raises(RuntimeError): + del text[0:5:2] + with pytest.raises(RuntimeError): + del text[-1:5] + with pytest.raises(RuntimeError): + del text[1:-1] + with pytest.raises(TypeError): + del text["invalid"] + + doc["test2"] = XmlFragment([XmlText()]) + + +def test_element(): + doc = Doc() + + with pytest.raises(ValueError): + doc["test"] = XmlElement("test") + + with pytest.raises(ValueError): + XmlElement() + + doc["test"] = frag = XmlFragment() + + el = XmlElement("div", {"class": "test"}) + frag.children.append(el) + assert str(el) == '
' + + el = XmlElement("div", [("class", "test")]) + frag.children.append(el) + assert str(el) == '
' + + el = XmlElement("div", None, [XmlText("Test")]) + frag.children.append(el) + assert str(el) == "
Test
" + + el = XmlElement("div") + frag.children.append(el) + assert str(el) == "
" + + with pytest.raises(ValueError): + el.to_py() + + el.attributes["class"] = "test" + assert str(el) == '
' + assert "class" in el.attributes + assert el.attributes["class"] == "test" + assert el.attributes.get("class") == "test" + assert len(el.attributes) == 1 + assert list(el.attributes) == [("class", "test")] + + del el.attributes["class"] + assert str(el) == "
" + assert "class" not in el.attributes + assert el.attributes.get("class") is None + assert len(el.attributes) == 0 + assert list(el.attributes) == [] + + node = XmlText("Hello") + el.children.append(node) + assert str(el) == "
Hello
" + assert len(el.children) == 1 + assert str(el.children[0]) == "Hello" + assert list(el.children) == [node] + + el.children[0] = XmlText("Goodbye") + assert str(el) == "
Goodbye
" + + del el.children[0] + assert str(el) == "
" + + el.children.append(XmlElement("foo")) + el.children.append(XmlElement("bar")) + el.children.append(XmlElement("baz")) + assert str(el) == "
" + + del el.children[0:2] + assert str(el) == "
" + + with pytest.raises(TypeError): + del el.children["invalid"] + with pytest.raises(IndexError): + el.children[1] + + text = XmlText("foo") + el.children.insert(0, text) + assert str(el) == "
foo
" + + el2 = XmlElement("bar") + el.children.insert(1, el2) + assert str(el) == "
foo
" + + with pytest.raises(IndexError): + el.children.insert(10, "test") + with pytest.raises(ValueError): + el.children.append(text) + with pytest.raises(ValueError): + el.children.append(el2) + with pytest.raises(TypeError): + el.children.append(object()) + + def test_observe(): doc = Doc() doc["test"] = fragment = XmlFragment(["Hello world!"])