From cfdd3680a4a0f671c5ef543ecd3f6d049f3a9415 Mon Sep 17 00:00:00 2001 From: Thomas Lauf Date: Sun, 22 Jul 2018 00:01:31 +0200 Subject: [PATCH] #9 TI-1: Move starting and stopping of transactions to commands --- src/Database.cpp | 66 ++++++++++++++---------------------- src/commands/CmdCancel.cpp | 2 ++ src/commands/CmdConfig.cpp | 16 +++------ src/commands/CmdContinue.cpp | 4 +++ src/commands/CmdDelete.cpp | 4 +++ src/commands/CmdFill.cpp | 4 +++ src/commands/CmdJoin.cpp | 4 +++ src/commands/CmdLengthen.cpp | 4 +++ src/commands/CmdMove.cpp | 4 +++ src/commands/CmdResize.cpp | 4 +++ src/commands/CmdShorten.cpp | 4 +++ src/commands/CmdSplit.cpp | 4 +++ src/commands/CmdStart.cpp | 4 +++ src/commands/CmdStop.cpp | 4 +++ src/commands/CmdTag.cpp | 4 +++ src/commands/CmdTrack.cpp | 4 +++ src/commands/CmdUntag.cpp | 4 +++ 17 files changed, 89 insertions(+), 51 deletions(-) diff --git a/src/Database.cpp b/src/Database.cpp index f7fc1b3e..d593ee58 100644 --- a/src/Database.cpp +++ b/src/Database.cpp @@ -24,16 +24,11 @@ // //////////////////////////////////////////////////////////////////////////////// -#include #include -#include #include -#include -#include #include #include #include -#include //////////////////////////////////////////////////////////////////////////////// void Database::initialize (const std::string& location) @@ -97,8 +92,6 @@ std::vector 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,58 +160,59 @@ 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) { - _currentTransaction = std::make_shared (); + throw "Subsequent call to start transaction"; } - ++_txn; + _currentTransaction = std::make_shared (); } //////////////////////////////////////////////////////////////////////////////// -// 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 (_txn == 0) + if (_currentTransaction == nullptr) { - File undo (_location + "/undo.data"); + throw "Call to end non-existent transaction"; + } - if (undo.open ()) - { - undo.append (_currentTransaction->toString()); + File undo (_location + "/undo.data"); - undo.close (); - _currentTransaction.reset (); - } - else - { - throw format ("Unable to write the undo transaction to {1}", undo._data); - } + if (undo.open ()) + { + undo.append (_currentTransaction->toString()); + + undo.close (); + _currentTransaction.reset (); + } + else + { + 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); } diff --git a/src/commands/CmdCancel.cpp b/src/commands/CmdCancel.cpp index 410c1d7d..4a0a840d 100644 --- a/src/commands/CmdCancel.cpp +++ b/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"; diff --git a/src/commands/CmdConfig.cpp b/src/commands/CmdConfig.cpp index 06e67abf..2e73655f 100644 --- a/src/commands/CmdConfig.cpp +++ b/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) diff --git a/src/commands/CmdContinue.cpp b/src/commands/CmdContinue.cpp index f5a65c9b..d69c2557 100644 --- a/src/commands/CmdContinue.cpp +++ b/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); diff --git a/src/commands/CmdDelete.cpp b/src/commands/CmdDelete.cpp index 43e7a6b6..37fa9a53 100644 --- a/src/commands/CmdDelete.cpp +++ b/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; } diff --git a/src/commands/CmdFill.cpp b/src/commands/CmdFill.cpp index 18765ccb..a6101303 100644 --- a/src/commands/CmdFill.cpp +++ b/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; } diff --git a/src/commands/CmdJoin.cpp b/src/commands/CmdJoin.cpp index 336eb4ec..26607560 100644 --- a/src/commands/CmdJoin.cpp +++ b/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'; diff --git a/src/commands/CmdLengthen.cpp b/src/commands/CmdLengthen.cpp index d76127f2..7115b490 100644 --- a/src/commands/CmdLengthen.cpp +++ b/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; } diff --git a/src/commands/CmdMove.cpp b/src/commands/CmdMove.cpp index 2d8ebfb8..330a65ab 100644 --- a/src/commands/CmdMove.cpp +++ b/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'; diff --git a/src/commands/CmdResize.cpp b/src/commands/CmdResize.cpp index d5712853..31fb79db 100644 --- a/src/commands/CmdResize.cpp +++ b/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; } diff --git a/src/commands/CmdShorten.cpp b/src/commands/CmdShorten.cpp index b46218f6..05102cd5 100644 --- a/src/commands/CmdShorten.cpp +++ b/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; } diff --git a/src/commands/CmdSplit.cpp b/src/commands/CmdSplit.cpp index d8358458..08ee6635 100644 --- a/src/commands/CmdSplit.cpp +++ b/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; } diff --git a/src/commands/CmdStart.cpp b/src/commands/CmdStart.cpp index 7367825c..eee7c3b5 100644 --- a/src/commands/CmdStart.cpp +++ b/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; } diff --git a/src/commands/CmdStop.cpp b/src/commands/CmdStop.cpp index 54809853..d791ad07 100644 --- a/src/commands/CmdStop.cpp +++ b/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; } diff --git a/src/commands/CmdTag.cpp b/src/commands/CmdTag.cpp index 94465b03..3f3aab20 100644 --- a/src/commands/CmdTag.cpp +++ b/src/commands/CmdTag.cpp @@ -53,6 +53,8 @@ int CmdTag ( bool dirty = true; + database.startTransaction (); + for (auto& id : ids) { if (id > static_cast (tracked.size ())) @@ -109,6 +111,8 @@ int CmdTag ( } } + database.endTransaction (); + return 0; } diff --git a/src/commands/CmdTrack.cpp b/src/commands/CmdTrack.cpp index c54ab58e..59aed308 100644 --- a/src/commands/CmdTrack.cpp +++ b/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; } diff --git a/src/commands/CmdUntag.cpp b/src/commands/CmdUntag.cpp index 496db963..9bb9a92c 100644 --- a/src/commands/CmdUntag.cpp +++ b/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; }