Add XML related tests and fixes for found bugs

Author: ColonelThirtyTwo

.gitignore

@@ -5,3 +5,4 @@ Cargo.lock
 .coverage
 /site
 /dist
+_pycrdt.*.pyd

python/pycrdt/_xml.py

@@ -135,41 +135,41 @@ 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]] | None):
+        assert value is not None
         _, attrs, contents = value
         with self.doc.transaction():
             for k, v in attrs:
@@ -179,6 +179,9 @@ def _init(
 
     @property
     def tag(self) -> str | None:
+        """
+        Gets the element's tag.
+        """
         return self.integrated.tag()
 
 
@@ -190,12 +193,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 +212,13 @@ 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) -> None:  # pragma: no cover
+        assert value is not None
         with self.doc.transaction() as txn:
             self.integrated.insert(txn._txn, 0, value)
 
@@ -236,7 +238,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 +285,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 +409,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 +461,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: ...

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<Bound<'_, PyIterator>>) -> 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);

src/xml.rs

@@ -1,14 +1,14 @@
 
 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::{
     DeepObservable, GetString as _, Observable as _, Text as _, TransactionMut, Xml as _, XmlElementPrelim, XmlElementRef, XmlFragment as _, XmlFragmentRef, XmlOut, XmlTextPrelim, XmlTextRef
 };
 
 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<Bound<'_, PyIterator>>) -> 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();

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 <em class="bold">World</em>!'
     assert len(frag.children) == 3
     assert str(frag.children[0]) == "Hello "
     assert str(frag.children[1]) == '<em class="bold">World</em>'
     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 <em class="bold">World</em>!'
 
 
+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) == '<div class="test"></div>'
+
+    el = XmlElement("div", [("class", "test")])
+    frag.children.append(el)
+    assert str(el) == '<div class="test"></div>'
+
+    el = XmlElement("div", None, [XmlText("Test")])
+    frag.children.append(el)
+    assert str(el) == "<div>Test</div>"
+
+    el = XmlElement("div")
+    frag.children.append(el)
+    assert str(el) == "<div></div>"
+
+    with pytest.raises(ValueError):
+        el.to_py()
+
+    el.attributes["class"] = "test"
+    assert str(el) == '<div class="test"></div>'
+    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) == "<div></div>"
+    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) == "<div>Hello</div>"
+    assert len(el.children) == 1
+    assert str(el.children[0]) == "Hello"
+    assert list(el.children) == [node]
+
+    el.children[0] = XmlText("Goodbye")
+    assert str(el) == "<div>Goodbye</div>"
+
+    del el.children[0]
+    assert str(el) == "<div></div>"
+
+    el.children.append(XmlElement("foo"))
+    el.children.append(XmlElement("bar"))
+    el.children.append(XmlElement("baz"))
+    assert str(el) == "<div><foo></foo><bar></bar><baz></baz></div>"
+
+    del el.children[0:2]
+    assert str(el) == "<div><baz></baz></div>"
+
+    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) == "<div>foo<baz></baz></div>"
+
+    el2 = XmlElement("bar")
+    el.children.insert(1, el2)
+    assert str(el) == "<div>foo<bar></bar><baz></baz></div>"
+
+    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!"])