From fc0601dc2b0ba097b9f7368d5e9eaedf671f4f67 Mon Sep 17 00:00:00 2001 From: BillSenior <89970704+BillSenior@users.noreply.github.com> Date: Wed, 28 Aug 2024 17:37:27 +0200 Subject: [PATCH] allow changing the stack size (#364 | GRIDEDIT-1404) --- .../UndoActions/UndoActionStack.hpp | 15 +++- .../src/UndoActions/UndoActionStack.cpp | 35 +++++++- libs/MeshKernel/tests/src/UndoStackTests.cpp | 89 +++++++++++++++++-- .../include/MeshKernelApi/MeshKernel.hpp | 7 ++ libs/MeshKernelApi/src/MeshKernel.cpp | 20 +++++ libs/MeshKernelApi/tests/src/UndoTests.cpp | 75 ++++++++++++++++ 6 files changed, 229 insertions(+), 12 deletions(-) diff --git a/libs/MeshKernel/include/MeshKernel/UndoActions/UndoActionStack.hpp b/libs/MeshKernel/include/MeshKernel/UndoActions/UndoActionStack.hpp index 2ac357752..d9c26a6f0 100644 --- a/libs/MeshKernel/include/MeshKernel/UndoActions/UndoActionStack.hpp +++ b/libs/MeshKernel/include/MeshKernel/UndoActions/UndoActionStack.hpp @@ -44,13 +44,19 @@ namespace meshkernel class UndoActionStack { public: - /// @brief Maximum number of undo action items - static const UInt MaxUndoSize; + /// @brief Default maximum number of undo action items + static const UInt DefaultMaxUndoSize; // Add info about the action, both short and long form // Perhaps Short for for menu items, long form for tooltips? // long form for any exceptions? + /// @brief Constructor with maximum number of undo actions allowed + UndoActionStack(const UInt maximumSize = DefaultMaxUndoSize); + + /// @brief Set the maximum undo stack size. + void SetMaximumSize(const UInt maximumSize); + /// @brief Add an UndoAction with an associated action-id. /// /// All added undo-actions must be in the committed state, if not then a ConstraintError @@ -96,7 +102,7 @@ namespace meshkernel /// The number of restored actions. UInt RestoredSize(const int actionId = constants::missing::intValue) const; - /// \brief Compute the approximate amount of memory being used, in bytes, for all undo actions. + /// @brief Compute the approximate amount of memory being used, in bytes, for all undo actions. std::uint64_t MemorySize() const; private: @@ -115,6 +121,9 @@ namespace meshkernel /// @brief Stack of restored undo actions std::list m_restored; + + /// @brief Maximum number of undo action items + UInt m_maxUndoSize = DefaultMaxUndoSize; }; } // namespace meshkernel diff --git a/libs/MeshKernel/src/UndoActions/UndoActionStack.cpp b/libs/MeshKernel/src/UndoActions/UndoActionStack.cpp index e8566fded..d500d4a0d 100644 --- a/libs/MeshKernel/src/UndoActions/UndoActionStack.cpp +++ b/libs/MeshKernel/src/UndoActions/UndoActionStack.cpp @@ -4,11 +4,13 @@ #include #include -const meshkernel::UInt meshkernel::UndoActionStack::MaxUndoSize = 50; +const meshkernel::UInt meshkernel::UndoActionStack::DefaultMaxUndoSize = 50; + +meshkernel::UndoActionStack::UndoActionStack(const UInt maximumSize) : m_maxUndoSize(maximumSize) {} void meshkernel::UndoActionStack::Add(UndoActionPtr&& action, const int actionId) { - if (action != nullptr) + if (m_maxUndoSize > 0 && action != nullptr) { if (action->GetState() != UndoAction::State::Committed) { @@ -22,7 +24,7 @@ void meshkernel::UndoActionStack::Add(UndoActionPtr&& action, const int actionId m_restored.remove_if([actionId](const UndoActionForMesh& undoAction) { return undoAction.m_actionId == actionId; }); - if (m_committed.size() > MaxUndoSize) + if (m_committed.size() > m_maxUndoSize) { // If the number of undo-actions is greater than the maximum, then remove the first item in the list. m_committed.pop_front(); @@ -34,6 +36,26 @@ void meshkernel::UndoActionStack::Add(UndoActionPtr&& action, const int actionId } } +void meshkernel::UndoActionStack::SetMaximumSize(const UInt maximumSize) +{ + if (maximumSize == 0) + { + m_committed.clear(); + m_restored.clear(); + } + else if (maximumSize < m_committed.size()) + { + UInt loopLimit = static_cast(m_committed.size()); + + for (UInt i = maximumSize; i < loopLimit; ++i) + { + m_committed.pop_front(); + } + } + + m_maxUndoSize = maximumSize; +} + meshkernel::UInt meshkernel::UndoActionStack::Size() const { return static_cast(m_committed.size() + m_restored.size()); @@ -93,6 +115,13 @@ std::optional meshkernel::UndoActionStack::Commit() // Now move to committed stack m_committed.emplace_back(std::move(m_restored.back())); m_restored.pop_back(); + + if (m_committed.size() > m_maxUndoSize) + { + // If the number of undo-actions is greater than the maximum, then remove the first item in the list. + m_committed.pop_front(); + } + return actionId; } diff --git a/libs/MeshKernel/tests/src/UndoStackTests.cpp b/libs/MeshKernel/tests/src/UndoStackTests.cpp index 32e694bae..c4a860633 100644 --- a/libs/MeshKernel/tests/src/UndoStackTests.cpp +++ b/libs/MeshKernel/tests/src/UndoStackTests.cpp @@ -450,7 +450,7 @@ TEST(UndoStackTests, MemorySizeStackTest) std::uint64_t expectedSize = sizeof(undoActionStack); // Fill undo actions to maximum - for (mk::UInt i = 0; i < mk::UndoActionStack::MaxUndoSize; ++i) + for (mk::UInt i = 0; i < mk::UndoActionStack::DefaultMaxUndoSize; ++i) { auto undoAction = std::make_unique(); expectedSize += undoAction->MemorySize(); @@ -472,20 +472,20 @@ TEST(UndoStackTests, ExeedingMaximumUndoActions) EXPECT_EQ(undoActionStack.Size(), 0); // Fill undo actions to maximum - for (mk::UInt i = 0; i < mk::UndoActionStack::MaxUndoSize; ++i) + for (mk::UInt i = 0; i < mk::UndoActionStack::DefaultMaxUndoSize; ++i) { undoActionStack.Add(std::make_unique()); EXPECT_EQ(undoActionStack.Size(), i + 1); } // Check the size - EXPECT_EQ(undoActionStack.Size(), mk::UndoActionStack::MaxUndoSize); + EXPECT_EQ(undoActionStack.Size(), mk::UndoActionStack::DefaultMaxUndoSize); // Now add another undo action. The number on the stack should not increase. undoActionStack.Add(std::make_unique()); // Check the size - EXPECT_EQ(undoActionStack.Size(), mk::UndoActionStack::MaxUndoSize); + EXPECT_EQ(undoActionStack.Size(), mk::UndoActionStack::DefaultMaxUndoSize); // Clear all undo actions form stack undoActionStack.Clear(); @@ -500,7 +500,7 @@ TEST(UndoStackTests, RemovingUndoActions) int actionId3 = 3; int unknownActionId = 4; - mk::UInt actionCount = (mk::UndoActionStack::MaxUndoSize - mk::UndoActionStack::MaxUndoSize % 3) / 3; + mk::UInt actionCount = (mk::UndoActionStack::DefaultMaxUndoSize - mk::UndoActionStack::DefaultMaxUndoSize % 3) / 3; EXPECT_EQ(undoActionStack.Size(), 0); @@ -541,7 +541,7 @@ TEST(UndoStackTests, RemovingUndoActionsRandomised) std::array actionCounts{0, 0, 0, 0}; int unknownActionId = 45; - mk::UInt actionCount = (mk::UndoActionStack::MaxUndoSize - mk::UndoActionStack::MaxUndoSize % NumberOfUndoActions) / NumberOfUndoActions; + mk::UInt actionCount = (mk::UndoActionStack::DefaultMaxUndoSize - mk::UndoActionStack::DefaultMaxUndoSize % NumberOfUndoActions) / NumberOfUndoActions; mk::UInt totalActionCount = NumberOfUndoActions * actionCount; EXPECT_EQ(undoActionStack.Size(), 0); @@ -583,3 +583,80 @@ TEST(UndoStackTests, RemovingUndoActionsRandomised) undoActionStack.Clear(); EXPECT_EQ(undoActionStack.Size(), 0); } + +TEST(UndoStackTests, InitaliseStackSizeToZero) +{ + mk::UndoActionStack undoActionStack(0); + EXPECT_EQ(undoActionStack.Size(), 0); + + // Add large number of undo actions + for (mk::UInt i = 0; i < mk::UndoActionStack::DefaultMaxUndoSize; ++i) + { + undoActionStack.Add(std::make_unique()); + } + + EXPECT_EQ(undoActionStack.Size(), 0); + // There should be nothing to undo + EXPECT_FALSE(undoActionStack.Undo()); +} + +TEST(UndoStackTests, SetStackSizeToZero) +{ + mk::UndoActionStack undoActionStack; + + EXPECT_EQ(undoActionStack.Size(), 0); + undoActionStack.SetMaximumSize(0); + + // Fill undo actions to maximum + for (mk::UInt i = 0; i < mk::UndoActionStack::DefaultMaxUndoSize; ++i) + { + undoActionStack.Add(std::make_unique()); + } + + EXPECT_EQ(undoActionStack.Size(), 0); + + // Cannot undo, there are no items stored in stack + EXPECT_FALSE(undoActionStack.Undo()); +} + +TEST(UndoStackTests, SetStackSizeToZeroAfterAdding) +{ + mk::UInt InitialStackSize = 20; + // This number must be small than InitialStackSize but larger than 0 + mk::UInt RevisedStackSize = 10; + + mk::UndoActionStack undoActionStack(InitialStackSize); + + // Fill undo actions to maximum + for (mk::UInt i = 0; i < InitialStackSize; ++i) + { + undoActionStack.Add(std::make_unique()); + } + + EXPECT_EQ(undoActionStack.Size(), InitialStackSize); + undoActionStack.SetMaximumSize(RevisedStackSize); + EXPECT_EQ(undoActionStack.Size(), RevisedStackSize); + + for (mk::UInt i = 0; i < RevisedStackSize; ++i) + { + EXPECT_TRUE(undoActionStack.Undo()); + } + + // Cannot undo further, there are no items left in stack + EXPECT_FALSE(undoActionStack.Undo()); + + // Redo all the undone actions + for (mk::UInt i = 0; i < RevisedStackSize; ++i) + { + EXPECT_TRUE(undoActionStack.Commit()); + } + + EXPECT_EQ(undoActionStack.Size(), RevisedStackSize); + + // Now finally set the maximum size to be zero. + undoActionStack.SetMaximumSize(0); + EXPECT_EQ(undoActionStack.Size(), 0); + + // Cannot undo, there are no items in stack + EXPECT_FALSE(undoActionStack.Undo()); +} diff --git a/libs/MeshKernelApi/include/MeshKernelApi/MeshKernel.hpp b/libs/MeshKernelApi/include/MeshKernelApi/MeshKernel.hpp index ea72fa81d..434005d78 100644 --- a/libs/MeshKernelApi/include/MeshKernelApi/MeshKernel.hpp +++ b/libs/MeshKernelApi/include/MeshKernelApi/MeshKernel.hpp @@ -74,6 +74,13 @@ namespace meshkernelapi /// @returns Error code MKERNEL_API int mkernel_is_valid_state(int meshKernelId, bool& isValid); + /// @brief Set the maximum size of the undo stack + /// + /// Setting the size to zero will disable the undo. + /// @param[in] undoStackSize The maximum size of the undo stack + /// @returns Error code + MKERNEL_API int mkernel_set_undo_size(int undoStackSize); + /// @brief Attempt to undo by one undo-action. /// @param[out] undone Indicates if the undo action was actually undone /// @param[out] meshKernelId The mesh kernel id related to the undo action diff --git a/libs/MeshKernelApi/src/MeshKernel.cpp b/libs/MeshKernelApi/src/MeshKernel.cpp index 090bd2bd4..b8bdc9d45 100644 --- a/libs/MeshKernelApi/src/MeshKernel.cpp +++ b/libs/MeshKernelApi/src/MeshKernel.cpp @@ -234,6 +234,26 @@ namespace meshkernelapi return lastExitCode; } + MKERNEL_API int mkernel_set_undo_size(int undoStackSize) + { + lastExitCode = meshkernel::ExitCode::Success; + + try + { + if (undoStackSize < 0) + { + throw meshkernel::MeshKernelError("Incorrect undo stack size: {}", undoStackSize); + } + + meshKernelUndoStack.SetMaximumSize(static_cast(undoStackSize)); + } + catch (...) + { + lastExitCode = HandleException(); + } + return lastExitCode; + } + MKERNEL_API int mkernel_undo_state(bool& undone, int& meshKernelId) { lastExitCode = meshkernel::ExitCode::Success; diff --git a/libs/MeshKernelApi/tests/src/UndoTests.cpp b/libs/MeshKernelApi/tests/src/UndoTests.cpp index eb1e1e0e9..78532d144 100644 --- a/libs/MeshKernelApi/tests/src/UndoTests.cpp +++ b/libs/MeshKernelApi/tests/src/UndoTests.cpp @@ -984,3 +984,78 @@ TEST(UndoTests, UnstructuredGridConnection) // The mesh kernel state object and all undo information should have be removed. EXPECT_TRUE(CheckUndoStateCount(meshKernelId1, 0, 0)); } + +TEST(UndoTests, SetUndoStackSize) +{ + + // Two mesh kernel ids will be created + // checks made on their validity at different stages of the test + // one of the id's will be deallocated + // further checks on validity + + int meshKernelId1 = meshkernel::constants::missing::intValue; + int meshKernelId2 = meshkernel::constants::missing::intValue; + int errorCode; + // Initialised with the opposite of the expected value + bool isValid = true; + + // Clear the meshkernel state and undo stack before starting the test. + errorCode = mkapi::mkernel_clear_state(); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + + // meshKernelId1 should be not valid + errorCode = mkapi::mkernel_is_valid_state(meshKernelId1, isValid); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_FALSE(isValid); + + errorCode = mkapi::mkernel_set_undo_size(0); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + + errorCode = mkapi::mkernel_allocate_state(0, meshKernelId1); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + + // Initialised with the opposite of the expected value + // meshKernelId1 should now be valid + isValid = false; + errorCode = mkapi::mkernel_is_valid_state(meshKernelId1, isValid); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_TRUE(isValid); + + // Initialised with the opposite of the expected value + // meshKernelId2 should still not be valid + isValid = true; + errorCode = mkapi::mkernel_is_valid_state(meshKernelId2, isValid); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_FALSE(isValid); + + errorCode = mkapi::mkernel_allocate_state(0, meshKernelId2); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + + // deallocate meshKernelId1 + errorCode = mkapi::mkernel_deallocate_state(meshKernelId1); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + + // Initialised with the opposite of the expected value + // meshKernelId2 should still be valid + isValid = false; + errorCode = mkapi::mkernel_is_valid_state(meshKernelId2, isValid); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_TRUE(isValid); + + int commitedSize = -1; + int restoredSize = -1; + + errorCode = mkapi::mkernel_undo_state_count(commitedSize, restoredSize); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_EQ(commitedSize, 0); + EXPECT_EQ(restoredSize, 0); + + bool didUndo = false; + int undoId = meshkernel::constants::missing::intValue; + + // Undo the deallocation + errorCode = mkapi::mkernel_undo_state(didUndo, undoId); + ASSERT_EQ(mk::ExitCode::Success, errorCode); + EXPECT_FALSE(didUndo); + EXPECT_EQ(undoId, meshkernel::constants::missing::intValue); +}