Open Replica read-only when possible (#3776)

* Open Replica read-only when possible

Specifically, when either
 - the command is read-only; or
 - the command requires GC (including recurrence updates) but GC is disabled by config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/Context.cpp

@@ -598,9 +598,6 @@ int Context::initialize(int argc, const char** argv) {
 
     createDefaultConfig();
 
-    bool create_if_missing = !config.getBoolean("exit.on.missing.db");
-    tdb2.open_replica(data_dir, create_if_missing);
-
     ////////////////////////////////////////////////////////////////////////////
     //
     // [3] Instantiate Command objects and capture command entities.
@@ -674,6 +671,21 @@ int Context::initialize(int argc, const char** argv) {
       if (foundAssumed) header("No command specified - assuming 'information'.");
     }
 
+    ////////////////////////////////////////////////////////////////////////////
+    //
+    // [7.5] Open the Replica.
+    //
+    ////////////////////////////////////////////////////////////////////////////
+
+    bool create_if_missing = !config.getBoolean("exit.on.missing.db");
+    Command* c = commands[cli2.getCommand()];
+
+    // We must allow writes if either 'gc' is enabled and the command performs GC, or the command
+    // itself is read-write.
+    bool read_write =
+        (config.getBoolean("gc") && (c->needs_gc() || c->needs_recur_update())) || !c->read_only();
+    tdb2.open_replica(data_dir, create_if_missing, read_write);
+
     ////////////////////////////////////////////////////////////////////////////
     //
     // [8] Initialize hooks.

src/TDB2.cpp

@@ -50,10 +50,13 @@ bool TDB2::debug_mode = false;
 static void dependency_scan(std::vector<Task>&);
 
 ////////////////////////////////////////////////////////////////////////////////
-void TDB2::open_replica(const std::string& location, bool create_if_missing) {
+void TDB2::open_replica(const std::string& location, bool create_if_missing, bool read_write) {
-  _replica = tc::new_replica_on_disk(location, create_if_missing);
+  _replica = tc::new_replica_on_disk(location, create_if_missing, read_write);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+void TDB2::open_replica_in_memory() { _replica = tc::new_replica_in_memory(); }
+
 ////////////////////////////////////////////////////////////////////////////////
 // Add the new task to the replica.
 void TDB2::add(Task& task) {
@@ -190,11 +193,8 @@ void TDB2::purge(Task& task) {
 
 ////////////////////////////////////////////////////////////////////////////////
 rust::Box<tc::Replica>& TDB2::replica() {
-  // Create a replica in-memory if `open_replica` has not been called. This
+  // One of the open_replica_ methods must be called before this one.
-  // occurs in tests.
+  assert(_replica);
-  if (!_replica) {
-    _replica = tc::new_replica_in_memory();
-  }
   return _replica.value();
 }
 

src/TDB2.h

@@ -46,7 +46,8 @@ class TDB2 {
 
   TDB2() = default;
 
-  void open_replica(const std::string &, bool create_if_missing);
+  void open_replica(const std::string &, bool create_if_missing, bool read_write);
+  void open_replica_in_memory();
   void add(Task &);
   void modify(Task &);
   void purge(Task &);

src/taskchampion-cpp/src/lib.rs

@@ -104,8 +104,11 @@ mod ffi {
         fn new_replica_in_memory() -> Result<Box<Replica>>;
 
         /// Create a new replica stored on-disk.
-        fn new_replica_on_disk(taskdb_dir: String, create_if_missing: bool)
+        fn new_replica_on_disk(
-            -> Result<Box<Replica>>;
+            taskdb_dir: String,
+            create_if_missing: bool,
+            read_write: bool,
+        ) -> Result<Box<Replica>>;
 
         /// Commit the given operations to the replica.
         fn commit_operations(&mut self, ops: Vec<Operation>) -> Result<()>;
@@ -490,11 +493,14 @@ impl From<tc::Replica> for Replica {
 fn new_replica_on_disk(
     taskdb_dir: String,
     create_if_missing: bool,
+    read_write: bool,
 ) -> Result<Box<Replica>, CppError> {
+    use tc::storage::AccessMode::*;
+    let access_mode = if read_write { ReadWrite } else { ReadOnly };
     let storage = tc::StorageConfig::OnDisk {
         taskdb_dir: PathBuf::from(taskdb_dir),
         create_if_missing,
-        access_mode: tc::storage::AccessMode::ReadWrite,
+        access_mode,
     }
     .into_storage()?;
     Ok(Box::new(tc::Replica::new(storage).into()))

test/CMakeLists.txt

@@ -157,6 +157,7 @@ set (pythonTests
   purge.test.py
   quotes.test.py
   rc.override.test.py
+  read-only.test.py
   recurrence.test.py
   reports.test.py
   search.test.py

test/make_tc_task.cpp

@@ -56,7 +56,7 @@ int main(int argc, char **argv) {
   }
   char *datadir = *++argv;
 
-  auto replica = tc::new_replica_on_disk(datadir, true);
+  auto replica = tc::new_replica_on_disk(datadir, /*create_if_missing=*/true, /*read_write=*/true);
   auto uuid = tc::uuid_v4();
   auto operations = tc::new_operations();
   auto task = tc::create_task(uuid, operations);

test/read-only.test.py

@@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+###############################################################################
+#
+# Copyright 2006 - 2021, Tomas Babej, Paul Beckingham, Federico Hernandez.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+# https://www.opensource.org/licenses/mit-license.php
+#
+###############################################################################
+
+import sys
+import os
+import platform
+import time
+import unittest
+
+# Ensure python finds the local simpletap module
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
+
+from basetest import Task, TestCase
+
+
+class TestReadOnly(TestCase):
+    def setUp(self):
+        self.t = Task()
+        self.t("add foo")
+
+        # set the mtime of the taskdb to an hour ago, so we can see any changes
+        self.taskdb = self.t.datadir + "/taskchampion.sqlite3"
+        os.utime(self.taskdb, (time.time() - 3600,) * 2)
+
+    def assertNotModified(self):
+        self.assertLess(os.stat(self.taskdb).st_mtime, time.time() - 1800)
+
+    def assertModified(self):
+        self.assertGreater(os.stat(self.taskdb).st_mtime, time.time() - 1800)
+
+    def test_read_only_command(self):
+        code, out, err = self.t("reports")
+        self.assertNotModified()
+
+    def test_report(self):
+        code, out, err = self.t("list")
+        self.assertModified()
+
+    def test_burndown(self):
+        code, out, err = self.t("burndown")
+        self.assertModified()
+
+    def test_report_gc_0(self):
+        self.t.config("gc", "0")
+        code, out, err = self.t("list")
+        self.assertNotModified()
+
+    def test_burndown_gc_0(self):
+        self.t.config("gc", "0")
+        code, out, err = self.t("burndown")
+        self.assertNotModified()
+
+
+if __name__ == "__main__":
+    from simpletap import TAPTestRunner
+
+    unittest.main(testRunner=TAPTestRunner())
+
+# vim: ai sts=4 et sw=4 ft=python

test/tdb2_test.cpp

@@ -61,7 +61,7 @@ int TEST_NAME(int, char**) {
     context.config.set("gc", 1);
     context.config.set("debug", 1);
 
-    context.tdb2.open_replica(".", true);
+    context.tdb2.open_replica(".", /*create_if_missing=*/true, /*read_write=*/true);
 
     // Try reading an empty database.
     std::vector<Task> pending = context.tdb2.pending_tasks();
@@ -108,7 +108,7 @@ int TEST_NAME(int, char**) {
 
     // Reset for reuse.
     cleardb();
-    context.tdb2.open_replica(".", true);
+    context.tdb2.open_replica(".", /*create_if_missing=*/true, /*read_write=*/true);
 
     // TODO complete a task
     // TODO gc

test/view_test.cpp

@@ -47,6 +47,7 @@ int TEST_NAME(int, char**) {
   UnitTest t(1);
   Context context;
   Context::setContext(&context);
+  context.tdb2.open_replica_in_memory();
 
   // Ensure environment has no influence.
   unsetenv("TASKDATA");