diff --git a/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp b/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp index 4da20e4e33..2b5d62daf7 100644 --- a/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp +++ b/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp @@ -269,7 +269,7 @@ class IOHandler // rename grayscale array to reflect original { auto& gray = m_DataStructure.getDataRefAs(imageDataPath.replaceName("gray" + imageDataPath.getTargetName())); - if(!gray.canRename(imageDataPath.getTargetName())) + if(gray.canRename(imageDataPath.getTargetName()) == false) { return MakeErrorResult(-18543, fmt::format("Unable to rename the grayscale array to {}", imageDataPath.getTargetName())); } diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.cpp index 85d7eddd09..c48d47edec 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.cpp @@ -56,6 +56,8 @@ Parameters RenameDataObjectFilter::parameters() const Parameters params; params.insertSeparator(Parameters::Separator{"Input Parameters"}); + params.insert(std::make_unique(k_AllowOverwrite_Key, "Allow Overwrite", + "If true existing object with `New Name` and all of its nested objects will be removed to free up the name for the target DataObject", false)); params.insert(std::make_unique(k_SourceDataObjectPath_Key, "DataObject to Rename", "DataPath pointing to the target DataObject", DataPath())); params.insert(std::make_unique(k_NewName_Key, "New Name", "Target DataObject name", "")); return params; @@ -71,10 +73,11 @@ IFilter::UniquePointer RenameDataObjectFilter::clone() const IFilter::PreflightResult RenameDataObjectFilter::preflightImpl(const DataStructure& dataStructure, const Arguments& filterArgs, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel) const { + auto allowOverwrite = filterArgs.value(k_AllowOverwrite_Key); auto dataGroupPath = filterArgs.value(k_SourceDataObjectPath_Key); auto newName = filterArgs.value(k_NewName_Key); - auto action = std::make_unique(dataGroupPath, newName); + auto action = std::make_unique(dataGroupPath, newName, allowOverwrite); OutputActions actions; actions.appendAction(std::move(action)); diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.hpp index 17b8067544..5dae8d2d44 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.hpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObjectFilter.hpp @@ -25,6 +25,7 @@ class SIMPLNXCORE_EXPORT RenameDataObjectFilter : public IFilter RenameDataObjectFilter& operator=(RenameDataObjectFilter&&) noexcept = delete; // Parameter Keys + static inline constexpr StringLiteral k_AllowOverwrite_Key = "allow_overwrite"; static inline constexpr StringLiteral k_SourceDataObjectPath_Key = "source_data_object_path"; static inline constexpr StringLiteral k_NewName_Key = "new_name"; diff --git a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp index 185492dcb8..9ae87f46d9 100644 --- a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp +++ b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp @@ -66,3 +66,88 @@ TEST_CASE("SimplnxCore::RenameDataAction(Valid Parameters)", "[SimplnxCore][Rena REQUIRE(dataObject->getName() == k_NewName); } + +TEST_CASE("SimplnxCore::RenameDataAction(Valid Overwrite)", "[SimplnxCore][RenameDataAction]") +{ + static constexpr StringLiteral k_NewName = Constants::k_GroupHName; + static const DataPath k_DataPath({Constants::k_GroupAName, Constants::k_GroupCName, Constants::k_GroupDName, Constants::k_ArrayIName}); + + RenameDataObjectFilter filter; + DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); + Arguments args; + + args.insert(RenameDataObjectFilter::k_AllowOverwrite_Key, std::make_any(true)); + args.insert(RenameDataObjectFilter::k_NewName_Key, std::make_any(k_NewName)); + args.insert(RenameDataObjectFilter::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); + + // There is a warning clause, but under current implementation it won't be reached + // bool warningFound = false; + // for(const auto& warning : preflightResult.outputActions.warnings()) + // { + // if(warning.code == -6602) + // { + // warningFound = true; + // } + // } + // REQUIRE(warningFound); + + auto result = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(result.result); + + // Verify rename was successful + { + DataPath newPath({Constants::k_GroupAName, Constants::k_GroupCName, Constants::k_GroupDName, k_NewName}); + auto* dataObject = dataStructure.getData(newPath); + REQUIRE(dataObject != nullptr); + + REQUIRE(dataObject->getName() == k_NewName); + } + + // Verify old DataGroup (`H`) was removed + { + DataPath oldHPath({Constants::k_GroupAName, Constants::k_GroupHName}); + auto* dataObject = dataStructure.getData(oldHPath); + REQUIRE(dataObject == nullptr); + } + + // Verify old DataGroup (`H`) sub-array (`N`) was removed + { + DataPath oldHChildPath({Constants::k_GroupAName, Constants::k_GroupHName, Constants::k_ArrayNName}); + auto* dataObject = dataStructure.getData(oldHChildPath); + REQUIRE(dataObject == nullptr); + } +} + +TEST_CASE("SimplnxCore::RenameDataAction(InValid Overwrite)", "[SimplnxCore][RenameDataAction]") +{ + static constexpr StringLiteral k_NewName = Constants::k_GroupDName; + static const DataPath k_DataPath({Constants::k_GroupAName, Constants::k_GroupCName, Constants::k_GroupDName, Constants::k_ArrayIName}); + + RenameDataObjectFilter filter; + + DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); + Arguments args; + + args.insert(RenameDataObjectFilter::k_AllowOverwrite_Key, std::make_any(true)); + args.insert(RenameDataObjectFilter::k_NewName_Key, std::make_any(k_NewName)); + args.insert(RenameDataObjectFilter::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); + + auto result = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(result.result); + + bool errorFound = false; + for(const auto& error : result.result.errors()) + { + if(error.code == -6601) + { + errorFound = true; + } + } + REQUIRE(errorFound); +} diff --git a/src/simplnx/Common/Result.hpp b/src/simplnx/Common/Result.hpp index a2b15a7deb..280ec0f9c2 100644 --- a/src/simplnx/Common/Result.hpp +++ b/src/simplnx/Common/Result.hpp @@ -259,19 +259,6 @@ inline bool ExtractResult(Result<> result, std::vector& errors, std::vect template inline Result MergeResults(Result first, Result second) { - usize warningsSize = first.warnings().size() + second.warnings().size(); - std::vector warnings; - warnings.reserve(warningsSize); - - for(auto&& warning : first.warnings()) - { - warnings.push_back(std::move(warning)); - } - for(auto&& warning : second.warnings()) - { - warnings.push_back(std::move(warning)); - } - usize errorsSize = 0; if(first.invalid()) { @@ -299,8 +286,18 @@ inline Result MergeResults(Result first, Result second) } } - Result result = errors.empty() ? Result{} : Result{nonstd::make_unexpected(std::move(errors))}; - result.warnings() = std::move(warnings); + Result result = errors.empty() ? Result{} : Result{{nonstd::make_unexpected(std::move(errors))}}; + + result.m_Warnings.reserve(first.warnings().size() + second.warnings().size()); + for(auto&& warning : first.warnings()) + { + result.m_Warnings.push_back(std::move(warning)); + } + for(auto&& warning : second.warnings()) + { + result.m_Warnings.push_back(std::move(warning)); + } + return result; } diff --git a/src/simplnx/DataStructure/BaseGroup.cpp b/src/simplnx/DataStructure/BaseGroup.cpp index 37af7527f5..a6d8a61858 100644 --- a/src/simplnx/DataStructure/BaseGroup.cpp +++ b/src/simplnx/DataStructure/BaseGroup.cpp @@ -246,3 +246,8 @@ void BaseGroup::checkUpdatedIdsImpl(const std::vector> { m_DataMap.updateIds(updatedIds); } + +std::vector BaseGroup::GetChildrenIds() +{ + return m_DataMap.getKeys(); +} \ No newline at end of file diff --git a/src/simplnx/DataStructure/BaseGroup.hpp b/src/simplnx/DataStructure/BaseGroup.hpp index ad500392da..909dfd7e32 100644 --- a/src/simplnx/DataStructure/BaseGroup.hpp +++ b/src/simplnx/DataStructure/BaseGroup.hpp @@ -328,6 +328,11 @@ class SIMPLNX_EXPORT BaseGroup : public DataObject */ std::vector GetChildrenNames(); + /** + * @brief Querys the DataMap for the object ids in m_DataMap + */ + std::vector GetChildrenIds(); + protected: /** * @brief Creates a BaseGroup with the target DataStructure and name. diff --git a/src/simplnx/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index e63f35d50d..e4828e368d 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -7,11 +7,47 @@ using namespace nx::core; +namespace +{ +Result<> TerminateNodesRecursively(DataStructure& dataStructure, DataObject::IdType id, IDataAction::Mode mode, const bool checkDependence) +{ + Result<> result = {}; + DataObject* node = dataStructure.getData(id); + + if(checkDependence) + { + auto parentIds = node->getParentIds(); + if(parentIds.size() > 1) + { + return {}; + } + } + + auto* baseGroup = dataStructure.getDataAs(id); + if(baseGroup != nullptr) + { + auto childIds = baseGroup->GetChildrenIds(); + if(!childIds.empty()) + { + for(const auto& childId : childIds) + { + result = MergeResults(result, std::move(TerminateNodesRecursively(dataStructure, childId, mode, checkDependence))); + } + } + } + + dataStructure.removeData(node->getId()); + + return result; +} +} // namespace + namespace nx::core { -RenameDataAction::RenameDataAction(const DataPath& path, const std::string& newName) +RenameDataAction::RenameDataAction(const DataPath& path, const std::string& newName, bool overwrite) : m_NewName(newName) , m_Path(path) +, m_Overwrite(overwrite) { } @@ -27,14 +63,64 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const return MakeErrorResult(-6601, ss); } - if(!dataObject->canRename(m_NewName)) + Result<> result = {}; + + if(m_Overwrite) + { + if(dataObject->getName() == m_NewName) + { + return result; + } + + std::vector pathVec = m_Path.getPathVector(); + for(const auto& name : pathVec) + { + if(name == m_NewName) + { + std::string ss = fmt::format("{}The object that would be overwritten is a parent container to {} cannot rename to {}", prefix, m_Path.getTargetName(), m_NewName); + return MakeErrorResult(-6601, ss); + } + } + + DataObject::IdType targetId = std::numeric_limits::max(); + for(auto dataObjectID : dataStructure.getAllDataObjectIds()) + { + if(dataStructure.getData(dataObjectID)->getName() == m_NewName) + { + targetId = dataObjectID; + break; + } + } + + if(targetId != std::numeric_limits::max()) + { + if(mode == Mode::Preflight) + { + std::string ss = fmt::format("{}Another object exists with that name, will overwrite destroying other DataObject at '{}' and replacing it with '{}'", prefix, m_NewName, m_Path.toString()); + result.warnings().emplace_back(Warning{-6602, ss}); + } + else + { + if(dataStructure.getDataAs(targetId) != nullptr) + { + // Recursive removal of overwritten object + result = MergeResults(result, ::TerminateNodesRecursively(dataStructure, targetId, mode, true)); + } + else + { + dataStructure.removeData(targetId); + } + } + } + } + else if(!dataObject->canRename(m_NewName)) { std::string ss = fmt::format("{}Could not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); - return MakeErrorResult(-6602, ss); + return MakeErrorResult(-6603, ss); } dataObject->rename(m_NewName); - return {}; + return result; } IDataAction::UniquePointer RenameDataAction::clone() const @@ -51,4 +137,10 @@ const DataPath& RenameDataAction::path() const { return m_Path; } + +bool RenameDataAction::overwrite() const +{ + return m_Overwrite; +} + } // namespace nx::core diff --git a/src/simplnx/Filter/Actions/RenameDataAction.hpp b/src/simplnx/Filter/Actions/RenameDataAction.hpp index 5bd8872e42..e2e07083d2 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.hpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.hpp @@ -13,7 +13,7 @@ class SIMPLNX_EXPORT RenameDataAction : public IDataAction public: RenameDataAction() = delete; - RenameDataAction(const DataPath& path, const std::string& newName); + RenameDataAction(const DataPath& path, const std::string& newName, bool overwrite = false); ~RenameDataAction() noexcept override; @@ -48,8 +48,15 @@ class SIMPLNX_EXPORT RenameDataAction : public IDataAction */ const DataPath& path() const; + /** + * @brief Returns the overwrite value + * @return + */ + bool overwrite() const; + private: std::string m_NewName; DataPath m_Path; + bool m_Overwrite; }; } // namespace nx::core