Update to Taskchampion-2.0.1 (#37)

Co-authored-by: illya-laifu <[email protected]>

.github/workflows/checks.yml

@@ -29,7 +29,7 @@ jobs:
 
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: "1.78.0" # MSRV
+          toolchain: "1.81.0" # MSRV
           override: true
           components: clippy
 

.github/workflows/ci.yml

@@ -191,7 +191,6 @@ jobs:
       - uses: actions/setup-python@v5
         with:
           python-version: 3.9
-      # See #36 for upgrading to poetry 2.0.
       - run: pip install 'poetry<2'
       - run: poetry install
       - run: poetry run pip install wheels-linux-x86_64/*

.github/workflows/tests.yml

@@ -29,15 +29,13 @@ jobs:
 
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: "1.78.0" # MSRV
+          toolchain: "1.81.0" # MSRV
           override: true
           components: clippy
 
       - uses: actions/setup-python@v5
         with:
           python-version: 3.9
-
-      # See #36 for upgrading to poetry 2.0.
       - run: pip install 'poetry<2'
       - run: poetry install
       - run: poetry run maturin develop

Cargo.toml

@@ -1,9 +1,9 @@
 [package]
 name = "taskchampion-py"
-version = "1.0.2"
+version = "2.0.1"
 edition = "2021"
 # This should match the MSRV of the `taskchampion` crate.
-rust-version = "1.78.0"
+rust-version = "1.81.0"
 
 [package.metadata.maturin]
 name = "taskchampion"
@@ -16,4 +16,4 @@ doc = false
 [dependencies]
 pyo3 = { version = "0.22.6", features = ["chrono"] }
 chrono = "*"
-taskchampion = { version = "=1.0.2" }
+taskchampion = { version = "=2.0.1" }

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "maturin"
 
 [project]
 name = "taskchampion-py"
-version = "1.0.2"
+version = "2.0.1"
 requires-python = ">=3.9"
 classifiers = [
     "Programming Language :: Rust",

src/access_mode.rs

@@ -0,0 +1,27 @@
+use pyo3::prelude::*;
+pub use taskchampion::storage::AccessMode as TCAccessMode;
+
+#[pyclass(eq, eq_int)]
+#[derive(Clone, Copy, PartialEq)]
+pub enum AccessMode {
+    ReadOnly,
+    ReadWrite,
+}
+
+impl From<TCAccessMode> for AccessMode {
+    fn from(status: TCAccessMode) -> Self {
+        match status {
+            TCAccessMode::ReadOnly => AccessMode::ReadOnly,
+            TCAccessMode::ReadWrite => AccessMode::ReadWrite,
+        }
+    }
+}
+
+impl From<AccessMode> for TCAccessMode {
+    fn from(status: AccessMode) -> Self {
+        match status {
+            AccessMode::ReadOnly => TCAccessMode::ReadOnly,
+            AccessMode::ReadWrite => TCAccessMode::ReadWrite,
+        }
+    }
+}

src/dependency_map.rs

@@ -1,14 +1,11 @@
 use pyo3::prelude::*;
+use std::sync::Arc;
 use taskchampion::{DependencyMap as TCDependencyMap, Uuid};
 
 // See `Replica::dependency_map` for the rationale for using a raw pointer here.
 
 #[pyclass]
-pub struct DependencyMap(*const TCDependencyMap);
-
-// SAFETY: `Replica::dependency_map` ensures that the TCDependencyMap is never freed (as the Rc is
-// leaked) and TaskChampion does not modify it, so no races can occur.
-unsafe impl Send for DependencyMap {}
+pub struct DependencyMap(Arc<TCDependencyMap>);
 
 #[pymethods]
 impl DependencyMap {
@@ -33,16 +30,14 @@ impl DependencyMap {
     }
 }
 
-impl From<*const TCDependencyMap> for DependencyMap {
-    fn from(value: *const TCDependencyMap) -> Self {
+impl From<Arc<TCDependencyMap>> for DependencyMap {
+    fn from(value: Arc<TCDependencyMap>) -> Self {
         DependencyMap(value)
     }
 }
 
 impl AsRef<TCDependencyMap> for DependencyMap {
     fn as_ref(&self) -> &TCDependencyMap {
-        // SAFETY: `Replica::dependency_map` ensures that the TCDependencyMap is never freed (as
-        // the Rc is leaked) and TaskChampion does not modify it, so no races can occur.
-        unsafe { &*self.0 as &TCDependencyMap }
+        Arc::as_ref(&self.0)
     }
 }

src/lib.rs

@@ -9,6 +9,8 @@ pub mod dependency_map;
 use dependency_map::*;
 pub mod operation;
 use operation::*;
+pub mod access_mode;
+use access_mode::*;
 pub mod operations;
 use operations::*;
 mod task;
@@ -28,6 +30,7 @@ fn taskchampion(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_class::<DependencyMap>()?;
     m.add_class::<Operation>()?;
     m.add_class::<Operations>()?;
+    m.add_class::<AccessMode>()?;
 
     Ok(())
 }

src/replica.rs

@@ -1,9 +1,8 @@
 use crate::task::TaskData;
 use crate::util::{into_runtime_error, uuid2tc};
-use crate::{DependencyMap, Operations, Task, WorkingSet};
+use crate::{AccessMode, DependencyMap, Operations, Task, WorkingSet};
 use pyo3::prelude::*;
 use std::collections::HashMap;
-use std::rc::Rc;
 use taskchampion::{Replica as TCReplica, ServerConfig, StorageConfig};
 
 #[pyclass(unsendable)]
@@ -22,13 +21,21 @@ impl Replica {
     /// Args:
     ///     path (str): path to the directory with the database
     ///     create_if_missing (bool): create the database if it does not exist
+    ///     access_mode (AccessMode): controls whether write access is allowed
     /// Raises:
     ///     RuntimeError: if database does not exist, and create_if_missing is false
-    pub fn new_on_disk(path: String, create_if_missing: bool) -> PyResult<Replica> {
+
+    #[pyo3(signature=(path, create_if_missing, access_mode=AccessMode::ReadWrite))]
+    pub fn new_on_disk(
+        path: String,
+        create_if_missing: bool,
+        access_mode: AccessMode,
+    ) -> PyResult<Replica> {
         Ok(Replica(TCReplica::new(
             StorageConfig::OnDisk {
                 taskdb_dir: path.into(),
                 create_if_missing,
+                access_mode: access_mode.into(),
             }
             .into_storage()
             .map_err(into_runtime_error)?,
@@ -91,19 +98,8 @@ impl Replica {
     }
 
     pub fn dependency_map(&mut self, force: bool) -> PyResult<DependencyMap> {
-        // `Rc<T>` is not thread-safe, so we must get an owned copy of the data it contains.
-        // Unfortunately, it cannot be cloned, so this is impossible (but both issues are fixed in
-        // https://github.com/GothenburgBitFactory/taskchampion/pull/514).
-        //
-        // Until that point, we leak the Rc (preventing it from ever being freed) and use a static
-        // reference to its contents. This is safe based on the weak but currently valid assumption
-        // that TaskChampion does not modify a DependencyMap after creating it.
-        //
-        // This is a temporary hack, and should not be used in "real" code!
         let dm = self.0.dependency_map(force).map_err(into_runtime_error)?;
-        // NOTE: this does not decrement the reference count and thus "leaks" the Rc.
-        let dm_ptr = Rc::into_raw(dm);
-        Ok(dm_ptr.into())
+        Ok(dm.into())
     }
 
     pub fn get_task(&mut self, uuid: String) -> PyResult<Option<Task>> {

src/task/annotation.rs

@@ -3,7 +3,7 @@ use pyo3::prelude::*;
 use taskchampion::Annotation as TCAnnotation;
 
 #[pyclass(frozen, eq)]
-#[derive(PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 /// An annotation for the task
 pub struct Annotation(TCAnnotation);
 

src/task/task.rs

@@ -5,9 +5,7 @@ use chrono::{DateTime, Utc};
 use pyo3::prelude::*;
 use taskchampion::Task as TCTask;
 
-// TODO: This type can be send once https://github.com/GothenburgBitFactory/taskchampion/pull/514
-// is available.
-#[pyclass(unsendable)]
+#[pyclass]
 /// A TaskChampion Task.
 ///
 /// This type is not Send, so it cannot be used from any thread but the one where it was created.
@@ -264,14 +262,7 @@ impl Task {
             .map_err(into_runtime_error)
     }
 
-    pub fn add_annotation(
-        &mut self,
-        annotation: &Annotation,
-        ops: &mut Operations,
-    ) -> PyResult<()> {
-        // Create an owned annotation (TODO: not needed once
-        // https://github.com/GothenburgBitFactory/taskchampion/pull/517 is available)
-        let annotation = Annotation::new(annotation.entry(), annotation.description());
+    pub fn add_annotation(&mut self, annotation: Annotation, ops: &mut Operations) -> PyResult<()> {
         self.0
             .add_annotation(annotation.into(), ops.as_mut())
             .map_err(into_runtime_error)

src/working_set.rs

@@ -1,5 +1,3 @@
-use std::fmt;
-
 use pyo3::prelude::*;
 use taskchampion::Uuid;
 use taskchampion::WorkingSet as TCWorkingSet;
@@ -29,7 +27,7 @@ impl WorkingSet {
     }
 
     pub fn __repr__(&self) -> String {
-        format!("{:?}", self)
+        format!("{:?}", self.0)
     }
 
     pub fn largest_index(&self) -> usize {
@@ -61,16 +59,6 @@ impl WorkingSet {
     }
 }
 
-// TODO: Use the Taskchampion Debug implementation when
-// https://github.com/GothenburgBitFactory/taskchampion/pull/520 is available.
-impl fmt::Debug for WorkingSet {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("WorkingSet {")?;
-        f.debug_list().entries(self.0.iter()).finish()?;
-        f.write_str("}")
-    }
-}
-
 impl AsRef<TCWorkingSet> for WorkingSet {
     fn as_ref(&self) -> &TCWorkingSet {
         &self.0

taskchampion.pyi

@@ -18,7 +18,11 @@ __all__ = [
 @final
 class Replica:
     @staticmethod
-    def new_on_disk(path: str, create_if_missing: bool): ...
+    def new_on_disk(
+        path: str,
+        create_if_missing: bool,
+        access_mode: "AccessMode" = AccessMode.ReadWrite,
+    ): ...
     @staticmethod
     def new_in_memory(): ...
     def create_task(self, uuid: str, ops: "Operations") -> "Task": ...
@@ -48,6 +52,11 @@ class Replica:
     def commit_operations(self, ops: "Operations") -> None: ...
     def commit_reversed_operations(self, operations: "Operations") -> None: ...
 
+@final
+class AccessMode(Enum):
+    ReadWrite = 1
+    ReadOnly = 2
+
 @final
 class Operation:
     @staticmethod

tests/test_replica.py

@@ -2,7 +2,7 @@
 from pathlib import Path
 
 import pytest
-from taskchampion import Replica, Operations
+from taskchampion import Replica, Operations, AccessMode
 
 
 @pytest.fixture
@@ -23,7 +23,6 @@ def replica_with_tasks(empty_replica: Replica):
 
 def test_constructor(tmp_path: Path):
     r = Replica.new_on_disk(str(tmp_path), True)
-
     assert r is not None
 
 
@@ -47,6 +46,14 @@ def test_constructor_throws_error_with_missing_database(tmp_path: Path):
         Replica.new_on_disk(str(tmp_path), False)
 
 
+def test_read_only(tmp_path: Path):
+    r = Replica.new_on_disk(str(tmp_path), True, AccessMode.ReadOnly)
+    ops = Operations()
+    r.create_task(str(uuid.uuid4()), ops)
+    with pytest.raises(RuntimeError):
+        r.commit_operations(ops)
+
+
 def test_create_task(empty_replica: Replica):
     u = uuid.uuid4()