Add helper methods for checking indexes, keys and transactions (#49)

Author: davidbrochart

python/pycrdt/array.py

@@ -38,10 +38,7 @@ def _init(self, value: list[Any] | None) -> None:
 
     def _set(self, index: int, value: Any) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             if isinstance(value, BaseDoc):
                 # subdoc
                 self.integrated.insert_doc(txn._txn, index, value._doc)
@@ -76,16 +73,16 @@ def insert(self, index, object) -> None:
 
     def pop(self, index: int = -1) -> Any:
         with self.doc.transaction():
+            index = self._check_index(index)
             res = self[index]
             del self[index]
             return res
 
     def move(self, source_index: int, destination_index: int) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
+            source_index = self._check_index(source_index)
+            destination_index = self._check_index(destination_index)
             self.integrated.move_to(txn._txn, source_index, destination_index)
 
     def __add__(self, value: list[Any]) -> Array:
@@ -102,11 +99,7 @@ def __radd__(self, value: list[Any]) -> Array:
     def __setitem__(self, key: int | slice, value: Any | list[Any]) -> None:
         with self.doc.transaction():
             if isinstance(key, int):
-                length = len(self)
-                if length == 0:
-                    raise IndexError("Array index out of range")
-                if key < 0:
-                    key += length
+                key = self._check_index(key)
                 del self[key]
                 self[key:key] = [value]
             elif isinstance(key, slice):
@@ -121,18 +114,21 @@ def __setitem__(self, key: int | slice, value: Any | list[Any]) -> None:
             else:
                 raise RuntimeError(f"Index not supported: {key}")
 
+    def _check_index(self, idx: int) -> int:
+        if not isinstance(idx, int):
+            raise RuntimeError("Index must be of type int")
+        length = len(self)
+        if idx < 0:
+            idx += length
+        if idx < 0 or idx >= length:
+            raise IndexError("Array index out of range")
+        return idx
+
     def __delitem__(self, key: int | slice) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             if isinstance(key, int):
-                length = len(self)
-                if length == 0:
-                    raise IndexError("Array index out of range")
-                if key < 0:
-                    key += length
+                key = self._check_index(key)
                 self.integrated.remove_range(txn._txn, key, 1)
             elif isinstance(key, slice):
                 if key.step is not None:
@@ -156,11 +152,7 @@ def __delitem__(self, key: int | slice) -> None:
     def __getitem__(self, key: int) -> BaseType:
         with self.doc.transaction() as txn:
             if isinstance(key, int):
-                length = len(self)
-                if length == 0:
-                    raise IndexError("Array index out of range")
-                if key < 0:
-                    key += length
+                key = self._check_index(key)
                 return self._maybe_as_type_or_doc(self.integrated.get(txn._txn, key))
             elif isinstance(key, slice):
                 i0 = 0 if key.start is None else key.start

python/pycrdt/base.py

@@ -5,7 +5,7 @@
 
 from ._pycrdt import Doc as _Doc
 from ._pycrdt import Transaction as _Transaction
-from .transaction import Transaction
+from .transaction import ReadTransaction, Transaction
 
 if TYPE_CHECKING:
     from .doc import Doc
@@ -74,6 +74,12 @@ def _get_or_insert(self, name: str, doc: Doc) -> Any:
     def _init(self, value: Any | None) -> None:
         ...
 
+    def _forbid_read_transaction(self, txn: Transaction):
+        if isinstance(txn, ReadTransaction):
+            raise RuntimeError(
+                "Read-only transaction cannot be used to modify document structure"
+            )
+
     def _current_transaction(self) -> Transaction:
         if self._doc is None:
             raise RuntimeError("Not associated with a document")

python/pycrdt/map.py

@@ -38,10 +38,7 @@ def _init(self, value: dict[str, Any] | None) -> None:
 
     def _set(self, key: str, value: Any) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             if isinstance(value, BaseDoc):
                 # subdoc
                 self.integrated.insert_doc(txn._txn, key, value._doc)
@@ -70,19 +67,14 @@ def to_py(self) -> dict | None:
         return dict(self)
 
     def __delitem__(self, key: str) -> None:
-        if not isinstance(key, str):
-            raise RuntimeError("Key must be of type string")
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
+            self._check_key(key)
             self.integrated.remove(txn._txn, key)
 
     def __getitem__(self, key: str) -> Any:
         with self.doc.transaction() as txn:
-            if not isinstance(key, str):
-                raise RuntimeError("Key must be of type string")
+            self._check_key(key)
             return self._maybe_as_type_or_doc(self.integrated.get(txn._txn, key))
 
     def __setitem__(self, key: str, value: Any) -> None:
@@ -112,6 +104,12 @@ def pop(self, key: str, default_value: Any | None = None) -> Any:
             del self[key]
             return res
 
+    def _check_key(self, key: str):
+        if not isinstance(key, str):
+            raise RuntimeError("Key must be of type string")
+        if key not in self.keys():
+            raise KeyError(f"KeyError: {key}")
+
     def keys(self):
         with self.doc.transaction() as txn:
             return iter(self.integrated.keys(txn._txn))

python/pycrdt/text.py

@@ -53,19 +53,13 @@ def to_py(self) -> str | None:
 
     def __iadd__(self, value: str) -> Text:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             self.integrated.insert(txn._txn, len(self), value)
             return self
 
     def __delitem__(self, key: int | slice) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             if isinstance(key, int):
                 self.integrated.remove_range(txn._txn, key, 1)
             elif isinstance(key, slice):
@@ -89,10 +83,7 @@ def __delitem__(self, key: int | slice) -> None:
 
     def __setitem__(self, key: int | slice, value: str) -> None:
         with self.doc.transaction() as txn:
-            if isinstance(txn, ReadTransaction):
-                raise RuntimeError(
-                    "Read-only transaction cannot be used to modify document structure"
-                )
+            self._forbid_read_transaction(txn)
             if isinstance(key, int):
                 value_len = len(value)
                 if value_len != 1: