#9 TI-1: Move starting and stopping of transactions to commands

src/Database.cpp

@@ -24,16 +24,11 @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-#include <cmake.h>
 #include <Database.h>
-#include <FS.h>
 #include <format.h>
-#include <algorithm>
-#include <sstream>
 #include <iterator>
 #include <iomanip>
 #include <TransactionsFactory.h>
-#include <ctime>
 
 ////////////////////////////////////////////////////////////////////////////////
 void Database::initialize (const std::string& location)
@@ -97,8 +92,6 @@ std::vector <std::string> Database::allLines ()
 ////////////////////////////////////////////////////////////////////////////////
 void Database::addInterval (const Interval& interval)
 {
-  startTransaction ();
-
   if (interval.range.is_open ())
   {
     // Get the index into _files for the appropriate Datafile, which may be
@@ -127,15 +120,11 @@ void Database::addInterval (const Interval& interval)
       recordIntervalAction ("", segmentedInterval.json ());
     }
   }
-
-  endTransaction ();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 void Database::deleteInterval (const Interval& interval)
 {
-  startTransaction ();
-
   auto intervalRange = interval.range;
   for (auto& segment : segmentRange (intervalRange))
   {
@@ -153,8 +142,6 @@ void Database::deleteInterval (const Interval& interval)
 
     recordIntervalAction (segmentedInterval.json (), "");
   }
-
-  endTransaction ();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -164,8 +151,6 @@ void Database::deleteInterval (const Interval& interval)
 // Interval belongs in a different file.
 void Database::modifyInterval (const Interval& from, const Interval& to)
 {
-  startTransaction ();
-
   if (!from.empty ())
   {
     deleteInterval (from);
@@ -175,33 +160,27 @@ void Database::modifyInterval (const Interval& from, const Interval& to)
   {
     addInterval (to);
   }
-
-  endTransaction ();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// The _txn member is a reference count, allowing multiple nested transactions.
-// This accommodates the Database::modifyInterval call, that in turn calls
-// ::addInterval and ::deleteInterval.
 void Database::startTransaction ()
 {
-  if (_txn == 0)
+  if (_currentTransaction != nullptr)
   {
+    throw "Subsequent call to start transaction";
+  }
+
   _currentTransaction = std::make_shared <Transaction> ();
 }
 
-  ++_txn;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
-// The _txn member is a reference count. The undo data is only written when
-// ::endTransaction decrements the counter to zero, therefore the undo command can
-// perform multiple atomic steps.
 void Database::endTransaction ()
 {
-  --_txn;
+  if (_currentTransaction == nullptr)
-  if (_txn == 0)
   {
+    throw "Call to end non-existent transaction";
+  }
+
   File undo (_location + "/undo.data");
 
   if (undo.open ())
@@ -216,17 +195,24 @@ void Database::endTransaction ()
     throw format ("Unable to write the undo transaction to {1}", undo._data);
   }
 }
-}
 
 ////////////////////////////////////////////////////////////////////////////////
-// Record undoable transactions. There are several types:
+// Record undoable actions. There are several types:
 //   interval    changes to stored intervals
 //   config      changes to configuration
+//
+// Actions are only recorded if a transaction is open
+//
 void Database::recordUndoAction (
   const std::string &type,
   const std::string &before,
   const std::string &after)
 {
+  if (_currentTransaction == nullptr)
+  {
+    return;
+  }
+
   _currentTransaction->addUndoAction (type, before, after);
 }
 

src/commands/CmdCancel.cpp

@@ -44,7 +44,9 @@ int CmdCancel (
     return 0;
   }
 
+  database.startTransaction ();
   database.deleteInterval(latest);
+  database.endTransaction ();
 
   if (rules.getBoolean ("verbose"))
     std::cout << "Canceled active time tracking.\n";

src/commands/CmdConfig.cpp

@@ -77,9 +77,7 @@ static bool setConfigVariable (
           auto before = line;
           line = line.substr (0, pos) + name + " = " + value;
 
-          database.startTransaction ();
           database.recordConfigAction (before, line);
-          database.endTransaction ();
 
           change = true;
         }
@@ -112,9 +110,7 @@ static bool setConfigVariable (
             auto before = line;
             line = line.substr (0, pos) + leaf + " " + value;
 
-            database.startTransaction ();
             database.recordConfigAction (before, line);
-            database.endTransaction ();
 
             change = true;
           }
@@ -137,9 +133,7 @@ static bool setConfigVariable (
         // Add new line.
         lines.push_back (name + " = " + json::encode (value));
 
-        database.startTransaction ();
         database.recordConfigAction ("", lines.back ());
-        database.endTransaction ();
 
         change = true;
       }
@@ -160,9 +154,7 @@ static bool setConfigVariable (
       // Add new line.
       lines.push_back (name + " = " + json::encode (value));
 
-      database.startTransaction ();
       database.recordConfigAction ("", lines.back ());
-      database.endTransaction ();
 
       change = true;
     }
@@ -212,9 +204,7 @@ static int unsetConfigVariable (
       if (! confirmation ||
           confirm (format ("Are you sure you want to remove '{1}'?", name)))
       {
-        database.startTransaction ();
         database.recordConfigAction (line, "");
-        database.endTransaction ();
 
         line = "";
         change = true;
@@ -288,13 +278,15 @@ int CmdConfig (
   std::string name = words[0];
   std::string value;
 
-  if (name.empty ())
+  if (name.empty ()) // is this possible?
   {
     return CmdShow (rules);
   }
 
   bool change = false;
 
+  database.startTransaction ();
+
   // timew config name value
   // timew config name ""
   if (words.size () > 1)
@@ -338,6 +330,8 @@ int CmdConfig (
     }
   }
 
+  database.endTransaction ();
+
   if (rules.getBoolean ("verbose"))
   {
     if (change)

src/commands/CmdContinue.cpp

@@ -73,6 +73,8 @@ int CmdContinue (
   Datetime start_time;
   Datetime end_time;
 
+  database.startTransaction ();
+
   if (filter.range.start.toEpoch () != 0)
   {
     start_time = filter.range.start;
@@ -110,6 +112,8 @@ int CmdContinue (
   validate (cli, rules, database, to_copy);
   database.addInterval (to_copy);
 
+  database.endTransaction ();
+
   if (rules.getBoolean ("verbose"))
     std::cout << intervalSummarize (database, rules, to_copy);
 

src/commands/CmdDelete.cpp

@@ -45,6 +45,8 @@ int CmdDelete (
   Interval filter;
   auto tracked = getTracked (database, rules, filter);
 
+  database.startTransaction ();
+
   bool dirty = true;
 
   for (auto& id : ids)
@@ -77,6 +79,8 @@ int CmdDelete (
       std::cout << "Deleted @" << id << '\n';
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdFill.cpp

@@ -46,6 +46,8 @@ int CmdFill (
   Interval filter;
   auto tracked = getTracked (database, rules, filter);
 
+  database.startTransaction ();
+
   // Apply tags to ids.
   for (auto& id : ids)
   {
@@ -65,6 +67,8 @@ int CmdFill (
     // Note: Feedback generated inside autoFill().
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdJoin.cpp

@@ -60,6 +60,8 @@ int CmdJoin (
 
   }
 
+  database.startTransaction ();
+
   auto first_id  = std::min (*ids.begin (), *ids.end ());
   auto second_id = std::max (*ids.begin (), *ids.end ());
 
@@ -77,6 +79,8 @@ int CmdJoin (
   validate (cli, rules, database, combined);
   database.addInterval (combined);
 
+  database.endTransaction ();
+
   if (rules.getBoolean ("verbose"))
     std::cout << "Joined @" << first_id << " and @" << second_id << '\n';
 

src/commands/CmdLengthen.cpp

@@ -53,6 +53,8 @@ int CmdLengthen (
       delta = arg.attribute ("raw");
   }
 
+  database.startTransaction ();
+
   // Load the data.
   // Note: There is no filter.
   Interval filter;
@@ -102,6 +104,8 @@ int CmdLengthen (
       std::cout << "Lengthened @" << id << " by " << dur.formatHours () << '\n';
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdMove.cpp

@@ -51,6 +51,8 @@ int CmdMove (
     throw std::string ("ID must be specified. See 'timew help move'.");
   }
 
+  database.startTransaction ();
+
   int id = *ids.begin ();
 
   std::string new_start;
@@ -107,6 +109,8 @@ int CmdMove (
   validate (cli, rules, database, i);
   database.addInterval (i);
 
+  database.endTransaction ();
+
   if (rules.getBoolean ("verbose"))
     std::cout << "Moved @" << id << " to " << i.range.start.toISOLocalExtended () << '\n';
 

src/commands/CmdResize.cpp

@@ -51,6 +51,8 @@ int CmdResize (
       delta = arg.attribute ("raw");
   }
 
+  database.startTransaction ();
+
   // Load the data.
   // Note: There is no filter.
   Interval filter;
@@ -77,6 +79,8 @@ int CmdResize (
       std::cout << "Resized @" << id << " to " << dur.formatHours () << '\n';
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdShorten.cpp

@@ -51,6 +51,8 @@ int CmdShorten (
       delta = arg.attribute ("raw");
   }
 
+  database.startTransaction ();
+
   // Load the data.
   // Note: There is no filter.
   Interval filter;
@@ -103,6 +105,8 @@ int CmdShorten (
       std::cout << "Shortened @" << id << " by " << dur.formatHours () << '\n';
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdSplit.cpp

@@ -48,6 +48,8 @@ int CmdSplit (
   Interval filter;
   auto tracked = getTracked (database, rules, filter);
 
+  database.startTransaction ();
+
   // Apply tags to ids.
   for (auto& id : ids)
   {
@@ -84,6 +86,8 @@ int CmdSplit (
       std::cout << "Split @" << id << '\n';
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdStart.cpp

@@ -38,6 +38,8 @@ int CmdStart (
   auto filter = getFilter (cli);
   auto latest = getLatestInterval (database);
 
+  database.startTransaction ();
+
   // If the latest interval is open, close it.
   if (latest.range.is_open ())
   {
@@ -88,6 +90,8 @@ int CmdStart (
   if (rules.getBoolean ("verbose"))
     std::cout << intervalSummarize (database, rules, now);
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdStop.cpp

@@ -54,6 +54,8 @@ int CmdStop (
   if (! latest.range.is_open ())
     throw std::string ("There is no active time tracking.");
 
+  database.startTransaction ();
+
   Interval modified {latest};
 
   // If a stop date is specified (and occupies filter.range.start) then use
@@ -106,6 +108,8 @@ int CmdStop (
       std::cout << '\n' << intervalSummarize (database, rules, modified);
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdTag.cpp

@@ -53,6 +53,8 @@ int CmdTag (
 
   bool dirty = true;
 
+  database.startTransaction ();
+
   for (auto& id : ids)
   {
     if (id > static_cast <int> (tracked.size ()))
@@ -109,6 +111,8 @@ int CmdTag (
     }
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdTrack.cpp

@@ -42,6 +42,8 @@ int CmdTrack (
       ! filter.range.is_ended ())
     return CmdStart (cli, rules, database);
 
+  database.startTransaction ();
+
   // Validation must occur before flattening.
   validate (cli, rules, database, filter);
 
@@ -53,6 +55,8 @@ int CmdTrack (
       std::cout << intervalSummarize (database, rules, interval);
   }
 
+  database.endTransaction ();
+
   return 0;
 }
 

src/commands/CmdUntag.cpp

@@ -46,6 +46,8 @@ int CmdUntag (
     throw std::string ("At least one tag must be specified. See 'timew help untag'.");
   }
 
+  database.startTransaction ();
+
   // Load the data.
   // Note: There is no filter.
   Interval filter;
@@ -109,6 +111,8 @@ int CmdUntag (
     }
   }
 
+  database.endTransaction ();
+
   return 0;
 }